diff --git a/src/core/client.ts b/src/core/client.ts index b66fe10..565cd3c 100644 --- a/src/core/client.ts +++ b/src/core/client.ts @@ -7,6 +7,24 @@ const DEFAULT_MAX_RETRIES = 5; const DEFAULT_BASE_DELAY = 50; const DEFAULT_TIMEOUT = 30_000; const DEFAULT_USER_AGENT = "registries/0.1.0"; +const MAX_TIMER_DELAY = 2_147_483_647; + +function parseRetryAfterValue(header: string | null | undefined): number | undefined { + if (!header) return undefined; + const trimmed = header.trim(); + if (!trimmed) return undefined; + + if (/^\d+$/.test(trimmed)) { + const seconds = Number(trimmed); + return Number.isNaN(seconds) ? undefined : seconds; + } + + const timestamp = /[a-z]/i.test(trimmed) ? Date.parse(trimmed) : NaN; + if (Number.isNaN(timestamp)) return undefined; + + const seconds = Math.ceil((timestamp - Date.now()) / 1000); + return Math.max(seconds, 0); +} /** * Parse a `Retry-After` header into seconds. @@ -18,19 +36,19 @@ const DEFAULT_USER_AGENT = "registries/0.1.0"; * Returns 60 when the header is absent, empty, or unparseable. */ export function parseRetryAfter(header: string | null | undefined): number { - if (!header) return 60; - const trimmed = header.trim(); - if (!trimmed) return 60; + return parseRetryAfterValue(header) ?? 60; +} - if (/^\d+$/.test(trimmed)) return Number(trimmed); +/** Apply a valid, timer-safe `Retry-After` value or reject an unschedulable delay. */ +export function retryDelayFor(header: string | null | undefined, fallbackDelay: number): number { + const retryAfter = parseRetryAfterValue(header); + if (retryAfter === undefined) return fallbackDelay; - const timestamp = /[a-z]/i.test(trimmed) ? Date.parse(trimmed) : NaN; - if (!Number.isNaN(timestamp)) { - const seconds = Math.ceil((timestamp - Date.now()) / 1000); - return Math.max(seconds, 0); + const retryAfterDelay = retryAfter * 1000; + if (!Number.isFinite(retryAfterDelay) || retryAfterDelay > MAX_TIMER_DELAY) { + throw new RateLimitError(retryAfter); } - - return 60; + return Math.max(fallbackDelay, retryAfterDelay); } /** HTTP client with retry, backoff, rate limiting, and timeout. */ @@ -58,8 +76,16 @@ export class Client { const remaining = typeof context.options.retry === "number" ? context.options.retry : 0; const attempt = maxRetries - remaining; const delay = baseDelay * Math.pow(2, attempt - 1); - const jitter = delay * Math.random() * 0.1; - return delay + jitter; + const jitteredDelay = delay + delay * Math.random() * 0.1; + const retryAfter = context.response?.headers.get("Retry-After"); + try { + return retryDelayFor(retryAfter, jitteredDelay); + } catch (error) { + if (error instanceof RateLimitError && context.response?.status !== 429) { + throw new HTTPError(context.response?.status ?? 0, String(context.request), ""); + } + throw error; + } }, retryStatusCodes: [408, 409, 425, 429, 500, 502, 503, 504], timeout: this.timeout, diff --git a/test/e2e/client.test.ts b/test/e2e/client.test.ts new file mode 100644 index 0000000..f61f000 --- /dev/null +++ b/test/e2e/client.test.ts @@ -0,0 +1,58 @@ +import { createServer } from "node:http"; +import { Client } from "../../src/core/client.ts"; +import { HTTPError } from "../../src/core/errors.ts"; + +describe("Client", () => { + it("honors Retry-After without misclassifying server errors", async () => { + let requests = 0; + let responseStatus: number | null = 429; + let retryAfter = "1"; + const server = createServer((_request, response) => { + requests++; + if (responseStatus !== null) { + const status = responseStatus; + responseStatus = null; + response.writeHead(status, { Connection: "close", "Retry-After": retryAfter }); + response.end(); + return; + } + + response.writeHead(200, { Connection: "close", "Content-Type": "application/json" }); + response.end('{"ok":true}'); + }); + + await new Promise((resolve, reject) => { + const onError = (error: Error) => reject(error); + server.once("error", onError); + server.listen(0, "127.0.0.1", () => { + server.off("error", onError); + resolve(); + }); + }); + + try { + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Expected TCP server address"); + const url = `http://127.0.0.1:${address.port}`; + + const startedAt = performance.now(); + await new Client({ maxRetries: 1, baseDelay: 10 }).getJSON(url); + expect(performance.now() - startedAt).toBeGreaterThanOrEqual(900); + expect(requests).toBe(2); + + responseStatus = 503; + // 2_147_484 seconds converts to 2_147_484_000 ms, just above Node's timer ceiling. + retryAfter = "2147484"; + const failure = new Client({ maxRetries: 1, baseDelay: 10 }).getJSON(url); + await expect(failure).rejects.toMatchObject({ + name: HTTPError.name, + statusCode: 503, + }); + expect(requests).toBe(3); + } finally { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + }); +}); diff --git a/test/unit/client.test.ts b/test/unit/client.test.ts index 44c8073..9e1c22e 100644 --- a/test/unit/client.test.ts +++ b/test/unit/client.test.ts @@ -1,4 +1,5 @@ -import { parseRetryAfter } from "../../src/core/client.ts"; +import { parseRetryAfter, retryDelayFor } from "../../src/core/client.ts"; +import { RateLimitError } from "../../src/core/errors.ts"; describe("parseRetryAfter", () => { it("parses numeric seconds", () => { @@ -60,3 +61,19 @@ describe("parseRetryAfter", () => { expect(parseRetryAfter(" ")).toBe(60); }); }); + +describe("retryDelayFor", () => { + it("falls back for invalid values", () => { + expect(retryDelayFor("not-a-delay", 75)).toBe(75); + }); + + it("keeps a longer local backoff", () => { + expect(retryDelayFor("0", 75)).toBe(75); + }); + + it("rejects values above the timer limit", () => { + // Node timers accept at most 2_147_483_647 ms, so these straddle that ceiling in seconds. + expect(retryDelayFor("2147483", 10)).toBe(2_147_483_000); + expect(() => retryDelayFor("2147484", 10)).toThrow(RateLimitError); + }); +});