Add specific messages for network-specific node error codes

[artifact] replace unzipper with unzip-stream

package-lock.json

@@ -9,7 +9,6 @@
         "@types/jest": "^29.5.4",
         "@types/node": "^20.5.7",
         "@types/signale": "^1.4.1",
-        "@types/unzip-stream": "^0.3.4",
         "concurrently": "^6.1.0",
         "eslint": "^8.0.1",
         "eslint-config-prettier": "^8.9.0",
@@ -2588,15 +2587,6 @@
       "integrity": "sha512-Hl219/BT5fLAaz6NDkSuhzasy49dwQS/DSdu4MdggFB8zcXv7vflBI3xp7FEmkmdDkBUI2bPUNeMttp2knYdxw==",
       "dev": true
     },
-    "node_modules/@types/unzip-stream": {
-      "version": "0.3.4",
-      "resolved": "https://registry.npmjs.org/@types/unzip-stream/-/unzip-stream-0.3.4.tgz",
-      "integrity": "sha512-ud0vtsNRF+joUCyvNMyo0j5DKX2Lh/im+xVgRzBEsfHhQYZ+i4fKTveova9XxLzt6Jl6G0e/0mM4aC0gqZYSnA==",
-      "dev": true,
-      "dependencies": {
-        "@types/node": "*"
-      }
-    },
     "node_modules/@types/yargs": {
       "version": "17.0.24",
       "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.24.tgz",

package.json

@@ -19,7 +19,6 @@
     "@types/jest": "^29.5.4",
     "@types/node": "^20.5.7",
     "@types/signale": "^1.4.1",
-    "@types/unzip-stream": "^0.3.4",
     "concurrently": "^6.1.0",
     "eslint": "^8.0.1",
     "eslint-config-prettier": "^8.9.0",
@@ -34,4 +33,4 @@
     "ts-jest": "^29.1.1",
     "typescript": "^5.2.2"
   }
-}
+}

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

@@ -7,6 +7,12 @@ import {noopLogs} from './common'
 
 jest.mock('@actions/http-client')
 
+const clientOptions = {
+  maxAttempts: 5,
+  retryIntervalMs: 1,
+  retryMultiplier: 1.5
+}
+
 describe('artifact-http-client', () => {
   beforeAll(() => {
     noopLogs()
@@ -94,11 +100,7 @@ describe('artifact-http-client', () => {
       }
     })
 
-    const client = internalArtifactTwirpClient({
+    const client = internalArtifactTwirpClient(clientOptions)
-      maxAttempts: 5,
-      retryIntervalMs: 1,
-      retryMultiplier: 1.5
-    })
     const artifact = await client.CreateArtifact({
       workflowRunBackendId: '1234',
       workflowJobRunBackendId: '5678',
@@ -133,11 +135,7 @@ describe('artifact-http-client', () => {
         post: mockPost
       }
     })
-    const client = internalArtifactTwirpClient({
+    const client = internalArtifactTwirpClient(clientOptions)
-      maxAttempts: 5,
-      retryIntervalMs: 1,
-      retryMultiplier: 1.5
-    })
     await expect(async () => {
       await client.CreateArtifact({
         workflowRunBackendId: '1234',
@@ -172,11 +170,7 @@ describe('artifact-http-client', () => {
         post: mockPost
       }
     })
-    const client = internalArtifactTwirpClient({
+    const client = internalArtifactTwirpClient(clientOptions)
-      maxAttempts: 5,
-      retryIntervalMs: 1,
-      retryMultiplier: 1.5
-    })
     await expect(async () => {
       await client.CreateArtifact({
         workflowRunBackendId: '1234',
@@ -190,4 +184,83 @@ describe('artifact-http-client', () => {
     expect(mockHttpClient).toHaveBeenCalledTimes(1)
     expect(mockPost).toHaveBeenCalledTimes(1)
   })
+
+  it('should fail with a descriptive error', async () => {
+    // 409 duplicate error
+    const mockPost = jest.fn(() => {
+      const msgFailed = new http.IncomingMessage(new net.Socket())
+      msgFailed.statusCode = 409
+      msgFailed.statusMessage = 'Conflict'
+      return {
+        message: msgFailed,
+        readBody: async () => {
+          return Promise.resolve(
+            `{"msg": "an artifact with this name already exists on the workflow run"}`
+          )
+        }
+      }
+    })
+
+    const mockHttpClient = (
+      HttpClient as unknown as jest.Mock
+    ).mockImplementation(() => {
+      return {
+        post: mockPost
+      }
+    })
+    const client = internalArtifactTwirpClient(clientOptions)
+    await expect(async () => {
+      await client.CreateArtifact({
+        workflowRunBackendId: '1234',
+        workflowJobRunBackendId: '5678',
+        name: 'artifact',
+        version: 4
+      })
+      await client.CreateArtifact({
+        workflowRunBackendId: '1234',
+        workflowJobRunBackendId: '5678',
+        name: 'artifact',
+        version: 4
+      })
+    }).rejects.toThrowError(
+      'Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run'
+    )
+    expect(mockHttpClient).toHaveBeenCalledTimes(1)
+    expect(mockPost).toHaveBeenCalledTimes(1)
+  })
+
+  it('should properly describe a network failure', async () => {
+    class FakeNodeError extends Error {
+      code: string
+      constructor(code: string) {
+        super()
+        this.code = code
+      }
+    }
+
+    const mockPost = jest.fn(() => {
+      throw new FakeNodeError('ENOTFOUND')
+    })
+
+    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(
+      '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'
+    )
+    expect(mockHttpClient).toHaveBeenCalledTimes(1)
+    expect(mockPost).toHaveBeenCalledTimes(1)
+  })
 })

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

@@ -104,6 +104,7 @@ const mockGetArtifactSuccess = jest.fn(() => {
   const message = new http.IncomingMessage(new net.Socket())
   message.statusCode = 200
   message.push(fs.readFileSync(fixtures.exampleArtifact.path))
+  message.push(null)
   return {
     message
   }
@@ -113,6 +114,7 @@ const mockGetArtifactFailure = jest.fn(() => {
   const message = new http.IncomingMessage(new net.Socket())
   message.statusCode = 500
   message.push('Internal Server Error')
+  message.push(null)
   return {
     message
   }

packages/artifact/package-lock.json

@@ -18,7 +18,6 @@
         "@octokit/plugin-retry": "^3.0.9",
         "@octokit/request-error": "^5.0.0",
         "@protobuf-ts/plugin": "^2.2.3-alpha.1",
-        "@types/unzipper": "^0.10.6",
         "archiver": "^5.3.1",
         "crypto": "^1.0.1",
         "jwt-decode": "^3.1.2",
@@ -27,6 +26,7 @@
       },
       "devDependencies": {
         "@types/archiver": "^5.3.2",
+        "@types/unzip-stream": "^0.3.4",
         "typedoc": "^0.25.4",
         "typedoc-plugin-markdown": "^3.17.1",
         "typescript": "^5.2.2"
@@ -471,10 +471,11 @@
         "@types/node": "*"
       }
     },
-    "node_modules/@types/unzipper": {
+    "node_modules/@types/unzip-stream": {
-      "version": "0.10.6",
+      "version": "0.3.4",
-      "resolved": "https://registry.npmjs.org/@types/unzipper/-/unzipper-0.10.6.tgz",
+      "resolved": "https://registry.npmjs.org/@types/unzip-stream/-/unzip-stream-0.3.4.tgz",
-      "integrity": "sha512-zcBj329AHgKLQyz209N/S9R0GZqXSkUQO4tJSYE3x02qg4JuDFpgKMj50r82Erk1natCWQDIvSccDddt7jPzjA==",
+      "integrity": "sha512-ud0vtsNRF+joUCyvNMyo0j5DKX2Lh/im+xVgRzBEsfHhQYZ+i4fKTveova9XxLzt6Jl6G0e/0mM4aC0gqZYSnA==",
+      "dev": true,
       "dependencies": {
         "@types/node": "*"
       }

packages/artifact/package.json

@@ -49,7 +49,6 @@
     "@octokit/plugin-retry": "^3.0.9",
     "@octokit/request-error": "^5.0.0",
     "@protobuf-ts/plugin": "^2.2.3-alpha.1",
-    "@types/unzipper": "^0.10.6",
     "archiver": "^5.3.1",
     "crypto": "^1.0.1",
     "jwt-decode": "^3.1.2",
@@ -58,6 +57,7 @@
   },
   "devDependencies": {
     "@types/archiver": "^5.3.2",
+    "@types/unzip-stream": "^0.3.4",
     "typedoc": "^0.25.4",
     "typedoc-plugin-markdown": "^3.17.1",
     "typescript": "^5.2.2"

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

@@ -2,7 +2,7 @@ import fs from 'fs/promises'
 import * as github from '@actions/github'
 import * as core from '@actions/core'
 import * as httpClient from '@actions/http-client'
-import unzipper from 'unzip-stream'
+import unzip from 'unzip-stream'
 import {
   DownloadArtifactOptions,
   DownloadArtifactResponse
@@ -47,7 +47,12 @@ async function streamExtract(url: string, directory: string): Promise<void> {
     )
   }
 
-  return response.message.pipe(unzipper.Extract({path: directory})).promise()
+  return new Promise((resolve, reject) => {
+    response.message
+      .pipe(unzip.Extract({path: directory}))
+      .on('close', resolve)
+      .on('error', reject)
+  })
 }
 
 export async function downloadArtifactPublic(

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

@@ -4,6 +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'
 
 // The twirp http client must implement this interface
 interface Rpc {
@@ -84,16 +85,21 @@ class ArtifactHttpClient implements Rpc {
         debug(`[Response] - ${response.message.statusCode}`)
         debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`)
         debug(`Body: ${body}`)
-
         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}`
+        }
       } catch (error) {
         isRetryable = true
         errorMessage = error.message
+        if (NetworkError.isNetworkErrorCode(error?.code)) {
+          throw new NetworkError(error?.code)
+        }
       }
 
       if (!isRetryable) {

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

@@ -35,3 +35,25 @@ export class GHESNotSupportedError extends Error {
     this.name = 'GHESNotSupportedError'
   }
 }
+
+export class NetworkError extends Error {
+  code: string
+
+  constructor(code: string) {
+    const message = `Unable to make request: ${code}\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`
+    super(message)
+    this.code = code
+    this.name = 'NetworkError'
+  }
+
+  static isNetworkErrorCode = (code?: string): boolean => {
+    if (!code) return false
+    return [
+      'ECONNRESET',
+      'ENOTFOUND',
+      'ETIMEDOUT',
+      'ECONNREFUSED',
+      'EHOSTUNREACH'
+    ].includes(code)
+  }
+}

packages/artifact/src/internal/upload/blob-upload.ts

@@ -5,6 +5,7 @@ import {getUploadChunkSize, getConcurrency} from '../shared/config'
 import * as core from '@actions/core'
 import * as crypto from 'crypto'
 import * as stream from 'stream'
+import {NetworkError} from '../shared/errors'
 
 export interface BlobUploadResponse {
   /**
@@ -52,12 +53,20 @@ export async function uploadZipToBlobStorage(
 
   core.info('Beginning upload of artifact content to blob storage')
 
-  await blockBlobClient.uploadStream(
+  try {
-    uploadStream,
+    await blockBlobClient.uploadStream(
-    bufferSize,
+      uploadStream,
-    maxConcurrency,
+      bufferSize,
-    options
+      maxConcurrency,
-  )
+      options
+    )
+  } catch (error) {
+    if (NetworkError.isNetworkErrorCode(error?.code)) {
+      throw new NetworkError(error?.code)
+    }
+
+    throw error
+  }
 
   core.info('Finished uploading artifact content to blob storage!')