From 80d4453535241a18711346095d15f73e527afb9e Mon Sep 17 00:00:00 2001 From: Wai Lun Lim Date: Sat, 20 Jul 2024 02:05:22 +0800 Subject: [PATCH 1/2] Fix lastRequestTime being always the original request time --- spec/index.spec.ts | 58 +++++++++++++++++++++++++++++++++++++++++++++- src/index.ts | 9 ++++--- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index d3e93a3..37d0f99 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -1,6 +1,6 @@ import http from 'http'; import nock from 'nock'; -import axios, { AxiosError, CanceledError, isAxiosError } from 'axios'; +import axios, { AxiosError, CanceledError, InternalAxiosRequestConfig, isAxiosError } from 'axios'; import axiosRetry, { isNetworkError, isSafeRequestError, @@ -532,6 +532,62 @@ describe('axiosRetry(axios, { validateResponse })', () => { }); }); + describe('when requests are made', () => { + it('should be that lastRequestTime is accurate', (done) => { + const client = axios.create(); + setupResponses(client, [ + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').reply(200, 'It worked!') + ]); + + function getLastRequestTime(config?: InternalAxiosRequestConfig) { + const lastRequestTime = config?.['axios-retry']?.lastRequestTime; + + expect(lastRequestTime).toBeInstanceOf(Number); + return lastRequestTime as number; + } + + const lastRequestTimes: number[] = []; + const DELAY = 500; + axiosRetry(client, { + retries: 3, + retryDelay: () => DELAY, + shouldResetTimeout: true, + onRetry(_retryCount, error, _requestConfig) { + const lastRequestTime = getLastRequestTime(error.config); + lastRequestTimes.push(lastRequestTime); + } + }); + + const startTime = Date.now(); + client + .get('http://example.com/test') + .then((response) => { + const lastRequestTime = getLastRequestTime(response.config); + lastRequestTimes.push(lastRequestTime); + + expect(lastRequestTimes.length).toBe(4); + + const [a, b, c, d] = lastRequestTimes; + + expect(a - startTime).toBeGreaterThanOrEqual(0); + expect(b - a).toBeGreaterThanOrEqual(DELAY); + expect(c - b).toBeGreaterThanOrEqual(DELAY); + expect(d - c).toBeGreaterThanOrEqual(DELAY); + + // check to ensure that the delays are not unreasonably long + expect(b - a).toBeLessThan(DELAY * 1.5); + expect(c - b).toBeLessThan(DELAY * 1.5); + expect(d - c).toBeLessThan(DELAY * 1.5); + + done(); + }) + .catch(done.fail); + }); + }); + describe('when validateResponse is supplied as request-specific configuration', () => { it('should use request-specific configuration instead', (done) => { const client = axios.create(); diff --git a/src/index.ts b/src/index.ts index 7f7443f..a72185e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -188,11 +188,14 @@ function getRequestOptions( function setCurrentState( config: AxiosRequestConfig, - defaultOptions: IAxiosRetryConfig | undefined + defaultOptions: IAxiosRetryConfig | undefined, + resetLastRequestTime = false ) { const currentState = getRequestOptions(config, defaultOptions || {}); currentState.retryCount = currentState.retryCount || 0; - currentState.lastRequestTime = currentState.lastRequestTime || Date.now(); + if (!currentState.lastRequestTime || resetLastRequestTime) { + currentState.lastRequestTime = Date.now(); + } config[namespace] = currentState; return currentState as Required; } @@ -282,7 +285,7 @@ async function handleMaxRetryTimesExceeded( const axiosRetry: AxiosRetry = (axiosInstance, defaultOptions) => { const requestInterceptorId = axiosInstance.interceptors.request.use((config) => { - setCurrentState(config, defaultOptions); + setCurrentState(config, defaultOptions, true); if (config[namespace]?.validateResponse) { // by setting this, all HTTP responses will be go through the error interceptor first config.validateStatus = () => false; From 122b9a17cedaf079749df86f34fdaa5c139ead83 Mon Sep 17 00:00:00 2001 From: Wai Lun Lim Date: Sat, 20 Jul 2024 02:23:38 +0800 Subject: [PATCH 2/2] Move new test case to be its own standalone describe --- spec/index.spec.ts | 112 ++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index 37d0f99..acd6dc6 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -532,62 +532,6 @@ describe('axiosRetry(axios, { validateResponse })', () => { }); }); - describe('when requests are made', () => { - it('should be that lastRequestTime is accurate', (done) => { - const client = axios.create(); - setupResponses(client, [ - () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), - () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), - () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), - () => nock('http://example.com').get('/test').reply(200, 'It worked!') - ]); - - function getLastRequestTime(config?: InternalAxiosRequestConfig) { - const lastRequestTime = config?.['axios-retry']?.lastRequestTime; - - expect(lastRequestTime).toBeInstanceOf(Number); - return lastRequestTime as number; - } - - const lastRequestTimes: number[] = []; - const DELAY = 500; - axiosRetry(client, { - retries: 3, - retryDelay: () => DELAY, - shouldResetTimeout: true, - onRetry(_retryCount, error, _requestConfig) { - const lastRequestTime = getLastRequestTime(error.config); - lastRequestTimes.push(lastRequestTime); - } - }); - - const startTime = Date.now(); - client - .get('http://example.com/test') - .then((response) => { - const lastRequestTime = getLastRequestTime(response.config); - lastRequestTimes.push(lastRequestTime); - - expect(lastRequestTimes.length).toBe(4); - - const [a, b, c, d] = lastRequestTimes; - - expect(a - startTime).toBeGreaterThanOrEqual(0); - expect(b - a).toBeGreaterThanOrEqual(DELAY); - expect(c - b).toBeGreaterThanOrEqual(DELAY); - expect(d - c).toBeGreaterThanOrEqual(DELAY); - - // check to ensure that the delays are not unreasonably long - expect(b - a).toBeLessThan(DELAY * 1.5); - expect(c - b).toBeLessThan(DELAY * 1.5); - expect(d - c).toBeLessThan(DELAY * 1.5); - - done(); - }) - .catch(done.fail); - }); - }); - describe('when validateResponse is supplied as request-specific configuration', () => { it('should use request-specific configuration instead', (done) => { const client = axios.create(); @@ -1222,3 +1166,59 @@ describe('axiosRetry interceptors', () => { expect(client.interceptors.response.handlers[0]).toBe(null); }); }); + +describe('when requests are made', () => { + it('should be that lastRequestTime is accurate', (done) => { + const client = axios.create(); + setupResponses(client, [ + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').replyWithError(NETWORK_ERROR), + () => nock('http://example.com').get('/test').reply(200, 'It worked!') + ]); + + function getLastRequestTime(config?: InternalAxiosRequestConfig) { + const lastRequestTime = config?.['axios-retry']?.lastRequestTime; + + expect(lastRequestTime).toBeInstanceOf(Number); + return lastRequestTime as number; + } + + const lastRequestTimes: number[] = []; + const DELAY = 500; + axiosRetry(client, { + retries: 3, + retryDelay: () => DELAY, + shouldResetTimeout: true, + onRetry(_retryCount, error, _requestConfig) { + const lastRequestTime = getLastRequestTime(error.config); + lastRequestTimes.push(lastRequestTime); + } + }); + + const startTime = Date.now(); + client + .get('http://example.com/test') + .then((response) => { + const lastRequestTime = getLastRequestTime(response.config); + lastRequestTimes.push(lastRequestTime); + + expect(lastRequestTimes.length).toBe(4); + + const [a, b, c, d] = lastRequestTimes; + + expect(a - startTime).toBeGreaterThanOrEqual(0); + expect(b - a).toBeGreaterThanOrEqual(DELAY); + expect(c - b).toBeGreaterThanOrEqual(DELAY); + expect(d - c).toBeGreaterThanOrEqual(DELAY); + + // check to ensure that the delays are not unreasonably long + expect(b - a).toBeLessThan(DELAY * 1.5); + expect(c - b).toBeLessThan(DELAY * 1.5); + expect(d - c).toBeLessThan(DELAY * 1.5); + + done(); + }) + .catch(done.fail); + }); +});