Skip to content

Commit

Permalink
fix(registry/worker): add exponential backoff to asset discovery http…
Browse files Browse the repository at this point in the history
… requests
  • Loading branch information
stas-nc committed Aug 29, 2024
1 parent 5d3bf46 commit 90174e5
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 4 deletions.
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 90174e5

Please sign in to comment.