Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support retry-after header #142

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"@babel/cli": "^7.4.4",
"@babel/preset-env": "^7.4.5",
"@types/mocha": "^8.0.0",
"@types/sinon": "^9.0.10",
"@types/node": "^14.0.0",
"axios": "^0.21.0",
"babel-plugin-transform-es2015-modules-umd": "^6.24.1",
Expand All @@ -51,6 +52,7 @@
"mocha": "^8.0.0",
"nock": "^13.0.0",
"semantic-release": "^17.0.4",
"sinon": "^9.2.4",
"typescript": "~4.3.0"
},
"files": [
Expand Down
70 changes: 59 additions & 11 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ export interface RetryConfig {
* Backoff Type; 'linear', 'static' or 'exponential'.
*/
backoffType?: 'linear' | 'static' | 'exponential';

/**
* Whether to check for 'Retry-After' header in response and use value as delay. Defaults to true.
*/
checkRetryAfter?: boolean;

/**
* Max permitted Retry-After value (in ms) - rejects if greater. Defaults to 5 mins.
*/
maxRetryAfter?: number;
}

export type RaxConfig = {
Expand Down Expand Up @@ -124,6 +134,26 @@ function normalizeArray<T>(obj?: T[]): T[] | undefined {
return arr;
}

/**
* Parse the Retry-After header.
* https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
* @param header Retry-After header value
* @returns Number of milliseconds, or undefined if invalid
*/
function parseRetryAfter(header: string): number | undefined {
// Header value may be string containing integer seconds
const value = Number(header);
if (!Number.isNaN(value)) {
return value * 1000;
}
// Or HTTP date time string
const dateTime = Date.parse(header);
if (!Number.isNaN(dateTime)) {
return dateTime - Date.now();
}
return undefined;
}

function onError(err: AxiosError) {
if (axios.isCancel(err)) {
return Promise.reject(err);
Expand All @@ -145,6 +175,10 @@ function onError(err: AxiosError) {
];
config.noResponseRetries =
typeof config.noResponseRetries === 'number' ? config.noResponseRetries : 2;
config.checkRetryAfter =
typeof config.checkRetryAfter === 'boolean' ? config.checkRetryAfter : true;
config.maxRetryAfter =
typeof config.maxRetryAfter === 'number' ? config.maxRetryAfter : 60000 * 5;

// If this wasn't in the list of status codes where we want
// to automatically retry, return.
Expand Down Expand Up @@ -174,18 +208,32 @@ function onError(err: AxiosError) {
}

// Create a promise that invokes the retry after the backOffDelay
const onBackoffPromise = new Promise(resolve => {
// Calculate time to wait with exponential backoff.
// Formula: (2^c - 1 / 2) * 1000
let delay: number;
if (config.backoffType === 'linear') {
delay = config.currentRetryAttempt! * 1000;
} else if (config.backoffType === 'static') {
delay = config.retryDelay!;
} else {
delay = ((Math.pow(2, config.currentRetryAttempt!) - 1) / 2) * 1000;
const onBackoffPromise = new Promise((resolve, reject) => {
let delay = 0;
// If enabled, check for 'Retry-After' header in response to use as delay
if (
config.checkRetryAfter &&
err.response &&
err.response.headers['retry-after']
) {
const retryAfter = parseRetryAfter(err.response.headers['retry-after']);
if (retryAfter && retryAfter > 0 && retryAfter <= config.maxRetryAfter!) {
delay = retryAfter;
} else {
return reject(err);
}
}
// Else calculate delay according to chosen strategy
// Default to exponential backoff - formula: (2^c - 1 / 2) * 1000
else {
if (config.backoffType === 'linear') {
delay = config.currentRetryAttempt! * 1000;
} else if (config.backoffType === 'static') {
delay = config.retryDelay!;
} else {
delay = ((Math.pow(2, config.currentRetryAttempt!) - 1) / 2) * 1000;
}
}

// We're going to retry! Incremenent the counter.
(err.config as RaxConfig).raxConfig!.currentRetryAttempt! += 1;
setTimeout(resolve, delay);
Expand Down
86 changes: 84 additions & 2 deletions test/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as assert from 'assert';
import axios, {AxiosRequestConfig} from 'axios';
import * as nock from 'nock';
import * as sinon from 'sinon';
import {describe, it, afterEach} from 'mocha';
import * as rax from '../src';
import {RaxConfig} from '../src';
Expand All @@ -12,6 +13,7 @@ nock.disableNetConnect();
describe('retry-axios', () => {
let interceptorId: number | undefined;
afterEach(() => {
sinon.restore();
nock.cleanAll();
if (interceptorId !== undefined) {
rax.detach(interceptorId);
Expand All @@ -32,6 +34,8 @@ describe('retry-axios', () => {
assert.strictEqual(config!.retryDelay, 100, 'retryDelay');
assert.strictEqual(config!.instance, axios, 'axios');
assert.strictEqual(config!.backoffType, 'exponential', 'backoffType');
assert.strictEqual(config!.checkRetryAfter, true);
assert.strictEqual(config!.maxRetryAfter, 60000 * 5);
const expectedMethods = ['GET', 'HEAD', 'PUT', 'OPTIONS', 'DELETE'];
for (const method of config!.httpMethodsToRetry!) {
assert(expectedMethods.indexOf(method) > -1, 'exected method: $method');
Expand Down Expand Up @@ -78,7 +82,8 @@ describe('retry-axios', () => {
assert.fail('Expected to throw');
});

it('should retry at least the configured number of times', async () => {
it('should retry at least the configured number of times', async function () {
this.timeout(10000);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to change the timeouts to the old function style after getting a weird error on the ci tests - https://github.com/JustinBeckwith/retry-axios/pull/142/checks?sha=f6f6c8d1bbe8b2a6da5301ee457fd6fe0970e121

const scopes = [
nock(url).get('/').times(3).reply(500),
nock(url).get('/').reply(200, 'milk'),
Expand All @@ -88,7 +93,7 @@ describe('retry-axios', () => {
const res = await axios(cfg);
assert.strictEqual(res.data, 'milk');
scopes.forEach(s => s.done());
}).timeout(10000);
});

it('should not retry more than configured', async () => {
const scope = nock(url).get('/').twice().reply(500);
Expand Down Expand Up @@ -396,4 +401,81 @@ describe('retry-axios', () => {
}
assert.fail('Expected to throw');
});

it('should retry with Retry-After header in seconds', async function () {
this.timeout(1000); // Short timeout to trip test if delay longer than expected
const scopes = [
nock(url).get('/').reply(429, undefined, {
'Retry-After': '5',
}),
nock(url).get('/').reply(200, 'toast'),
];
interceptorId = rax.attach();
const {promise, resolve} = invertedPromise();
const clock = sinon.useFakeTimers({
shouldAdvanceTime: true, // Otherwise interferes with nock
});
const axiosPromise = axios({
url,
raxConfig: {
onRetryAttempt: resolve,
retryDelay: 10000, // Higher default to ensure Retry-After is used
backoffType: 'static',
},
});
await promise;
clock.tick(5000); // Advance clock by expected retry delay
const res = await axiosPromise;
assert.strictEqual(res.data, 'toast');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually testing if the delays work. I know it's annoying, but could I trouble you to set up a test with a sinon timer, and verify it's doing what we expect? I've had to write similar tests in the past if you're looking for an example:
https://github.com/JustinBeckwith/linkinator/blob/main/test/test.retry.ts#L83

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done using the same pattern - also set a higher default retry delay on the rax config to make sure the Retry-After value is actually being used

scopes.forEach(s => s.done());
});

it('should retry with Retry-After header in http datetime', async function () {
this.timeout(1000);
const scopes = [
nock(url).get('/').reply(429, undefined, {
'Retry-After': 'Thu, 01 Jan 1970 00:00:05 UTC',
}),
nock(url).get('/').reply(200, 'toast'),
];
interceptorId = rax.attach();
const {promise, resolve} = invertedPromise();
const clock = sinon.useFakeTimers({
shouldAdvanceTime: true,
});
const axiosPromise = axios({
url,
raxConfig: {
onRetryAttempt: resolve,
backoffType: 'static',
retryDelay: 10000,
},
});
await promise;
clock.tick(5000);
const res = await axiosPromise;
assert.strictEqual(res.data, 'toast');
scopes.forEach(s => s.done());
});

it('should not retry if Retry-After greater than maxRetryAfter', async () => {
const scopes = [
nock(url).get('/').reply(429, undefined, {'Retry-After': '2'}),
nock(url).get('/').reply(200, 'toast'),
];
interceptorId = rax.attach();
const cfg: rax.RaxConfig = {url, raxConfig: {maxRetryAfter: 1000}};
await assert.rejects(axios(cfg));
assert.strictEqual(scopes[1].isDone(), false);
});
});

function invertedPromise() {
let resolve!: () => void;
let reject!: (err: Error) => void;
const promise = new Promise<void>((innerResolve, innerReject) => {
resolve = innerResolve;
reject = innerReject;
});
return {promise, resolve, reject};
}