fix(adapter-utils): treat Retry-After: 0 as retry-immediately
Greptile flagged `parsed > 0` as a P1 (Retry-After: 0 falls back to
exponential backoff). The mechanism they described — `new Date("0")`
returning Invalid Date in Node 20+ — does not reproduce: V8 parses
`"0"` as year-2000 in local TZ on Node 20.20.2 and 22.22.1, so the
date branch coincidentally returns 0 ms via `Math.max(0, past - now)`.
The current code is therefore *accidentally correct* on Node 20+ in
2026, but relies on implementation-defined Date parsing per ES spec
and would break under a future V8 change or a system clock set before
year 2000. Switching `parsed > 0` to `parsed >= 0` makes the intent
explicit, matches RFC 7231 §7.1.3 ("0" = "you may retry at once"),
and skips the date branch entirely on the common case.
Also pins the behavior with two test changes:
- Strengthen the existing onRetry test to assert `delayMs: 0`.
- New explicit test: Retry-After: 0 with baseDelayMs/maxDelayMs set
to 60s — passes only if delayMs is actually 0, not exponential.
This commit is contained in:
parent
c8838ce3a2
commit
2668b1fe42
|
|
@ -2687,6 +2687,8 @@ describe("buildPaperclipEnv", () => {
|
|||
const env = buildPaperclipEnv({ id: "agent-1", companyId: "company-1" });
|
||||
expect(env.PAPERCLIP_API_URL).toBe("http://localhost:3200");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("fetchWithRetry", () => {
|
||||
let originalFetch: typeof fetch;
|
||||
|
|
@ -2784,11 +2786,33 @@ describe("fetchWithRetry", () => {
|
|||
attempt: 1,
|
||||
maxRetries: 3,
|
||||
status: 429,
|
||||
delayMs: 0,
|
||||
retryAfterHeader: "0",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("treats Retry-After: 0 as 'retry immediately' (delayMs: 0)", async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(makeResponse(429, { "retry-after": "0" }))
|
||||
.mockResolvedValueOnce(makeResponse(200));
|
||||
globalThis.fetch = fetchMock as unknown as typeof fetch;
|
||||
|
||||
const onRetry = vi.fn();
|
||||
|
||||
const res = await fetchWithRetry(
|
||||
"https://example.test",
|
||||
{},
|
||||
{ baseDelayMs: 60_000, maxDelayMs: 60_000, onRetry },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(onRetry).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ delayMs: 0, retryAfterHeader: "0" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("treats custom retryableStatuses as retryable (e.g. 403)", async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
|
|
|
|||
|
|
@ -3620,7 +3620,7 @@ export async function fetchWithRetry(
|
|||
|
||||
if (retryAfter) {
|
||||
const parsed = Number(retryAfter);
|
||||
if (!Number.isNaN(parsed) && parsed > 0) {
|
||||
if (!Number.isNaN(parsed) && parsed >= 0) {
|
||||
delayMs = parsed * 1000;
|
||||
} else {
|
||||
const date = new Date(retryAfter).getTime();
|
||||
|
|
|
|||
Loading…
Reference in New Issue