From 1f62681217dc47d555f633e6a8a4a0c1f183af94 Mon Sep 17 00:00:00 2001 From: stanleyphu Date: Tue, 19 Dec 2023 23:01:06 -0800 Subject: [PATCH 1/2] Add request retries for network/connection and 502 errors --- package-lock.json | 75 ++++++++++++++++++++++++++----- package.json | 5 ++- src/HttpClient.ts | 80 +++++++++++++++++++++++++--------- test/WarrantClientTest.spec.ts | 80 ++++++++++++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 32 deletions(-) create mode 100644 test/WarrantClientTest.spec.ts diff --git a/package-lock.json b/package-lock.json index 8c732cd..09c4515 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,11 +11,12 @@ "devDependencies": { "@types/chai": "^4.3.9", "@types/mocha": "^10.0.3", - "@types/node": "^18.11.18", + "@types/node": "^18.13.0", "chai": "^4.3.10", "mocha": "^10.2.0", "ts-node": "^10.9.1", - "typescript": "^4.3.2" + "typescript": "^4.3.2", + "undici": "^6.0.1" }, "engines": { "node": ">=18.13.0" @@ -33,6 +34,15 @@ "node": ">=12" } }, + "node_modules/@fastify/busboy": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@fastify/busboy/-/busboy-2.1.0.tgz", + "integrity": "sha512-+KpH+QxZU7O4675t3mnkQKcZZg56u+K/Ct2K+N2AZYNVK8kyeo/bI18tI8aPm3tvNNRyTWfj6s5tnGNlcbQRsA==", + "dev": true, + "engines": { + "node": ">=14" + } + }, "node_modules/@jridgewell/resolve-uri": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/@jridgewell/resolve-uri/-/resolve-uri-3.1.1.tgz", @@ -95,10 +105,13 @@ "dev": true }, "node_modules/@types/node": { - "version": "18.11.18", - "resolved": "https://registry.npmjs.org/@types/node/-/node-18.11.18.tgz", - "integrity": "sha512-DHQpWGjyQKSHj3ebjFI/wRKcqQcdR+MoFBygntYOZytCqNfkd2ZC4ARDJ2DQqhjH5p85Nnd3jhUJIXrszFX/JA==", - "dev": true + "version": "18.19.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.19.3.tgz", + "integrity": "sha512-k5fggr14DwAytoA/t8rPrIz++lXK7/DqckthCmoZOKNsEbJkId4Z//BqgApXBUGrGddrigYa1oqheo/7YmW4rg==", + "dev": true, + "dependencies": { + "undici-types": "~5.26.4" + } }, "node_modules/acorn": { "version": "8.11.2", @@ -1099,6 +1112,24 @@ "node": ">=4.2.0" } }, + "node_modules/undici": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.0.1.tgz", + "integrity": "sha512-eZFYQLeS9BiXpsU0cuFhCwfeda2MnC48EVmmOz/eCjsTgmyTdaHdVsPSC/kwC2GtW2e0uH0HIPbadf3/bRWSxw==", + "dev": true, + "dependencies": { + "@fastify/busboy": "^2.0.0" + }, + "engines": { + "node": ">=18.0" + } + }, + "node_modules/undici-types": { + "version": "5.26.5", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-5.26.5.tgz", + "integrity": "sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA==", + "dev": true + }, "node_modules/v8-compile-cache-lib": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/v8-compile-cache-lib/-/v8-compile-cache-lib-3.0.1.tgz", @@ -1217,6 +1248,12 @@ "@jridgewell/trace-mapping": "0.3.9" } }, + "@fastify/busboy": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@fastify/busboy/-/busboy-2.1.0.tgz", + "integrity": "sha512-+KpH+QxZU7O4675t3mnkQKcZZg56u+K/Ct2K+N2AZYNVK8kyeo/bI18tI8aPm3tvNNRyTWfj6s5tnGNlcbQRsA==", + "dev": true + }, "@jridgewell/resolve-uri": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/@jridgewell/resolve-uri/-/resolve-uri-3.1.1.tgz", @@ -1276,10 +1313,13 @@ "dev": true }, "@types/node": { - "version": "18.11.18", - "resolved": "https://registry.npmjs.org/@types/node/-/node-18.11.18.tgz", - "integrity": "sha512-DHQpWGjyQKSHj3ebjFI/wRKcqQcdR+MoFBygntYOZytCqNfkd2ZC4ARDJ2DQqhjH5p85Nnd3jhUJIXrszFX/JA==", - "dev": true + "version": "18.19.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.19.3.tgz", + "integrity": "sha512-k5fggr14DwAytoA/t8rPrIz++lXK7/DqckthCmoZOKNsEbJkId4Z//BqgApXBUGrGddrigYa1oqheo/7YmW4rg==", + "dev": true, + "requires": { + "undici-types": "~5.26.4" + } }, "acorn": { "version": "8.11.2", @@ -1983,6 +2023,21 @@ "integrity": "sha512-uauPG7XZn9F/mo+7MrsRjyvbxFpzemRjKEZXS4AK83oP2KKOJPvb+9cO/gmnv8arWZvhnjVOXz7B49m1l0e9Ew==", "dev": true }, + "undici": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.0.1.tgz", + "integrity": "sha512-eZFYQLeS9BiXpsU0cuFhCwfeda2MnC48EVmmOz/eCjsTgmyTdaHdVsPSC/kwC2GtW2e0uH0HIPbadf3/bRWSxw==", + "dev": true, + "requires": { + "@fastify/busboy": "^2.0.0" + } + }, + "undici-types": { + "version": "5.26.5", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-5.26.5.tgz", + "integrity": "sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA==", + "dev": true + }, "v8-compile-cache-lib": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/v8-compile-cache-lib/-/v8-compile-cache-lib-3.0.1.tgz", diff --git a/package.json b/package.json index 9956006..30f6c2d 100644 --- a/package.json +++ b/package.json @@ -37,10 +37,11 @@ "devDependencies": { "@types/chai": "^4.3.9", "@types/mocha": "^10.0.3", - "@types/node": "^18.11.18", + "@types/node": "^18.13.0", "chai": "^4.3.10", "mocha": "^10.2.0", "ts-node": "^10.9.1", - "typescript": "^4.3.2" + "typescript": "^4.3.2", + "undici": "^6.0.1" } } diff --git a/src/HttpClient.ts b/src/HttpClient.ts index 4609d38..c4702a5 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -34,6 +34,12 @@ interface FetchRequestOptions { body?: string; } +const MAX_RETRY_ATTEMPTS = 3; +const BACKOFF_MULTIPLIER = 1.5; +const MINIMUM_SLEEP_TIME = 500; + +const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + export default class ApiClient implements HttpClient { private config: HttpClientConfig; @@ -44,11 +50,7 @@ export default class ApiClient implements HttpClient { public async get(requestOptions: HttpClientRequestOptions): Promise { const [requestUrl, fetchRequestOptions] = this.buildRequestUrlAndOptions("GET", requestOptions); - /* @ts-ignore */ - const response = await fetch(requestUrl, fetchRequestOptions); - if (!response.ok) { - throw this.buildError(await response.json()); - } + const response = await this.fetchWithRetry(requestUrl, fetchRequestOptions); return this.parseResponse(response); } @@ -56,11 +58,7 @@ export default class ApiClient implements HttpClient { public async delete(requestOptions: HttpClientRequestOptions): Promise { const [requestUrl, fetchRequestOptions] = this.buildRequestUrlAndOptions("DELETE", requestOptions); - /* @ts-ignore */ - const response = await fetch(requestUrl, fetchRequestOptions); - if (!response.ok) { - throw this.buildError(await response.json()); - } + const response = await this.fetchWithRetry(requestUrl, fetchRequestOptions); return this.parseResponse(response); } @@ -68,11 +66,7 @@ export default class ApiClient implements HttpClient { public async post(requestOptions: HttpClientRequestOptions): Promise { const [requestUrl, fetchRequestOptions] = this.buildRequestUrlAndOptions("POST", requestOptions); - /* @ts-ignore */ - const response = await fetch(requestUrl, fetchRequestOptions); - if (!response.ok) { - throw this.buildError(await response.json()); - } + const response = await this.fetchWithRetry(requestUrl, fetchRequestOptions); return this.parseResponse(response); } @@ -80,15 +74,61 @@ export default class ApiClient implements HttpClient { public async put(requestOptions: HttpClientRequestOptions): Promise { const [requestUrl, fetchRequestOptions] = this.buildRequestUrlAndOptions("PUT", requestOptions); - /* @ts-ignore */ - const response = await fetch(requestUrl, fetchRequestOptions); - if (!response.ok) { - throw this.buildError(await response.json()); - } + const response = await this.fetchWithRetry(requestUrl, fetchRequestOptions); return this.parseResponse(response); } + private async fetchWithRetry(requestUrl: string, fetchRequestOptions: FetchRequestOptions): Promise { + let response: any = null; + let requestError: any = null; + let retryAttempts = 1; + + const makeRequest = async (): Promise => { + try { + response = await fetch(requestUrl, fetchRequestOptions); + } catch (e) { + requestError = e; + } + + if (this.shouldRetryRequest(response, requestError, retryAttempts)) { + retryAttempts++; + await sleep(this.getSleepTime(retryAttempts)); + return makeRequest(); + } + + if (!response.ok) { + throw this.buildError(await response.json()); + } + + return response; + } + + return makeRequest(); + } + + private shouldRetryRequest(response: any, requestError: any, retryAttempt: number): boolean { + if (retryAttempt > MAX_RETRY_ATTEMPTS) { + return false; + } + + if (requestError != null && requestError instanceof TypeError) { + return true; + } + + if (response?.status == 502) { + return true; + } + + return false; + } + + private getSleepTime(retryAttempt: number): number { + let sleepTime = MINIMUM_SLEEP_TIME * Math.pow(BACKOFF_MULTIPLIER, retryAttempt); + const jitter = Math.random() + 0.5; + return sleepTime * jitter; + } + private buildRequestUrlAndOptions(method: FetchRequestOptions["method"], requestOptions?: HttpClientRequestOptions): [string, FetchRequestOptions] { let baseUrl = this.config.baseUrl; const fetchRequestOptions: FetchRequestOptions = { diff --git a/test/WarrantClientTest.spec.ts b/test/WarrantClientTest.spec.ts new file mode 100644 index 0000000..59340df --- /dev/null +++ b/test/WarrantClientTest.spec.ts @@ -0,0 +1,80 @@ +import WarrantClient from "../src/WarrantClient"; +import { ApiError } from "../src/types"; +import { assert } from "chai"; +import { MockAgent, setGlobalDispatcher } from "undici"; + +describe('WarrantClientTest', function () { + before(function () { + this.warrant = new WarrantClient({ apiKey: "my_api_key", endpoint: "http://localhost:8000" }); + + const agent = new MockAgent(); + agent.disableNetConnect(); + this.client = agent.get("http://localhost:8000") + setGlobalDispatcher(agent); + }); + + it('should make request after retries', async function () { + this.timeout(10000); + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(502); + + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(502); + + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(200, { "objectType": "user", "objectId": "some-user" }); + + const fetchedUser = await this.warrant.User.get("some-user"); + + assert.strictEqual(fetchedUser.userId, "some-user"); + assert.strictEqual(fetchedUser.meta, undefined); + }); + + it('should stop requests after max retries', async function () { + this.timeout(10000); + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(502, { + message: "Bad Gateway" + }); + + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(502, { + message: "Bad Gateway" + }); + + this.client + .intercept({ + path: "/v2/objects/user/some-user", + method: "GET" + }) + .reply(502, { + message: "Bad Gateway" + }); + + try { + await this.warrant.User.get("some-user"); + } catch (e) { + assert.instanceOf(e, ApiError); + } + }); +}); From c86bdb888a08a4b6f6575da8b6278ccda58e1e9d Mon Sep 17 00:00:00 2001 From: stanleyphu Date: Wed, 20 Dec 2023 11:12:57 -0800 Subject: [PATCH 2/2] Retry requests on more 50X status codes --- src/HttpClient.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/HttpClient.ts b/src/HttpClient.ts index c4702a5..b572490 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -37,6 +37,7 @@ interface FetchRequestOptions { const MAX_RETRY_ATTEMPTS = 3; const BACKOFF_MULTIPLIER = 1.5; const MINIMUM_SLEEP_TIME = 500; +const RETRY_STATUS_CODES = [500, 502, 504]; const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) @@ -116,7 +117,7 @@ export default class ApiClient implements HttpClient { return true; } - if (response?.status == 502) { + if (response != null && RETRY_STATUS_CODES.includes(response.status)) { return true; }