fix(#411): QA-012 — clamp RetryOptions to sensible ranges
fetchWithRetry now clamps maxRetries>=0, baseDelayMs>=100, backoffFactor>=1 to prevent infinite loops or zero-delay hammering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -280,4 +280,68 @@ describe("fetchWithRetry", (): void => {
|
|||||||
expect(global.fetch).toHaveBeenCalledTimes(3);
|
expect(global.fetch).toHaveBeenCalledTimes(3);
|
||||||
expect(sleepMock).toHaveBeenCalledTimes(2);
|
expect(sleepMock).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("RetryOptions value clamping", (): void => {
|
||||||
|
it("should clamp negative maxRetries to 0 (no retries)", async (): Promise<void> => {
|
||||||
|
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<void> => {
|
||||||
|
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<void> => {
|
||||||
|
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<void> => {
|
||||||
|
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]);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -54,9 +54,9 @@ export async function fetchWithRetry(
|
|||||||
options?: RequestInit,
|
options?: RequestInit,
|
||||||
retryOptions?: RetryOptions
|
retryOptions?: RetryOptions
|
||||||
): Promise<Response> {
|
): Promise<Response> {
|
||||||
const maxRetries = retryOptions?.maxRetries ?? DEFAULT_MAX_RETRIES;
|
const maxRetries = Math.max(0, Math.floor(retryOptions?.maxRetries ?? DEFAULT_MAX_RETRIES));
|
||||||
const baseDelayMs = retryOptions?.baseDelayMs ?? DEFAULT_BASE_DELAY_MS;
|
const baseDelayMs = Math.max(100, retryOptions?.baseDelayMs ?? DEFAULT_BASE_DELAY_MS);
|
||||||
const backoffFactor = retryOptions?.backoffFactor ?? DEFAULT_BACKOFF_FACTOR;
|
const backoffFactor = Math.max(1, retryOptions?.backoffFactor ?? DEFAULT_BACKOFF_FACTOR);
|
||||||
|
|
||||||
let lastError: unknown = null;
|
let lastError: unknown = null;
|
||||||
let lastResponse: Response | null = null;
|
let lastResponse: Response | null = null;
|
||||||
|
|||||||
Reference in New Issue
Block a user