-
Notifications
You must be signed in to change notification settings - Fork 175
Open
Description
Recently we up our axios-retry
lib version and still applied our patch to fix this bug 😅
So I decided to quickly share it, maybe someone wants to implement it in future version and just reuse our patch for that.
So here is almost full jest test we have for checking desired behavior:
const retrySpy = jest.fn();
const customInterceptorSpy = jest.fn();
const customErrorInterceptorSpy = jest.fn();
let RETRY_COUNT = 0;
let ERROR_RETURN_COUNT = 0;
const getErrorReturnCount = () => ERROR_RETURN_COUNT;
const client = axios.create({
baseURL: 'http://localhost:8181',
transformResponse: [
// example of transformer, we want to know it works with it as well
(data) => {
try {
return JSONBig({ storeAsString: true }).parse(data);
} catch (err) {
return data;
}
}
]
});
const retryOptions = {
retries: 5,
retryDelay: axiosRetry.exponentialDelay,
retryCondition: (axiosError) => {
retrySpy();
return isAxiosRetryable(axiosError);
}
};
// #1. Axios-retry must go first
axiosRetry(client, retryOptions);
// #2. then our custom interceptors
client.interceptors.response.use(
(axiosResponse) => {
// This function shouldn't be called more than once, if it does -
// we will get undefined error (But it actually do w/o our patch❗️)
customInterceptorSpy();
return axiosResponse.data;
},
(axiosError) => {
// This function shouldn't be called more than once, if it does -
// we will get undefined error
customErrorInterceptorSpy();
return Promise.reject(new Error(axiosError.message));
}
);
const server = http.createServer((req, res) => {
RETRY_COUNT++;
const shouldThrowError = RETRY_COUNT <= getErrorReturnCount();
try {
res.writeHead(shouldThrowError ? 504 : 200, { 'Content-Type': 'application/json' }).end(
JSON.stringify(
shouldThrowError
? { code: 504, message: 'Request Timeout', request_id: '12345' }
: { code: 0, message: 'OK', request_id: '12345', data: [{ id: 'some-id' }] }
)
);
} catch (e) {
res.writeHead(e.statusCode, { 'Content-Type': 'application/json' });
res.end(e.message);
}
});
describe('Axios-retry', () => {
beforeAll(async () => {
await new Promise((resolve) => {
server.listen(8181, () => {
console.log('Sample HTTP Server running at 8181');
resolve(true);
});
server.on('error', (err) => {
console.error(err);
});
});
});
beforeEach(() => {
RETRY_COUNT = 0;
retrySpy.mockReset();
customInterceptorSpy.mockReset();
customErrorInterceptorSpy.mockReset();
});
afterAll(async () => {
await server.close();
});
test('should return ok - on successful request, no retry called', async () => {
const res = await client.get('');
expect(res.data[0].id).toBeTruthy();
expect(customInterceptorSpy).toBeCalledTimes(1);
expect(customErrorInterceptorSpy).not.toBeCalled();
expect(retrySpy).not.toBeCalled();
});
test('should return ok - on successful request, after several retries', async () => {
ERROR_RETURN_COUNT = 2;
const res = await client.get('');
expect(res.data[0].id).toBeTruthy();
expect(customInterceptorSpy).toBeCalledTimes(1);
expect(customErrorInterceptorSpy).not.toBeCalled();
expect(retrySpy).toBeCalledTimes(ERROR_RETURN_COUNT);
});
test('should return ok - on successful request, after all retries', async () => {
ERROR_RETURN_COUNT = 5;
const res = await client.get('');
expect(res.data[0].id).toBeTruthy();
expect(customInterceptorSpy).toBeCalledTimes(1);
expect(customErrorInterceptorSpy).not.toBeCalled();
expect(retrySpy).toBeCalledTimes(ERROR_RETURN_COUNT);
}, 10_000);
test('should return error - on failed request, after all reties exceed', async () => {
ERROR_RETURN_COUNT = 10;
await expect(client.get('')).rejects.toThrowError('Request failed with status code 504');
expect(customInterceptorSpy).not.toBeCalled();
expect(customErrorInterceptorSpy).toBeCalledTimes(1);
expect(retrySpy).toBeCalledTimes(5);
}, 10_000);
});
And here is our dirty patch (for CJS, if you're using ESM, consider just patch esm file same way):
diff --git a/node_modules/axios-retry/lib/cjs/index.js b/node_modules/axios-retry/lib/cjs/index.js
index 1cfaeed..bcfd989 100644
--- a/node_modules/axios-retry/lib/cjs/index.js
+++ b/node_modules/axios-retry/lib/cjs/index.js
@@ -335,11 +335,19 @@ function axiosRetry(axios, defaultOptions) {
config.transformRequest = [function (data) {
return data;
}];
+
+ var initialResInterceptors = axios.interceptors.response.handlers;
+ var [retryInterceptors] = initialResInterceptors; // we still need 1st retry interceptor
+
onRetry(currentState.retryCount, error, config);
return _context.abrupt("return", new Promise(function (resolve) {
return setTimeout(function () {
+ // fix for not calling the interceptors for every retry-request
+ axios.interceptors.response.handlers = [retryInterceptors];
return resolve(axios(config));
}, delay);
+ }).finally(() => {
+ axios.interceptors.response.handlers = initialResInterceptors;
}));
case 20:
Maybe we just do not use this library in the proper way 🙃
maximm-tt, eric-gitta-moore and chekrdcb-eli and nakree1
Metadata
Metadata
Assignees
Labels
No labels