From df495c67b5bab5d8a9cee571aa561e0770219164 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Feb 2026 13:53:29 -0600 Subject: [PATCH] =?UTF-8?q?fix(#411):=20QA-012=20=E2=80=94=20clamp=20Retry?= =?UTF-8?q?Options=20to=20sensible=20ranges?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetchWithRetry now clamps maxRetries>=0, baseDelayMs>=100, backoffFactor>=1 to prevent infinite loops or zero-delay hammering. Co-Authored-By: Claude Opus 4.6 --- .../web/src/lib/auth/fetch-with-retry.test.ts | 64 +++++++++++++++++++ apps/web/src/lib/auth/fetch-with-retry.ts | 6 +- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/apps/web/src/lib/auth/fetch-with-retry.test.ts b/apps/web/src/lib/auth/fetch-with-retry.test.ts index 72c1d31..fb9f7ac 100644 --- a/apps/web/src/lib/auth/fetch-with-retry.test.ts +++ b/apps/web/src/lib/auth/fetch-with-retry.test.ts @@ -280,4 +280,68 @@ describe("fetchWithRetry", (): void => { expect(global.fetch).toHaveBeenCalledTimes(3); expect(sleepMock).toHaveBeenCalledTimes(2); }); + + describe("RetryOptions value clamping", (): void => { + it("should clamp negative maxRetries to 0 (no retries)", async (): Promise => { + const serverError = mockResponse(500); + vi.mocked(global.fetch).mockResolvedValueOnce(serverError); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: -5, + }); + + // maxRetries clamped to 0 means only the initial attempt, no retries + expect(result.status).toBe(500); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should clamp fractional maxRetries by flooring", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + const okResponse = mockResponse(200); + vi.mocked(global.fetch).mockRejectedValueOnce(networkError).mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 1.9, + baseDelayMs: 100, + }); + + // 1.9 floors to 1, so 1 initial + 1 retry = 2 attempts + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(sleepMock).toHaveBeenCalledTimes(1); + }); + + it("should clamp baseDelayMs below 100 up to 100", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config", undefined, { + baseDelayMs: 0, + }); + + // baseDelayMs clamped to 100, so first retry delay = 100 * 2^0 = 100 + expect(recordedDelays[0]).toBe(100); + }); + + it("should clamp backoffFactor below 1 up to 1 (linear delay)", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 3, + baseDelayMs: 200, + backoffFactor: 0, + }); + + // backoffFactor clamped to 1, so delays are 200*1^0=200, 200*1^1=200 (constant) + expect(recordedDelays).toEqual([200, 200]); + }); + }); }); diff --git a/apps/web/src/lib/auth/fetch-with-retry.ts b/apps/web/src/lib/auth/fetch-with-retry.ts index 3afaa2e..db2f26b 100644 --- a/apps/web/src/lib/auth/fetch-with-retry.ts +++ b/apps/web/src/lib/auth/fetch-with-retry.ts @@ -54,9 +54,9 @@ export async function fetchWithRetry( options?: RequestInit, retryOptions?: RetryOptions ): Promise { - const maxRetries = retryOptions?.maxRetries ?? DEFAULT_MAX_RETRIES; - const baseDelayMs = retryOptions?.baseDelayMs ?? DEFAULT_BASE_DELAY_MS; - const backoffFactor = retryOptions?.backoffFactor ?? DEFAULT_BACKOFF_FACTOR; + const maxRetries = Math.max(0, Math.floor(retryOptions?.maxRetries ?? DEFAULT_MAX_RETRIES)); + const baseDelayMs = Math.max(100, retryOptions?.baseDelayMs ?? DEFAULT_BASE_DELAY_MS); + const backoffFactor = Math.max(1, retryOptions?.backoffFactor ?? DEFAULT_BACKOFF_FACTOR); let lastError: unknown = null; let lastResponse: Response | null = null;