Added custom retry codes configuration options (#1569)

Extended the `RequestOptions` interface with `retryCodes` and `noRetryCodes` options, added the implementation for that in `index.js` by creating the function `_shouldRetryOnCode`, which has the hierarchy `retryCodes` > `noRetryCodes` > `DefaultHttpResponseRetryCodes`. Also fixed the failing unit tests by updating the imports (can be reverted if that was not needed, but in my case it was) and wrote new unit tests for the retry mechanism. Wrote the unit tests by adding the variable `retryCount` to `HttpClientResponse` to be able to track the amount of retries done. This is also very usefull for logging in production, as there is no current method for that.

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

@@ -1,5 +1,5 @@
-import * as httpm from '../lib'
-import * as am from '../lib/auth'
+import * as httpm from '../src/index'
+import * as am from '../src/auth'
 
 describe('auth', () => {
   beforeEach(() => {})

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

@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
-import * as httpm from '..'
+import * as httpm from '../src/index'
 import * as path from 'path'
 import * as fs from 'fs'
 

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

@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
-import * as httpm from '..'
+import * as httpm from '../src/index'
 
 describe('headers', () => {
   let _http: httpm.HttpClient

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

@@ -1,4 +1,4 @@
-import * as httpm from '../lib'
+import * as httpm from '../src/index'
 
 describe('basics', () => {
   let _http: httpm.HttpClient

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

@@ -1,8 +1,8 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
 import * as http from 'http'
-import * as httpm from '../lib/'
-import * as pm from '../lib/proxy'
+import * as httpm from '../src/index'
+import * as pm from '../src/proxy'
 import {ProxyAgent} from 'undici'
 // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports
 const proxy = require('proxy')

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

@@ -0,0 +1,98 @@
+import * as httpm from '../src/index'
+
+describe('basics', () => {
+  let _http: httpm.HttpClient
+
+  beforeEach(() => {
+    _http = new httpm.HttpClient('http-client-tests', undefined, {
+      allowRetries: true,
+      maxRetries: 5,
+      retryCodes: [404, 500, 502],
+      noRetryCodes: [403, 404, 504]
+    })
+  })
+
+  afterEach(() => {})
+
+  it('constructs', () => {
+    const http: httpm.HttpClient = new httpm.HttpClient('thttp-client-tests')
+    expect(http).toBeDefined()
+  })
+
+  it('no retry on error code 400 (not given in options)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/400'
+      )}&status_code=400`
+    )
+
+    expect(res.retryCount).toBe(undefined)
+    expect(res.message.statusCode).toBe(400)
+  })
+
+  it('no retry on error code 403 (noRetryOnCodes)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/403'
+      )}&status_code=403`
+    )
+
+    expect(res.retryCount).toBe(undefined)
+    expect(res.message.statusCode).toBe(403)
+  })
+
+  it('retry on error code 404 (retryOnCodes used over noRetryOnCode)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/404'
+      )}&status_code=404`
+    )
+
+    expect(res.retryCount).toBe(5)
+    expect(res.message.statusCode).toBe(404)
+  })
+
+  it('retry on error code 500 (retryOnCodes only)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/500'
+      )}&status_code=500`
+    )
+
+    expect(res.retryCount).toBe(5)
+    expect(res.message.statusCode).toBe(500)
+  })
+
+  it('retry on error code 502 (retryOnCodes and HttpResponseRetryCodes)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/502'
+      )}&status_code=502`
+    )
+
+    expect(res.retryCount).toBe(5)
+    expect(res.message.statusCode).toBe(502)
+  })
+
+  it('retry on error code 503 (HttpResponseRetryCodes only)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/503'
+      )}&status_code=503`
+    )
+
+    expect(res.retryCount).toBe(5)
+    expect(res.message.statusCode).toBe(503)
+  })
+
+  it('no retry on error code 504 (noRetryOnCodes used over HttpResponseRetryCodes)', async () => {
+    const res: httpm.HttpClientResponse = await _http.get(
+      `https://postman-echo.com/redirect-to?url=${encodeURIComponent(
+        'https://postman-echo.com/status/504'
+      )}&status_code=504`
+    )
+
+    expect(res.retryCount).toBe(undefined)
+    expect(res.message.statusCode).toBe(504)
+  })
+})

packages/http-client/src/index.ts

@@ -63,7 +63,7 @@ const HttpRedirectCodes: number[] = [
   HttpCodes.TemporaryRedirect,
   HttpCodes.PermanentRedirect
 ]
-const HttpResponseRetryCodes: number[] = [
+const DefaultHttpResponseRetryCodes: number[] = [
   HttpCodes.BadGateway,
   HttpCodes.ServiceUnavailable,
   HttpCodes.GatewayTimeout
@@ -90,6 +90,8 @@ export class HttpClientResponse {
   }
 
   message: http.IncomingMessage
+  retryCount: number | undefined
+
   async readBody(): Promise<string> {
     return new Promise<string>(async resolve => {
       let output = Buffer.alloc(0)
@@ -141,6 +143,8 @@ export class HttpClient {
   private _proxyAgentDispatcher: any
   private _keepAlive = false
   private _disposed = false
+  private _retryOnCodes: number[] = []
+  private _noRetryOnCodes: number[] = []
 
   constructor(
     userAgent?: string,
@@ -180,6 +184,14 @@ export class HttpClient {
       if (requestOptions.maxRetries != null) {
         this._maxRetries = requestOptions.maxRetries
       }
+
+      if (requestOptions.retryCodes != null) {
+        this._retryOnCodes = requestOptions.retryCodes
+      }
+
+      if (requestOptions.noRetryCodes != null) {
+        this._noRetryOnCodes = requestOptions.noRetryCodes
+      }
     }
   }
 
@@ -435,9 +447,10 @@ export class HttpClient {
 
       if (
         !response.message.statusCode ||
-        !HttpResponseRetryCodes.includes(response.message.statusCode)
+        !this._shouldRetryOnCode(response.message.statusCode)
       ) {
         // If not a retry code, return immediately instead of retrying
+        response.retryCount = numTries - 1
         return response
       }
 
@@ -449,6 +462,7 @@ export class HttpClient {
       }
     } while (numTries < maxTries)
 
+    response.retryCount = numTries - 1
     return response
   }
 
@@ -828,6 +842,19 @@ export class HttpClient {
       }
     })
   }
+
+  private _shouldRetryOnCode(statusCode: number): boolean {
+    if (this._retryOnCodes.includes(statusCode)) {
+      return true
+    } else if (
+      DefaultHttpResponseRetryCodes.includes(statusCode) &&
+      !this._noRetryOnCodes.includes(statusCode)
+    ) {
+      return true
+    } else {
+      return false
+    }
+  }
 }
 
 const lowercaseKeys = (obj: {[index: string]: any}): any =>

packages/http-client/src/interfaces.ts

@@ -82,6 +82,8 @@ export interface RequestOptions {
   // Allows retries only on Read operations (since writes may not be idempotent)
   allowRetries?: boolean
   maxRetries?: number
+  retryCodes?: number[]
+  noRetryCodes?: number[]
 }
 
 export interface TypedResponse<T> {