appeasing linter

Update artifact module quick start

.github/workflows/artifact-tests.yml

@@ -1,5 +1,3 @@
-# Temporarily disabled while v2.0.0 of @actions/artifact is under development
-
 name: artifact-unit-tests
 on:
   push:
@@ -12,8 +10,8 @@ on:
       - '**.md'
 
 jobs:
-  build:
-    name: Build
+  upload:
+    name: Upload
 
     strategy:
       matrix:
@@ -42,19 +40,13 @@ jobs:
         npm run tsc
       working-directory: packages/artifact
 
-    - name: Set artifact file contents
-      shell: bash
-      run: |
-        echo "file1=hello from file 1" >> $GITHUB_ENV
-        echo "file2=hello from file 2" >> $GITHUB_ENV
-
     - name: Create files that will be uploaded
       run: |
         mkdir artifact-path
-        echo '${{ env.file1 }}' > artifact-path/first.txt
-        echo '${{ env.file2 }}' > artifact-path/second.txt
+        echo -n 'hello from file 1' > artifact-path/first.txt
+        echo -n 'hello from file 2' > artifact-path/second.txt
 
-    - name: Upload Artifacts using actions/github-script@v7
+    - name: Upload Artifacts
       uses: actions/github-script@v7
       with:
         script: |
@@ -73,9 +65,16 @@ jobs:
 
           console.log(`Successfully uploaded artifact ${id}`)
 
+          try {
+            await artifact.uploadArtifact(artifactName, fileContents, './')
+            throw new Error('should have failed second upload')
+          } catch (err) {
+            console.log('Successfully blocked second artifact upload')
+          }
   verify:
+    name: Verify
     runs-on: ubuntu-latest
-    needs: [build]
+    needs: [upload]
     steps:
     - name: Checkout
       uses: actions/checkout@v4
@@ -96,35 +95,72 @@ jobs:
         npm run tsc
       working-directory: packages/artifact
 
-    - name: List artifacts using actions/github-script@v7
+    - name: List and Download Artifacts
       uses: actions/github-script@v7
       with:
         script: |
-          const {default: artifact} = require('./packages/artifact/lib/artifact')
+          const {default: artifactClient} = require('./packages/artifact/lib/artifact')
 
-          const workflowRunId = process.env.GITHUB_RUN_ID
-          const repository = process.env.GITHUB_REPOSITORY
-          const repositoryOwner = repository.split('/')[0]
-          const repositoryName = repository.split('/')[1]
+          const {readFile} = require('fs/promises')
+          const path = require('path')
 
-          const listResult = await artifact.listArtifacts(workflowRunId, repositoryOwner, repositoryName, '${{ secrets.GITHUB_TOKEN }}')
+          const findBy = {
+            repositoryOwner: process.env.GITHUB_REPOSITORY.split('/')[0],
+            repositoryName: process.env.GITHUB_REPOSITORY.split('/')[1],
+            token: '${{ secrets.GITHUB_TOKEN }}',
+            workflowRunId: process.env.GITHUB_RUN_ID
+          }
+
+          const listResult = await artifactClient.listArtifacts({latest: true, findBy})
           console.log(listResult)
 
           const artifacts = listResult.artifacts
+          const expected = [
+            'my-artifact-ubuntu-latest',
+            'my-artifact-windows-latest',
+            'my-artifact-macos-latest'
+          ]
 
-          if (artifacts.length !== 3) {
-            throw new Error('Expected 3 artifacts but only found ' + artifacts.length + ' artifacts')
-          }
+          const foundArtifacts = artifacts.filter(artifact =>
+            expected.includes(artifact.name)
+          )
 
-          const artifactNames = artifacts.map(artifact => artifact.name)
-          if (!artifactNames.includes('my-artifact-ubuntu-latest')){
-            throw new Error("Expected artifact list to contain an artifact named my-artifact-ubuntu-latest but it's missing")
-          }
-          if (!artifactNames.includes('my-artifact-windows-latest')){
-            throw new Error("Expected artifact list to contain an artifact named my-artifact-windows-latest but it's missing")
-          }
-          if (!artifactNames.includes('my-artifact-macos-latest')){
-            throw new Error("Expected artifact list to contain an artifact named my-artifact-macos-latest but it's missing")
+          if (foundArtifacts.length !== 3) {
+            console.log('Unexpected length of found artifacts', foundArtifacts)
+            throw new Error(
+              `Expected 3 artifacts but found ${foundArtifacts.length} artifacts.`
+            )
           }
 
           console.log('Successfully listed artifacts that were uploaded')
+
+          const files = [
+            {name: 'artifact-path/first.txt', content: 'hello from file 1'},
+            {name: 'artifact-path/second.txt', content: 'hello from file 2'}
+          ]
+
+          for (const artifact of foundArtifacts) {
+            const {downloadPath} = await artifactClient.downloadArtifact(artifact.id, {
+              path: artifact.name,
+              findBy
+            })
+
+            console.log('Downloaded artifact to:', downloadPath)
+
+            for (const file of files) {
+              const filepath = path.join(
+                process.env.GITHUB_WORKSPACE,
+                downloadPath,
+                file.name
+              )
+
+              console.log('Checking file:', filepath)
+
+              const content = await readFile(filepath, 'utf8')
+              if (content.trim() !== file.content.trim()) {
+                throw new Error(
+                  `Expected file '${file.name}' to contain '${file.content}' but found '${content}'`
+                )
+              }
+            }
+          }

jest.config.js

@@ -8,4 +8,4 @@ module.exports = {
     '^.+\\.ts$': 'ts-jest'
   },
   verbose: true
-}
+}

package-lock.json

@@ -5781,9 +5781,9 @@
       }
     },
     "node_modules/follow-redirects": {
-      "version": "1.15.2",
-      "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz",
-      "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==",
+      "version": "1.15.4",
+      "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.4.tgz",
+      "integrity": "sha512-Cr4D/5wlrb0z9dgERpUL3LrmPKVDsETIJhaCMeDfuFYcqa5bldGV6wBsAN6X/vxlXQtFBMrXdXxdL8CbDTGniw==",
       "dev": true,
       "funding": [
         {

package.json

@@ -13,7 +13,7 @@
     "lint": "eslint packages/**/*.ts",
     "lint-fix": "eslint packages/**/*.ts --fix",
     "new-package": "scripts/create-package",
-    "test": "jest --testTimeout 10000"
+    "test": "jest --testTimeout 60000"
   },
   "devDependencies": {
     "@types/jest": "^29.5.4",

packages/artifact/README.md

@@ -63,10 +63,16 @@ Import the module:
 
 ```js
 // ES6 module
-import artifact from '@actions/artifact'
+import {DefaultArtifactClient} from '@actions/artifact'
 
 // CommonJS
-const {default: artifact} = require('@actions/artifact')
+const {DefaultArtifactClient} = require('@actions/artifact')
+```
+
+Then instantiate:
+
+```js
+const artifact = new DefaultArtifactClient()
 ```
 
 ℹ️ For a comprehensive list of classes, interfaces, functions and more, see the [generated documentation](./docs/generated/README.md).

packages/artifact/__tests__/artifact-http-client.test.ts

@@ -4,6 +4,7 @@ import {HttpClient} from '@actions/http-client'
 import * as config from '../src/internal/shared/config'
 import {internalArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client'
 import {noopLogs} from './common'
+import {NetworkError, UsageError} from '../src/internal/shared/errors'
 
 jest.mock('@actions/http-client')
 
@@ -257,9 +258,42 @@ describe('artifact-http-client', () => {
         name: 'artifact',
         version: 4
       })
-    }).rejects.toThrowError(
-      'Failed to CreateArtifact: Unable to make request: ENOTFOUND\nIf you are using self-hosted runners, please make sure your runner has access to all GitHub endpoints: https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#communication-between-self-hosted-runners-and-github'
-    )
+    }).rejects.toThrowError(new NetworkError('ENOTFOUND').message)
+    expect(mockHttpClient).toHaveBeenCalledTimes(1)
+    expect(mockPost).toHaveBeenCalledTimes(1)
+  })
+
+  it('should properly describe a usage error', async () => {
+    const mockPost = jest.fn(() => {
+      const msgFailed = new http.IncomingMessage(new net.Socket())
+      msgFailed.statusCode = 403
+      msgFailed.statusMessage = 'Forbidden'
+      return {
+        message: msgFailed,
+        readBody: async () => {
+          return Promise.resolve(
+            `{"msg": "insufficient usage to create artifact"}`
+          )
+        }
+      }
+    })
+
+    const mockHttpClient = (
+      HttpClient as unknown as jest.Mock
+    ).mockImplementation(() => {
+      return {
+        post: mockPost
+      }
+    })
+    const client = internalArtifactTwirpClient()
+    await expect(async () => {
+      await client.CreateArtifact({
+        workflowRunBackendId: '1234',
+        workflowJobRunBackendId: '5678',
+        name: 'artifact',
+        version: 4
+      })
+    }).rejects.toThrowError(new UsageError().message)
     expect(mockHttpClient).toHaveBeenCalledTimes(1)
     expect(mockPost).toHaveBeenCalledTimes(1)
   })

packages/artifact/__tests__/download-artifact.test.ts

@@ -9,7 +9,8 @@ import archiver from 'archiver'
 
 import {
   downloadArtifactInternal,
-  downloadArtifactPublic
+  downloadArtifactPublic,
+  streamExtractExternal
 } from '../src/internal/download/download-artifact'
 import {getUserAgentString} from '../src/internal/shared/user-agent'
 import {noopLogs} from './common'
@@ -248,7 +249,44 @@ describe('download-artifact', () => {
       })
     })
 
-    it('should fail if blob storage response is non-200', async () => {
+    it('should fail if blob storage storage chunk does not respond within 30s', async () => {
+      // mock http client to delay response data by 30s
+      const msg = new http.IncomingMessage(new net.Socket())
+      msg.statusCode = 200
+
+      const mockGet = jest.fn(async () => {
+        return new Promise((resolve, reject) => {
+          // Resolve with a 200 status code immediately
+          resolve({
+            message: msg,
+            readBody: async () => {
+              return Promise.resolve(`{"ok": true}`)
+            }
+          })
+
+          // Reject with an error after 31 seconds
+          setTimeout(() => {
+            reject(new Error('Request timeout'))
+          }, 31000) // Timeout after 31 seconds
+        })
+      })
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGet
+          }
+        }
+      )
+
+      await expect(
+        streamExtractExternal(fixtures.blobStorageUrl, fixtures.workspaceDir)
+      ).rejects.toBeInstanceOf(Error)
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+    }, 35000) // add longer timeout to allow for timer to run out
+
+    it('should fail if blob storage response is non-200 after 5 retries', async () => {
       const downloadArtifactMock = github.getOctokit(fixtures.token).rest
         .actions.downloadArtifact as MockedDownloadArtifact
       downloadArtifactMock.mockResolvedValueOnce({
@@ -290,7 +328,60 @@ describe('download-artifact', () => {
       expect(mockGetArtifactFailure).toHaveBeenCalledWith(
         fixtures.blobStorageUrl
       )
-    })
+      expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5)
+    }, 38000)
+
+    it('should retry if blob storage response is non-200 and then succeed with a 200', async () => {
+      const downloadArtifactMock = github.getOctokit(fixtures.token).rest
+        .actions.downloadArtifact as MockedDownloadArtifact
+      downloadArtifactMock.mockResolvedValueOnce({
+        headers: {
+          location: fixtures.blobStorageUrl
+        },
+        status: 302,
+        url: '',
+        data: Buffer.from('')
+      })
+
+      const mockGetArtifact = jest
+        .fn(mockGetArtifactSuccess)
+        .mockImplementationOnce(mockGetArtifactFailure)
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetArtifact
+          }
+        }
+      )
+
+      const response = await downloadArtifactPublic(
+        fixtures.artifactID,
+        fixtures.repositoryOwner,
+        fixtures.repositoryName,
+        fixtures.token
+      )
+
+      expect(downloadArtifactMock).toHaveBeenCalledWith({
+        owner: fixtures.repositoryOwner,
+        repo: fixtures.repositoryName,
+        artifact_id: fixtures.artifactID,
+        archive_format: 'zip',
+        request: {
+          redirect: 'manual'
+        }
+      })
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      expect(mockGetArtifactFailure).toHaveBeenCalledWith(
+        fixtures.blobStorageUrl
+      )
+      expect(mockGetArtifactFailure).toHaveBeenCalledTimes(1)
+      expect(mockGetArtifactSuccess).toHaveBeenCalledWith(
+        fixtures.blobStorageUrl
+      )
+      expect(mockGetArtifactSuccess).toHaveBeenCalledTimes(1)
+      expect(response.downloadPath).toBe(fixtures.workspaceDir)
+    }, 28000)
   })
 
   describe('internal', () => {

packages/artifact/src/internal/download/download-artifact.ts

@@ -38,20 +38,65 @@ async function exists(path: string): Promise<boolean> {
 }
 
 async function streamExtract(url: string, directory: string): Promise<void> {
+  let retryCount = 0
+  while (retryCount < 5) {
+    try {
+      await streamExtractExternal(url, directory)
+      return
+    } catch (error) {
+      retryCount++
+      core.debug(
+        `Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...`
+      )
+      // wait 5 seconds before retrying
+      await new Promise(resolve => setTimeout(resolve, 5000))
+    }
+  }
+
+  throw new Error(`Artifact download failed after ${retryCount} retries.`)
+}
+
+export async function streamExtractExternal(
+  url: string,
+  directory: string
+): Promise<void> {
   const client = new httpClient.HttpClient(getUserAgentString())
   const response = await client.get(url)
-
   if (response.message.statusCode !== 200) {
     throw new Error(
       `Unexpected HTTP response from blob storage: ${response.message.statusCode} ${response.message.statusMessage}`
     )
   }
 
+  const timeout = 30 * 1000 // 30 seconds
+
   return new Promise((resolve, reject) => {
+    const timerFn = (): void => {
+      response.message.destroy(
+        new Error(`Blob storage chunk did not respond in ${timeout}ms`)
+      )
+    }
+    const timer = setTimeout(timerFn, timeout)
+
     response.message
+      .on('data', () => {
+        timer.refresh()
+      })
+      .on('error', (error: Error) => {
+        core.debug(
+          `response.message: Artifact download failed: ${error.message}`
+        )
+        clearTimeout(timer)
+        reject(error)
+      })
       .pipe(unzip.Extract({path: directory}))
-      .on('close', resolve)
-      .on('error', reject)
+      .on('close', () => {
+        clearTimeout(timer)
+        resolve()
+      })
+      .on('error', (error: Error) => {
+        reject(error)
+      })
   })
 }
 

packages/artifact/src/internal/shared/artifact-twirp-client.ts

@@ -4,7 +4,7 @@ import {info, debug} from '@actions/core'
 import {ArtifactServiceClientJSON} from '../../generated'
 import {getResultsServiceUrl, getRuntimeToken} from './config'
 import {getUserAgentString} from './user-agent'
-import {NetworkError} from './errors'
+import {NetworkError, UsageError} from './errors'
 
 // The twirp http client must implement this interface
 interface Rpc {
@@ -64,7 +64,7 @@ class ArtifactHttpClient implements Rpc {
         this.httpClient.post(url, JSON.stringify(data), headers)
       )
 
-      return JSON.parse(body)
+      return body
     } catch (error) {
       throw new Error(`Failed to ${method}: ${error.message}`)
     }
@@ -72,34 +72,49 @@ class ArtifactHttpClient implements Rpc {
 
   async retryableRequest(
     operation: () => Promise<HttpClientResponse>
-  ): Promise<{response: HttpClientResponse; body: string}> {
+  ): Promise<{response: HttpClientResponse; body: object}> {
     let attempt = 0
     let errorMessage = ''
+    let rawBody = ''
     while (attempt < this.maxAttempts) {
       let isRetryable = false
 
       try {
         const response = await operation()
         const statusCode = response.message.statusCode
-        const body = await response.readBody()
+        rawBody = await response.readBody()
         debug(`[Response] - ${response.message.statusCode}`)
         debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`)
-        debug(`Body: ${body}`)
+        const body = JSON.parse(rawBody)
+        debug(`Body: ${JSON.stringify(body, null, 2)}`)
         if (this.isSuccessStatusCode(statusCode)) {
           return {response, body}
         }
         isRetryable = this.isRetryableHttpStatusCode(statusCode)
         errorMessage = `Failed request: (${statusCode}) ${response.message.statusMessage}`
-        const responseMessage = JSON.parse(body).msg
-        if (responseMessage) {
-          errorMessage = `${errorMessage}: ${responseMessage}`
+        if (body.msg) {
+          if (UsageError.isUsageErrorMessage(body.msg)) {
+            throw new UsageError()
+          }
+
+          errorMessage = `${errorMessage}: ${body.msg}`
         }
       } catch (error) {
-        isRetryable = true
-        errorMessage = error.message
+        if (error instanceof SyntaxError) {
+          debug(`Raw Body: ${rawBody}`)
+          throw error
+        }
+
+        if (error instanceof UsageError) {
+          throw error
+        }
+
         if (NetworkError.isNetworkErrorCode(error?.code)) {
           throw new NetworkError(error?.code)
         }
+
+        isRetryable = true
+        errorMessage = error.message
       }
 
       if (!isRetryable) {

packages/artifact/src/internal/shared/errors.ts

@@ -57,3 +57,16 @@ export class NetworkError extends Error {
     ].includes(code)
   }
 }
+
+export class UsageError extends Error {
+  constructor() {
+    const message = `Artifact storage quota has been hit. Unable to upload any new artifacts. Usage is recalculated every 6-12 hours.\nMore info on storage limits: https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#calculating-minute-and-storage-spending`
+    super(message)
+    this.name = 'UsageError'
+  }
+
+  static isUsageErrorMessage = (msg?: string): boolean => {
+    if (!msg) return false
+    return msg.includes('insufficient usage')
+  }
+}