From 75a63caee9e05586a194488b4d075b2b2d71722a Mon Sep 17 00:00:00 2001 From: yutak23 Date: Fri, 15 Dec 2023 22:00:31 +0900 Subject: [PATCH 1/4] feat: add disableOtherResponseInterceptors option --- spec/index.spec.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++++ src/index.ts | 40 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index 9c61473..2bfe6b5 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -569,6 +569,53 @@ describe('axiosRetry(axios, { retries, onRetry })', () => { }); }); +describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { + afterEach(() => { + nock.cleanAll(); + nock.enableNetConnect(); + }); + + it('should not multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); + nock('http://example.com').get('/test').reply(200, 'It worked!'); + + axiosRetry(client, { disableOtherResponseInterceptors: true }); + + let anotherInterceptorCallCount = 0; + client.interceptors.response.use((result) => { + anotherInterceptorCallCount += 1; + return result; + }, null); + + client.get('http://example.com/test').then((result) => { + expect(result.status).toBe(200); + expect(anotherInterceptorCallCount).toBe(1); + done(); + }, done.fail); + }); + + it('should multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); + nock('http://example.com').get('/test').reply(200, 'It worked!'); + + axiosRetry(client, { disableOtherResponseInterceptors: false }); + + let anotherInterceptorCallCount = 0; + client.interceptors.response.use((result) => { + anotherInterceptorCallCount += 1; + return result; + }, null); + + client.get('http://example.com/test').then((result) => { + expect(result.status).toBe(200); + expect(anotherInterceptorCallCount).toBe(3); + done(); + }, done.fail); + }); +}); + describe('isNetworkError(error)', () => { it('should be true for network errors like connection refused', () => { const connectionRefusedError = new AxiosError(); diff --git a/src/index.ts b/src/index.ts index 5e81f91..4e210b8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,23 @@ -import type { AxiosError, AxiosRequestConfig, AxiosInstance, AxiosStatic } from 'axios'; +import type { + AxiosError, + AxiosRequestConfig, + AxiosInstance, + AxiosStatic, + AxiosInterceptorManager, + AxiosResponse, + InternalAxiosRequestConfig +} from 'axios'; import isRetryAllowed from 'is-retry-allowed'; +interface AxiosResponseInterceptorManagerExtended extends AxiosInterceptorManager { + handlers: Array<{ + fulfilled: ((value: AxiosResponse) => AxiosResponse | Promise) | null; + rejected: ((error: any) => any) | null; + synchronous: boolean; + runWhen: (config: InternalAxiosRequestConfig) => boolean; + }>; +} + export interface IAxiosRetryConfig { /** * The number of times to retry before failing @@ -12,6 +29,11 @@ export interface IAxiosRetryConfig { * default: false */ shouldResetTimeout?: boolean; + /** + * Disable other response interceptors when axios-retry is retrying the request + * default: false + */ + disableOtherResponseInterceptors?: boolean; /** * A callback to further control if a request should be retried. * default: it retries if it is a network error or a 5xx error on an idempotent request (GET, HEAD, OPTIONS, PUT or DELETE). @@ -141,6 +163,7 @@ export const DEFAULT_OPTIONS: Required = { retryCondition: isNetworkOrIdempotentRequestError, retryDelay: noDelay, shouldResetTimeout: false, + disableOtherResponseInterceptors: false, onRetry: () => {} }; @@ -196,13 +219,14 @@ async function shouldRetry( return shouldRetryOrPromise; } +let responseInterceptorId: number; const axiosRetry: AxiosRetry = (axiosInstance, defaultOptions) => { const requestInterceptorId = axiosInstance.interceptors.request.use((config) => { setCurrentState(config, defaultOptions); return config; }); - const responseInterceptorId = axiosInstance.interceptors.response.use(null, async (error) => { + responseInterceptorId = axiosInstance.interceptors.response.use(null, async (error) => { const { config } = error; // If we have no information to retry the request if (!config) { @@ -227,7 +251,17 @@ const axiosRetry: AxiosRetry = (axiosInstance, defaultOptions) => { config.transformRequest = [(data) => data]; await onRetry(currentState.retryCount, error, config); return new Promise((resolve) => { - setTimeout(() => resolve(axiosInstance(config)), delay); + setTimeout(() => { + if (currentState.disableOtherResponseInterceptors && currentState.retryCount === 1) { + const extendedInterceptor = axiosInstance.interceptors + .response as AxiosResponseInterceptorManagerExtended; + const axiosRetryInterceptor = extendedInterceptor.handlers[responseInterceptorId]; + extendedInterceptor.handlers = [axiosRetryInterceptor]; + resolve(axiosInstance(config)); + return; + } + resolve(axiosInstance(config)); + }, delay); }); } return Promise.reject(error); From 7e7c61e299c964f4364dc494721f4539591f1f51 Mon Sep 17 00:00:00 2001 From: yutak23 Date: Mon, 18 Dec 2023 09:09:17 +0900 Subject: [PATCH 2/4] test: add testing for patterns of failure after retries --- spec/index.spec.ts | 112 +++++++++++++++++++++++++++++++++------------ 1 file changed, 82 insertions(+), 30 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index 2bfe6b5..ee899b6 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -575,44 +575,96 @@ describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { nock.enableNetConnect(); }); - it('should not multiple response interceptor', (done) => { - const client = axios.create(); - nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); - nock('http://example.com').get('/test').reply(200, 'It worked!'); + describe('successful after retry', () => { + it('should not multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); + nock('http://example.com').get('/test').reply(200, 'It worked!'); - axiosRetry(client, { disableOtherResponseInterceptors: true }); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: true }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use((result) => { - anotherInterceptorCallCount += 1; - return result; - }, null); + let anotherInterceptorCallCount = 0; + client.interceptors.response.use((result) => { + anotherInterceptorCallCount += 1; + return result; + }, null); + + client.get('http://example.com/test').then((result) => { + expect(result.status).toBe(200); + expect(anotherInterceptorCallCount).toBe(1); + done(); + }, done.fail); + }); + + it('should multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); + nock('http://example.com').get('/test').reply(200, 'It worked!'); + + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: false }); - client.get('http://example.com/test').then((result) => { - expect(result.status).toBe(200); - expect(anotherInterceptorCallCount).toBe(1); - done(); - }, done.fail); + let anotherInterceptorCallCount = 0; + client.interceptors.response.use((result) => { + anotherInterceptorCallCount += 1; + return result; + }, null); + + client.get('http://example.com/test').then((result) => { + expect(result.status).toBe(200); + expect(anotherInterceptorCallCount).toBe(3); + done(); + }, done.fail); + }); }); - it('should multiple response interceptor', (done) => { - const client = axios.create(); - nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); - nock('http://example.com').get('/test').reply(200, 'It worked!'); + describe('failure after retry', () => { + it('should not multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(3).replyWithError(NETWORK_ERROR); - axiosRetry(client, { disableOtherResponseInterceptors: false }); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: true }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use((result) => { - anotherInterceptorCallCount += 1; - return result; - }, null); + let anotherInterceptorCallCount = 0; + client.interceptors.response.use(null, (error) => { + anotherInterceptorCallCount += 1; + return Promise.reject(error); + }); + + client + .get('http://example.com/test') + .then( + () => done.fail(), + (error) => { + expect(anotherInterceptorCallCount).toBe(1); + done(); + } + ) + .catch(done.fail); + }); + + it('should multiple response interceptor', (done) => { + const client = axios.create(); + nock('http://example.com').get('/test').times(3).replyWithError(NETWORK_ERROR); + + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: false }); - client.get('http://example.com/test').then((result) => { - expect(result.status).toBe(200); - expect(anotherInterceptorCallCount).toBe(3); - done(); - }, done.fail); + let anotherInterceptorCallCount = 0; + client.interceptors.response.use(null, (error) => { + anotherInterceptorCallCount += 1; + return Promise.reject(error); + }); + + client + .get('http://example.com/test') + .then( + () => done.fail(), + (error) => { + expect(anotherInterceptorCallCount).toBe(4); + done(); + } + ) + .catch(done.fail); + }); }); }); From e06b2dbf94a39de094e94a9a35a51f1479cbe5df Mon Sep 17 00:00:00 2001 From: yutak23 Date: Mon, 18 Dec 2023 09:17:25 +0900 Subject: [PATCH 3/4] fix: runWhen should be nullable --- src/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4e210b8..8974f23 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,7 +14,7 @@ interface AxiosResponseInterceptorManagerExtended extends AxiosInterceptorManage fulfilled: ((value: AxiosResponse) => AxiosResponse | Promise) | null; rejected: ((error: any) => any) | null; synchronous: boolean; - runWhen: (config: InternalAxiosRequestConfig) => boolean; + runWhen: (config: InternalAxiosRequestConfig) => boolean | null; }>; } @@ -253,10 +253,10 @@ const axiosRetry: AxiosRetry = (axiosInstance, defaultOptions) => { return new Promise((resolve) => { setTimeout(() => { if (currentState.disableOtherResponseInterceptors && currentState.retryCount === 1) { - const extendedInterceptor = axiosInstance.interceptors + const responseInterceptors = axiosInstance.interceptors .response as AxiosResponseInterceptorManagerExtended; - const axiosRetryInterceptor = extendedInterceptor.handlers[responseInterceptorId]; - extendedInterceptor.handlers = [axiosRetryInterceptor]; + const axiosRetryInterceptor = responseInterceptors.handlers[responseInterceptorId]; + responseInterceptors.handlers = [axiosRetryInterceptor]; resolve(axiosInstance(config)); return; } From 8a4c4c139e02e733fdbbb7343997286514997d6d Mon Sep 17 00:00:00 2001 From: yutak23 Date: Fri, 22 Dec 2023 18:31:22 +0900 Subject: [PATCH 4/4] fix: support before and after fullfilled, rejected --- spec/index.spec.ts | 140 +++++++++++++++++++++++++++++++++++++-------- src/index.ts | 10 +++- 2 files changed, 124 insertions(+), 26 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index ee899b6..441aca8 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -581,17 +581,40 @@ describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); nock('http://example.com').get('/test').reply(200, 'It worked!'); + let anotherInterceptorBeforeFulfilledCallCount = 0; + let anotherInterceptorBeforeRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorBeforeFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorBeforeRejectedCallCount += 1; + return Promise.reject(error); + } + ); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: true }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use((result) => { - anotherInterceptorCallCount += 1; - return result; - }, null); + let anotherInterceptorAfterFulfilledCallCount = 0; + let anotherInterceptorAfterRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorAfterFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorAfterRejectedCallCount += 1; + return Promise.reject(error); + } + ); client.get('http://example.com/test').then((result) => { expect(result.status).toBe(200); - expect(anotherInterceptorCallCount).toBe(1); + expect(anotherInterceptorBeforeFulfilledCallCount).toBe(1); + expect(anotherInterceptorBeforeRejectedCallCount).toBe(1); + expect(anotherInterceptorAfterFulfilledCallCount).toBe(1); + expect(anotherInterceptorAfterRejectedCallCount).toBe(0); done(); }, done.fail); }); @@ -601,17 +624,40 @@ describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { nock('http://example.com').get('/test').times(2).replyWithError(NETWORK_ERROR); nock('http://example.com').get('/test').reply(200, 'It worked!'); + let anotherInterceptorBeforeFulfilledCallCount = 0; + let anotherInterceptorBeforeRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorBeforeFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorBeforeRejectedCallCount += 1; + return Promise.reject(error); + } + ); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: false }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use((result) => { - anotherInterceptorCallCount += 1; - return result; - }, null); + let anotherInterceptorAfterFulfilledCallCount = 0; + let anotherInterceptorAfterRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorAfterFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorAfterRejectedCallCount += 1; + return Promise.reject(error); + } + ); client.get('http://example.com/test').then((result) => { expect(result.status).toBe(200); - expect(anotherInterceptorCallCount).toBe(3); + expect(anotherInterceptorBeforeFulfilledCallCount).toBe(1); + expect(anotherInterceptorBeforeRejectedCallCount).toBe(2); + expect(anotherInterceptorAfterFulfilledCallCount).toBe(3); + expect(anotherInterceptorAfterRejectedCallCount).toBe(0); done(); }, done.fail); }); @@ -622,20 +668,43 @@ describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { const client = axios.create(); nock('http://example.com').get('/test').times(3).replyWithError(NETWORK_ERROR); + let anotherInterceptorBeforeFulfilledCallCount = 0; + let anotherInterceptorBeforeRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorBeforeFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorBeforeRejectedCallCount += 1; + return Promise.reject(error); + } + ); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: true }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use(null, (error) => { - anotherInterceptorCallCount += 1; - return Promise.reject(error); - }); + let anotherInterceptorAfterFulfilledCallCount = 0; + let anotherInterceptorAfterRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorAfterFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorAfterRejectedCallCount += 1; + return Promise.reject(error); + } + ); client .get('http://example.com/test') .then( () => done.fail(), (error) => { - expect(anotherInterceptorCallCount).toBe(1); + expect(anotherInterceptorBeforeFulfilledCallCount).toBe(0); + expect(anotherInterceptorBeforeRejectedCallCount).toBe(1); + expect(anotherInterceptorAfterFulfilledCallCount).toBe(0); + expect(anotherInterceptorAfterRejectedCallCount).toBe(1); done(); } ) @@ -646,20 +715,43 @@ describe('axiosRetry(axios, { disableOtherResponseInterceptors })', () => { const client = axios.create(); nock('http://example.com').get('/test').times(3).replyWithError(NETWORK_ERROR); + let anotherInterceptorBeforeFulfilledCallCount = 0; + let anotherInterceptorBeforeRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorBeforeFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorBeforeRejectedCallCount += 1; + return Promise.reject(error); + } + ); + axiosRetry(client, { retries: 3, disableOtherResponseInterceptors: false }); - let anotherInterceptorCallCount = 0; - client.interceptors.response.use(null, (error) => { - anotherInterceptorCallCount += 1; - return Promise.reject(error); - }); + let anotherInterceptorAfterFulfilledCallCount = 0; + let anotherInterceptorAfterRejectedCallCount = 0; + client.interceptors.response.use( + (result) => { + anotherInterceptorAfterFulfilledCallCount += 1; + return result; + }, + (error) => { + anotherInterceptorAfterRejectedCallCount += 1; + return Promise.reject(error); + } + ); client .get('http://example.com/test') .then( () => done.fail(), (error) => { - expect(anotherInterceptorCallCount).toBe(4); + expect(anotherInterceptorBeforeFulfilledCallCount).toBe(0); + expect(anotherInterceptorBeforeRejectedCallCount).toBe(4); + expect(anotherInterceptorAfterFulfilledCallCount).toBe(0); + expect(anotherInterceptorAfterRejectedCallCount).toBe(4); done(); } ) diff --git a/src/index.ts b/src/index.ts index 8974f23..fba639e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -255,8 +255,14 @@ const axiosRetry: AxiosRetry = (axiosInstance, defaultOptions) => { if (currentState.disableOtherResponseInterceptors && currentState.retryCount === 1) { const responseInterceptors = axiosInstance.interceptors .response as AxiosResponseInterceptorManagerExtended; - const axiosRetryInterceptor = responseInterceptors.handlers[responseInterceptorId]; - responseInterceptors.handlers = [axiosRetryInterceptor]; + const interceptors = responseInterceptors.handlers.splice(0, responseInterceptorId + 1); + + // Disable only intercepter on rejected (do not disable fullfilled) + responseInterceptors.handlers = interceptors.map((v, index) => { + if (index === responseInterceptorId) return v; + return { ...v, rejected: null }; + }); + resolve(axiosInstance(config)); return; }