From 90174e55018209c2eb0500fbb891db0e59c44b71 Mon Sep 17 00:00:00 2001 From: Stanislav Mishchyshyn Date: Thu, 29 Aug 2024 17:22:42 +0300 Subject: [PATCH 1/2] fix(registry/worker): add exponential backoff to asset discovery http requests --- .../common/services/assets/AssetsDiscovery.ts | 11 ++- registry/server/util/axiosExponentialRetry.ts | 34 +++++++ .../services/axiosExponentialBackoff.spec.ts | 92 +++++++++++++++++++ 3 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 registry/server/util/axiosExponentialRetry.ts create mode 100644 registry/tests/services/axiosExponentialBackoff.spec.ts diff --git a/registry/server/common/services/assets/AssetsDiscovery.ts b/registry/server/common/services/assets/AssetsDiscovery.ts index 3ce5b461..66c018bc 100644 --- a/registry/server/common/services/assets/AssetsDiscovery.ts +++ b/registry/server/common/services/assets/AssetsDiscovery.ts @@ -8,6 +8,7 @@ import AssetsDiscoveryWhiteLists from './AssetsDiscoveryWhiteLists'; import { getLogger } from '../../../util/logger'; import { parseJSON } from '../json'; import { axiosErrorTransformer } from '../../../util/axiosErrorTransformer'; +import { exponentialRetry } from '../../../util/axiosExponentialRetry'; import newrelic from 'newrelic'; type AssetsDiscoveryEntity = { @@ -77,10 +78,12 @@ export default class AssetsDiscovery { const startOfRequest = performance.now(); let reqUrl = this.buildAssetsUrl(entity); - const res: Readonly = await axios.get(reqUrl, { - responseType: 'json', - timeout: maxRequestTimeout, - }); + const res: Readonly = await exponentialRetry(() => + axios.get(reqUrl, { + responseType: 'json', + timeout: maxRequestTimeout, + }), + ); const data = manifestProcessor(reqUrl, res.data, AssetsDiscoveryWhiteLists[this.tableName]); const dbAssets = { spaBundle: entity.spaBundle, diff --git a/registry/server/util/axiosExponentialRetry.ts b/registry/server/util/axiosExponentialRetry.ts new file mode 100644 index 00000000..36a8e1da --- /dev/null +++ b/registry/server/util/axiosExponentialRetry.ts @@ -0,0 +1,34 @@ +import axios, { AxiosError } from 'axios'; +import { setTimeout } from 'node:timers/promises'; + +function isRetryableError(error: AxiosError): boolean { + return ( + !error.response || + error.response.status === 429 || + (error.response.status >= 500 && error.response.status <= 599) + ); +} +type ExponentialRetryOptions = { + attempt?: number; + baseDelay?: number; + maxAttempts?: number; + maxDelay?: number; +}; + +export async function exponentialRetry any>( + fn: T, + { attempt = 1, maxAttempts = 5, baseDelay = 100, maxDelay = 10_000 }: ExponentialRetryOptions = {}, +): Promise> { + try { + return await fn(); + } catch (error) { + if (attempt < maxAttempts && axios.isAxiosError(error) && isRetryableError(error)) { + const delay = Math.min(2 ** attempt * baseDelay, maxDelay); + const jitter = Math.round(Math.random() * delay); + await setTimeout(jitter); + return exponentialRetry(fn, { attempt: attempt + 1, maxAttempts, maxDelay, baseDelay }); + } else { + throw error; + } + } +} diff --git a/registry/tests/services/axiosExponentialBackoff.spec.ts b/registry/tests/services/axiosExponentialBackoff.spec.ts new file mode 100644 index 00000000..db536757 --- /dev/null +++ b/registry/tests/services/axiosExponentialBackoff.spec.ts @@ -0,0 +1,92 @@ +import { AxiosError } from 'axios'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { exponentialRetry } from '../../server/util/axiosExponentialRetry'; + +describe('exponentialRetry', () => { + let clock: sinon.SinonFakeTimers; + let fn: sinon.SinonStub; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + fn = sinon.stub(); + }); + + afterEach(() => { + sinon.restore(); + clock.restore(); + }); + + it('should return the result if the function succeeds on the first attempt', async () => { + const result = 'success'; + fn.resolves(result); + + const response = await exponentialRetry(fn); + + expect(response).to.equal(result); + expect(fn.calledOnce).to.be.true; + }); + + it('should retry on a retryable error and eventually succeed', async () => { + const error = new Error() as AxiosError; + error.isAxiosError = true; + error.response = { status: 500 } as any; + fn.onFirstCall().rejects(error); + fn.onSecondCall().resolves('success'); + + const response = await exponentialRetry(fn); + + expect(response).to.equal('success'); + expect(fn.calledTwice).to.be.true; + }); + + it('should retry up to the maxAttempts and then throw the error', async () => { + const error = new Error() as AxiosError; + error.isAxiosError = true; + error.response = { status: 500 } as any; + fn.rejects(error); + + try { + await exponentialRetry(fn, { maxAttempts: 3 }); + expect.fail('Expected to throw an error'); + } catch (err) { + expect(err).to.deep.equal(error); + } + + expect(fn.callCount).to.equal(3); + }); + + it('should not retry on a non-retryable error', async () => { + const error = new Error() as AxiosError; + error.isAxiosError = true; + error.response = { status: 400 } as any; + fn.rejects(error); + + try { + await exponentialRetry(fn); + expect.fail('Expected to throw an error'); + } catch (err) { + expect(err).to.deep.equal(error); + } + + expect(fn.calledOnce).to.be.true; + }); + + it('should apply exponential backoff with jitter', async () => { + const error = new Error() as AxiosError; + error.isAxiosError = true; + error.response = { status: 500 } as any; + fn.onFirstCall().rejects(error); + fn.onSecondCall().resolves('success'); + + const baseDelay = 150; + const promise = exponentialRetry(fn, { baseDelay }); + + await clock.tickAsync(baseDelay); // Wait for the first delay (baseDelay) + + const response = await promise; + + expect(response).to.equal('success'); + expect(fn.calledTwice).to.be.true; + }); +}); From d920579a19de9335396588d6707f64aa62528aa3 Mon Sep 17 00:00:00 2001 From: Stanislav Mishchyshyn Date: Fri, 30 Aug 2024 15:57:09 +0300 Subject: [PATCH 2/2] fix(ci): align commit hashes in ci workflow --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 413fb074..a6585695 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: ref: "" steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v3 with: ref: ${{ matrix.branch.ref }} @@ -100,7 +100,7 @@ jobs: ref: "" steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v3 with: ref: ${{ matrix.branch.ref }} @@ -377,7 +377,7 @@ jobs: runs-on: ubuntu-latest if: github.ref == 'refs/heads/master' steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Build the Docker image run: |