Improve error messages (duplicate artifacts; too many artifacts) (act…

…ions#1600) * cleaning up error messages * updating package-json * updating package-lock * . * . * testing return message * updating error check * adding test * rmv unused var * updating status code to match conflict message
coolkiid · Dec 11, 2023 · a205c9d · a205c9d
1 parent 15a688b
commit a205c9d
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 2 deletions.
diff --git a/__tests__/artifact-http-client.test.ts b/__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)
+  })
 })
diff --git a/src/internal/shared/artifact-twirp-client.ts b/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