Repository: actions/toolkit
Pull request #1600 (2023-12-08 19:12:45)

Improve error messages (duplicate artifacts; too many artifacts)
Adds a few catches on vague errors:
- 409s that are tied to Conflict errors (aka artifact cannot be uploaded due to duplicate name error) now displays a more user friendly message
- 400s that are tied to InvalidArgument errors (aka too many artifacts for 1 job) now displays a more user friendly message



Patch:
diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts
index f5f7277..4feba62 100644
--- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts
+++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts
@@ -84,13 +84,15 @@ 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


Test patch:
diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts
index 2dd6c5c..3c6c378 100644
--- a/packages/artifact/__tests__/artifact-http-client.test.ts
+++ b/packages/artifact/__tests__/artifact-http-client.test.ts
@@ -190,4 +190,52 @@ 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({
+      maxAttempts: 5,
+      retryIntervalMs: 1,
+      retryMultiplier: 1.5
+    })
+    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)
+  })
 })