Skip to content

Commit

Permalink
Merge pull request #678 from sjinks/revert-664-add-timeout
Browse files Browse the repository at this point in the history 
Revert "feat: add timeout capabilities for each retry"
  • Loading branch information
@sjinks
sjinks authored Jul 27, 2023
2 parents 2371786 + 2e47b3f commit 16890cf
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 89 deletions.
5 changes: 1 addition & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const options = {
retries: 3,
retryDelay: 1000,
retryOn: [419, 503, 504],
retryTimeout: 2000,
};

const fetch = fetchBuilder(originalFetch, options);
Expand All @@ -39,11 +38,9 @@ It accepts two arguments:
* `response`: `Response` or `null` if `error !== null`
It should return an integer, which is treated as the delay in ms before the enxt attempt is made. The default value for `retryDelay` is `1000`.
* `retryOn?: number[] | (attempt: number, retries: number, error: Error | null, response: Response | null) => boolean`: if specified as an array of integers, it is treated as a list of HTTP codes which trigger retry. When specified as a function, that functoin accepts the same parameters as the one described in `retryDelay`, and an additional parameter called `retries`, whcih is the number of configured retries. The function should return a truthy value if the request should be retried. *If `retryOn` is a function, `retries` is ignored.* The default value for `retryOn` in `[429, 503, 504]`.
* `retryTimeout?: number` : timeout in milliseconds for each retry attempt.

It returns a function to be used instead of `fetch()`.

The returned function accepts the same arguments as `fetch(input: RequestInfo, init?: RequestInit)`, and foure additional properties in `init` object. Those are `retries`, `retryDelay`, `retryOn`, and `retryTimeout`.
The returned function accepts the same arguments as `fetch(input: RequestInfo, init?: RequestInit)`, and three additional properties in `init` object. Those are `retries`, `retryDelay`, and `retryOn`.

## Examples

Expand Down
35 changes: 4 additions & 31 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export interface FetchRetryParams {
retries?: number;
retryDelay?: number | RequestDelayFunction;
retryOn?: number[] | RetryRequestFunction;
retryTimeout?: number;
}

function sanitize(params: FetchRetryParams, defaults: Required<FetchRetryParams>): Required<FetchRetryParams> {
Expand All @@ -27,10 +26,6 @@ function sanitize(params: FetchRetryParams, defaults: Required<FetchRetryParams>
result.retryOn = defaults.retryOn;
}

if (typeof result.retryTimeout === 'undefined') {
result.retryTimeout = defaults.retryTimeout;
}

return result;
}

Expand All @@ -39,15 +34,14 @@ export default function <F extends (...args: any) => Promise<any> = typeof fetch
fetchFunc: F,
params: FetchRetryParams = {},
): (input: Parameters<F>[0], init?: Parameters<F>[1] & FetchRetryParams) => ReturnType<F> {
const defaults = sanitize(params, { retries: 3, retryDelay: 500, retryOn: [419, 503, 504], retryTimeout: 300000 });
const defaults = sanitize(params, { retries: 3, retryDelay: 500, retryOn: [419, 503, 504] });

return function (input: Parameters<F>[0], init?: Parameters<F>[1] & FetchRetryParams): ReturnType<F> {
const frp = sanitize(
{
retries: init?.retries,
retryDelay: init?.retryDelay,
retryOn: init?.retryOn,
retryTimeout: init?.retryTimeout,
},
defaults,
);
Expand All @@ -64,39 +58,18 @@ export default function <F extends (...args: any) => Promise<any> = typeof fetch

return new Promise(function (resolve, reject): void {
const extendedFetch = function (attempt: number): void {
const _init: Parameters<F>[1] = typeof init === 'undefined' ? {} : init;

let abortTimeout: NodeJS.Timeout;

if (typeof frp?.retryTimeout === 'number') {
const ac = new AbortController();
abortTimeout = setTimeout(
() => {
ac.abort();
},
frp?.retryTimeout,
);
_init.signal = ac.signal;
}

fetchFunc(input, _init)
fetchFunc(input, init)
.then(function (response: Response): void {
if (abortTimeout !== undefined) {
clearTimeout(abortTimeout);
}

if (retryOnFn(attempt, frp.retries, null, response)) {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
retry(attempt, null, response);
} else {
resolve(response);
}
})
.catch(function (error: Error): void {
if (abortTimeout !== undefined) {
clearTimeout(abortTimeout);
}

if (retryOnFn(attempt, frp.retries, error, null)) {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
retry(attempt, error, null);
} else {
reject(error);
Expand Down
64 changes: 10 additions & 54 deletions src/test/unit/index.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Response, type RequestInfo, type RequestInit } from 'node-fetch';
import { Response } from 'node-fetch';
import builder from '../../';

const mockedFetch = jest.fn<Promise<Response>, [RequestInfo, RequestInit | undefined]>();
const mockedFetch = jest.fn();

describe('fetch builder', (): void => {
it('should return a function', (): void => {
Expand All @@ -25,7 +25,6 @@ describe('fetch builder', (): void => {
describe('fetch retry', (): void => {
beforeEach((): void => {
jest.resetAllMocks();
jest.useRealTimers();
});

it('passes input parameter to fetch()', async (): Promise<void> => {
Expand All @@ -38,7 +37,7 @@ describe('fetch retry', (): void => {
return expect(f(expectedParam))
.resolves.toEqual(expectedResponse)
.then((): void => {
expect(mockedFetch).toBeCalledWith(expectedParam, { signal: expect.any(AbortSignal) });
expect(mockedFetch).toBeCalledWith(expectedParam, undefined);
});
});

Expand Down Expand Up @@ -120,8 +119,7 @@ describe('fetch retry', (): void => {
});

it('should call retry functions', async (): Promise<void> => {
jest.useFakeTimers();
const delayFn = jest.fn((): number => 20);
const delayFn = jest.fn((): number => 0);
const retryFn = jest.fn();

retryFn.mockReturnValueOnce(true);
Expand All @@ -134,54 +132,12 @@ describe('fetch retry', (): void => {
mockedFetch.mockResolvedValueOnce(new Response('419', { status: 419 }));
mockedFetch.mockResolvedValueOnce(new Response('200', { status: 200 }));

const promise = f('https://example.test', { retries: 3, retryDelay: delayFn, retryOn: retryFn });
await jest.runAllTimersAsync();
const response = await promise;

expect(response).toMatchObject({ status: 504 });
expect(mockedFetch.mock.calls.length).toBe(2);
expect(retryFn.mock.calls.length).toBe(2);
expect(delayFn.mock.calls.length).toBe(2 - 1);
});

it('should timeout before the server answers when timeout value is configured', async (): Promise<void> => {
jest.useFakeTimers();
const retryFn = jest.fn();
const timeout = 2;
const callTimes: Date[] = [];

const f = builder(mockedFetch, { retries: 1, retryDelay: 0, retryOn: retryFn, retryTimeout: timeout * 1000 });

retryFn.mockReturnValueOnce(true);
retryFn.mockReturnValueOnce(false);

mockedFetch.mockImplementation((url, init) => {
callTimes.push(new Date());
return new Promise((resolve, reject) => {
const timer = setTimeout(
() => {
console.log('resolving');
resolve(new Response('503', { status: 503 }));
},
timeout * 1000 + 500,
);

init?.signal?.addEventListener('abort', () => {
clearTimeout(timer);
reject(new Error('abort'));
});
return expect(f('https://example.test', { retries: 3, retryDelay: delayFn, retryOn: retryFn }))
.resolves.toMatchObject({ status: 504 })
.then((): void => {
expect(mockedFetch.mock.calls.length).toBe(2);
expect(retryFn.mock.calls.length).toBe(2);
expect(delayFn.mock.calls.length).toBe(2 - 1);
});
});

const promise = f('https://example.test');
await jest.advanceTimersToNextTimerAsync(); // will abort the request here and retry

const [result] = await Promise.allSettled([promise, jest.advanceTimersToNextTimerAsync()]); // will abort the request for the second time here and will not retry

expect(result.status).toBe('rejected');
expect((result as PromiseRejectedResult).reason).toMatchObject({ message: 'abort' });
expect(mockedFetch.mock.calls.length).toBe(2);
expect(callTimes.length).toBe(2);
expect(callTimes[1].getSeconds() - callTimes[0].getSeconds()).toBe(2);
});
});

0 comments on commit 16890cf

Please sign in to comment.