Retry artifact uploads on HTTP 500 (#833)

* Retry on 500 * bump package version * fix tests * Remove spurious change * fix another test * Roll back package version
actions · Jun 4, 2021 · db21627 · db21627
1 parent bb2f393
commit db21627
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 8 deletions.
diff --git a/packages/artifact/__tests__/download.test.ts b/packages/artifact/__tests__/download.test.ts
@@ -71,7 +71,7 @@ describe('Download Tests', () => {
     setupFailedResponse()
     const downloadHttpClient = new DownloadHttpClient()
     expect(downloadHttpClient.listArtifacts()).rejects.toThrow(
-      'List Artifacts failed: Artifact service responded with 500'
+      'List Artifacts failed: Artifact service responded with 400'
     )
   })
 
@@ -113,7 +113,7 @@ describe('Download Tests', () => {
         configVariables.getRuntimeUrl()
       )
     ).rejects.toThrow(
-      `Get Container Items failed: Artifact service responded with 500`
+      `Get Container Items failed: Artifact service responded with 400`
     )
   })
 
@@ -166,7 +166,7 @@ describe('Download Tests', () => {
   it('Test retryable status codes during artifact download', async () => {
     // The first http response should return a retryable status call while the subsequent call should return a 200 so
     // the download should successfully finish
-    const retryableStatusCodes = [429, 502, 503, 504]
+    const retryableStatusCodes = [429, 500, 502, 503, 504]
     for (const statusCode of retryableStatusCodes) {
       const fileContents = Buffer.from('try, try again\n', defaultEncoding)
       const targetPath = path.join(root, `FileC-${statusCode}.txt`)
@@ -468,7 +468,7 @@ describe('Download Tests', () => {
   function setupFailedResponse(): void {
     jest.spyOn(HttpClient.prototype, 'get').mockImplementationOnce(async () => {
       const mockMessage = new http.IncomingMessage(new net.Socket())
-      mockMessage.statusCode = 500
+      mockMessage.statusCode = 400
       return new Promise<HttpClientResponse>(resolve => {
         resolve({
           message: mockMessage,

diff --git a/packages/artifact/__tests__/retry.test.ts b/packages/artifact/__tests__/retry.test.ts
@@ -107,8 +107,8 @@ test('retry fails after exhausting retries', async () => {
 })
 
 test('retry fails after non-retryable status code', async () => {
-  await testRetry([500, 200], {
-    responseCode: 500,
-    errorMessage: 'test failed: Artifact service responded with 500'
+  await testRetry([400, 200], {
+    responseCode: 400,
+    errorMessage: 'test failed: Artifact service responded with 400'
   })
 })
diff --git a/packages/artifact/src/internal/utils.ts b/packages/artifact/src/internal/utils.ts
@@ -72,8 +72,9 @@ export function isRetryableStatusCode(statusCode: number | undefined): boolean {
 
   const retryableStatusCodes = [
     HttpCodes.BadGateway,
-    HttpCodes.ServiceUnavailable,
     HttpCodes.GatewayTimeout,
+    HttpCodes.InternalServerError,
+    HttpCodes.ServiceUnavailable,
     HttpCodes.TooManyRequests,
     413 // Payload Too Large
   ]