From ccb1df45d159adde17d61acb0300bc025626372a Mon Sep 17 00:00:00 2001
From: Rob Herley <robherley@github.com>
Date: Fri, 19 Apr 2024 14:03:47 +0000
Subject: [PATCH] artifact client: retry on non-JSON response

---
 .../__tests__/artifact-http-client.test.ts    | 48 +++++++++++++++++++
 .../internal/shared/artifact-twirp-client.ts  |  1 -
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts
index e6768346..feb2b04a 100644
--- a/packages/artifact/__tests__/artifact-http-client.test.ts
+++ b/packages/artifact/__tests__/artifact-http-client.test.ts
@@ -116,6 +116,54 @@ describe('artifact-http-client', () => {
     expect(mockPost).toHaveBeenCalledTimes(2)
   })
 
+  it('should retry if invalid body response', async () => {
+    const mockPost = jest
+      .fn(() => {
+        const msgSucceeded = new http.IncomingMessage(new net.Socket())
+        msgSucceeded.statusCode = 200
+        return {
+          message: msgSucceeded,
+          readBody: async () => {
+            return Promise.resolve(
+              `{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`
+            )
+          }
+        }
+      })
+      .mockImplementationOnce(() => {
+        const msgFailed = new http.IncomingMessage(new net.Socket())
+        msgFailed.statusCode = 502
+        msgFailed.statusMessage = 'Bad Gateway'
+        return {
+          message: msgFailed,
+          readBody: async () => {
+            return Promise.resolve('💥')
+          }
+        }
+      })
+    const mockHttpClient = (
+      HttpClient as unknown as jest.Mock
+    ).mockImplementation(() => {
+      return {
+        post: mockPost
+      }
+    })
+
+    const client = internalArtifactTwirpClient(clientOptions)
+    const artifact = await client.CreateArtifact({
+      workflowRunBackendId: '1234',
+      workflowJobRunBackendId: '5678',
+      name: 'artifact',
+      version: 4
+    })
+
+    expect(mockHttpClient).toHaveBeenCalledTimes(1)
+    expect(artifact).toBeDefined()
+    expect(artifact.ok).toBe(true)
+    expect(artifact.signedUploadUrl).toBe('http://localhost:8080/upload')
+    expect(mockPost).toHaveBeenCalledTimes(2)
+  })
+
   it('should fail if the request fails 5 times', async () => {
     const mockPost = jest.fn(() => {
       const msgFailed = new http.IncomingMessage(new net.Socket())
diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts
index 1f987a1a..00c65bc7 100644
--- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts
+++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts
@@ -102,7 +102,6 @@ class ArtifactHttpClient implements Rpc {
       } catch (error) {
         if (error instanceof SyntaxError) {
           debug(`Raw Body: ${rawBody}`)
-          throw error
         }
 
         if (error instanceof UsageError) {