Skip to content

Success axios interceptors fired multiple times #246

@lavagri

Description

@lavagri

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 🙃

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions