From 7b65a2e3109657cf7ac6e52d089e916d96b9d407 Mon Sep 17 00:00:00 2001 From: Arnel Robles Date: Thu, 12 Nov 2026 20:38:40 +0800 Subject: [PATCH 1/2] stop retrying 4xx and honor the caller abort signal in resilient-fetcher --- .changeset/fix-no-4xx-retry.md | 5 +++ packages/resilient-fetcher/src/index.test.ts | 45 +++++++++++++++++++- packages/resilient-fetcher/src/index.ts | 39 ++++++++++++++++- 3 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-no-4xx-retry.md diff --git a/.changeset/fix-no-4xx-retry.md b/.changeset/fix-no-4xx-retry.md new file mode 100644 index 0000000..951be08 --- /dev/null +++ b/.changeset/fix-no-4xx-retry.md @@ -0,0 +1,5 @@ +--- +"resilient-fetcher": patch +--- + +Stop retrying non-ok responses that retryOn rejects (4xx by default), and honor a caller-supplied AbortSignal. diff --git a/packages/resilient-fetcher/src/index.test.ts b/packages/resilient-fetcher/src/index.test.ts index a396d0d..d317bab 100644 --- a/packages/resilient-fetcher/src/index.test.ts +++ b/packages/resilient-fetcher/src/index.test.ts @@ -77,8 +77,7 @@ describe('resilientFetch', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - // TODO: Fix fake timer/AbortController conflict - logic verified working in integration - test.skip('does not retry on 4xx by default', async () => { + test('does not retry on 4xx by default', async () => { mockFetch.mockResolvedValue({ ok: false, status: 404 }); const promise = resilientFetch('https://example.com', { @@ -91,6 +90,22 @@ describe('resilientFetch', () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); + test('retries 5xx up to the retry limit then throws', async () => { + mockFetch.mockResolvedValue({ ok: false, status: 503 }); + + const promise = resilientFetch('https://example.com', { + retries: 2, + retryDelay: 50 + }).catch(e => e); + + await vi.advanceTimersByTimeAsync(50); + await vi.advanceTimersByTimeAsync(50); + + const error = await promise; + expect(error.message).toBe('Request failed with status 503'); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + test('custom retryOn condition', async () => { mockFetch .mockResolvedValueOnce({ ok: false, status: 429 }) @@ -147,6 +162,32 @@ describe('resilientFetch', () => { expect(call[1].signal).toBeInstanceOf(AbortSignal); }); + test('caller abort stops the request and is not retried', async () => { + mockFetch.mockImplementation((_url, options) => { + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => { + const err = new Error('Aborted'); + err.name = 'AbortError'; + reject(err); + }); + }); + }); + + const caller = new AbortController(); + const promise = resilientFetch('https://example.com', { + retries: 3, + retryDelay: 50, + signal: caller.signal + }).catch(e => e); + + caller.abort(); + await vi.advanceTimersByTimeAsync(500); + + const error = await promise; + expect(error.name).toBe('AbortError'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + test('aborts on timeout', async () => { vi.useRealTimers(); mockFetch.mockImplementation((_url, options) => { diff --git a/packages/resilient-fetcher/src/index.ts b/packages/resilient-fetcher/src/index.ts index 5c76d54..3750a14 100644 --- a/packages/resilient-fetcher/src/index.ts +++ b/packages/resilient-fetcher/src/index.ts @@ -37,6 +37,14 @@ const defaultRetryOn = (_error: Error | null, response: Response | null): boolea return response.status >= 500; }; +/** Thrown for a non-ok response that is not retried. Never passed to retryOn. */ +class HttpStatusError extends Error { + constructor(status: number) { + super(`Request failed with status ${status}`); + this.name = 'HttpStatusError'; + } +} + /** * A resilient wrapper around the native fetch API with retry and timeout capabilities. * @@ -74,9 +82,19 @@ export async function resilientFetch( finalOptions = await onRequest(finalUrl, fetchOptions); } + const callerSignal = finalOptions.signal ?? undefined; + while (true) { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeout); + const onCallerAbort = () => controller.abort(); + if (callerSignal) { + if (callerSignal.aborted) { + controller.abort(); + } else { + callerSignal.addEventListener('abort', onCallerAbort, { once: true }); + } + } try { const response = await fetch(finalUrl, { @@ -94,7 +112,7 @@ export async function resilientFetch( await new Promise(resolve => setTimeout(resolve, delay)); continue; } - throw new Error(`Request failed with status ${response.status}`); + throw new HttpStatusError(response.status); } // Apply response interceptor @@ -105,6 +123,23 @@ export async function resilientFetch( return response; } catch (error) { clearTimeout(timeoutId); + + // The caller aborted: surface the abort and never retry + if (callerSignal?.aborted) { + if (onError) { + onError(error as Error); + } + throw error; + } + + // A non-ok HTTP response already went through retryOn above + if (error instanceof HttpStatusError) { + if (onError) { + onError(error); + } + throw error; + } + attempt++; const isAbort = (error as Error).name === 'AbortError'; @@ -123,6 +158,8 @@ export async function resilientFetch( } throw err; + } finally { + callerSignal?.removeEventListener('abort', onCallerAbort); } } } From 622155c6ed1c0ed7b102b455595b540eb895f7b9 Mon Sep 17 00:00:00 2001 From: Arnel Robles Date: Thu, 12 Nov 2026 20:43:25 +0800 Subject: [PATCH 2/2] skip new attempts after a caller abort --- packages/resilient-fetcher/src/index.test.ts | 10 ++++++++++ packages/resilient-fetcher/src/index.ts | 13 ++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/resilient-fetcher/src/index.test.ts b/packages/resilient-fetcher/src/index.test.ts index d317bab..b44acaa 100644 --- a/packages/resilient-fetcher/src/index.test.ts +++ b/packages/resilient-fetcher/src/index.test.ts @@ -188,6 +188,16 @@ describe('resilientFetch', () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); + test('already aborted caller signal makes no fetch call', async () => { + const caller = new AbortController(); + caller.abort(); + + await expect( + resilientFetch('https://example.com', { signal: caller.signal }) + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + test('aborts on timeout', async () => { vi.useRealTimers(); mockFetch.mockImplementation((_url, options) => { diff --git a/packages/resilient-fetcher/src/index.ts b/packages/resilient-fetcher/src/index.ts index 3750a14..ffaaa95 100644 --- a/packages/resilient-fetcher/src/index.ts +++ b/packages/resilient-fetcher/src/index.ts @@ -85,16 +85,15 @@ export async function resilientFetch( const callerSignal = finalOptions.signal ?? undefined; while (true) { + // Do not start an attempt after the caller aborted (also covers the backoff sleep) + if (callerSignal?.aborted) { + throw callerSignal.reason ?? new DOMException('This operation was aborted', 'AbortError'); + } + const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeout); const onCallerAbort = () => controller.abort(); - if (callerSignal) { - if (callerSignal.aborted) { - controller.abort(); - } else { - callerSignal.addEventListener('abort', onCallerAbort, { once: true }); - } - } + callerSignal?.addEventListener('abort', onCallerAbort, { once: true }); try { const response = await fetch(finalUrl, {