Skip to content

Commit

Permalink
Merge pull request #599 from namecheap/bugfix/assets-disvoery-http-error
Browse files Browse the repository at this point in the history
fix(registry/worker): add exponential backoff to asset discovery http requests
  • Loading branch information
stas-nc authored Aug 30, 2024
2 parents 5d3bf46 + d920579 commit cb5ed7b
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 7 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
ref: ""

steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v3
with:
ref: ${{ matrix.branch.ref }}

Expand Down Expand Up @@ -100,7 +100,7 @@ jobs:
ref: ""

steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v3
with:
ref: ${{ matrix.branch.ref }}

Expand Down Expand Up @@ -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: |
Expand Down
11 changes: 7 additions & 4 deletions registry/server/common/services/assets/AssetsDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -77,10 +78,12 @@ export default class AssetsDiscovery {
const startOfRequest = performance.now();

let reqUrl = this.buildAssetsUrl(entity);
const res: Readonly<AxiosResponse> = await axios.get(reqUrl, {
responseType: 'json',
timeout: maxRequestTimeout,
});
const res: Readonly<AxiosResponse> = await exponentialRetry(() =>
axios.get(reqUrl, {
responseType: 'json',
timeout: maxRequestTimeout,
}),
);
const data = manifestProcessor(reqUrl, res.data, AssetsDiscoveryWhiteLists[this.tableName]);
const dbAssets = {
spaBundle: entity.spaBundle,
Expand Down
34 changes: 34 additions & 0 deletions registry/server/util/axiosExponentialRetry.ts
Original file line number Diff line number Diff line change
@@ -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<T extends () => any>(
fn: T,
{ attempt = 1, maxAttempts = 5, baseDelay = 100, maxDelay = 10_000 }: ExponentialRetryOptions = {},
): Promise<ReturnType<T>> {
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;
}
}
}
92 changes: 92 additions & 0 deletions registry/tests/services/axiosExponentialBackoff.spec.ts
Original file line number Diff line number Diff line change
@@ -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;
});
});

0 comments on commit cb5ed7b

Please sign in to comment.