From 8aba1795235f161be062b2eaf11bc190c40edcfb Mon Sep 17 00:00:00 2001 From: janssen <118828444+janssen-tiobe@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:00:56 +0200 Subject: [PATCH] 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 | 4 +- packages/http-client/__tests__/basics.test.ts | 2 +- .../http-client/__tests__/headers.test.ts | 2 +- .../http-client/__tests__/keepalive.test.ts | 2 +- packages/http-client/__tests__/proxy.test.ts | 4 +- packages/http-client/__tests__/retry.test.ts | 98 +++++++++++++++++++ packages/http-client/src/index.ts | 31 +++++- packages/http-client/src/interfaces.ts | 2 + 8 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 packages/http-client/__tests__/retry.test.ts diff --git a/packages/http-client/__tests__/auth.test.ts b/packages/http-client/__tests__/auth.test.ts index 50d0a6a4..75c5316c 100644 --- a/packages/http-client/__tests__/auth.test.ts +++ b/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(() => {}) diff --git a/packages/http-client/__tests__/basics.test.ts b/packages/http-client/__tests__/basics.test.ts index 1e715ce9..e682a412 100644 --- a/packages/http-client/__tests__/basics.test.ts +++ b/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' diff --git a/packages/http-client/__tests__/headers.test.ts b/packages/http-client/__tests__/headers.test.ts index c1ca0ec3..6c9f34d7 100644 --- a/packages/http-client/__tests__/headers.test.ts +++ b/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 diff --git a/packages/http-client/__tests__/keepalive.test.ts b/packages/http-client/__tests__/keepalive.test.ts index 1faff5ff..98180cdf 100644 --- a/packages/http-client/__tests__/keepalive.test.ts +++ b/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 diff --git a/packages/http-client/__tests__/proxy.test.ts b/packages/http-client/__tests__/proxy.test.ts index c921b4bc..2d1dda23 100644 --- a/packages/http-client/__tests__/proxy.test.ts +++ b/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') diff --git a/packages/http-client/__tests__/retry.test.ts b/packages/http-client/__tests__/retry.test.ts new file mode 100644 index 00000000..2fd1f1c0 --- /dev/null +++ b/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) + }) +}) diff --git a/packages/http-client/src/index.ts b/packages/http-client/src/index.ts index f1170ea0..cd82c76d 100644 --- a/packages/http-client/src/index.ts +++ b/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 { return new Promise(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 => diff --git a/packages/http-client/src/interfaces.ts b/packages/http-client/src/interfaces.ts index 96b0fec7..03a2db4a 100644 --- a/packages/http-client/src/interfaces.ts +++ b/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 {