From f1623604d8eccaeef333290dd224a01278e49d9e Mon Sep 17 00:00:00 2001 From: Aldrich_CC <109075336+Chen17-sq@users.noreply.github.com> Date: Sat, 16 May 2026 13:38:05 +0800 Subject: [PATCH] fix(proxy/ProviderClient): honour Retry-After on 429 in callProviderWithRetry Closes #5672. valhalla/jawn's ProviderClient retried 429 responses with the same exponentialDelay it uses for 5xx, never consulting the Retry-After header. With async-retry's defaults this means three retries fire well within most providers' cooldown windows, each one earning another 429. Add a small parseRetryAfter helper that accepts the RFC 7231 delta-seconds and HTTP-date forms, returns the wait in milliseconds, and caps the result at 60 s so a hostile or misconfigured provider cannot freeze the proxy for an unbounded amount of time. Past dates and malformed values return null and the caller falls back to its existing backoff. Inside the retry callback, after detecting a 429, the proxy now sleeps for the parsed Retry-After value (when present) before throwing, so async-retry's exponential backoff applies on top of the provider-mandated minimum rather than racing it. 5xx behaviour is unchanged. The worker variant at worker/src/lib/clients/ProviderClient.ts has the same retry pattern and would benefit from the same treatment in a follow-up; this PR stays narrowly scoped to the file the issue cites. --- valhalla/jawn/src/lib/proxy/ProviderClient.ts | 51 +++++++++++++++++++ .../proxy/__tests__/ProviderClient.test.ts | 50 ++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 valhalla/jawn/src/lib/proxy/__tests__/ProviderClient.test.ts diff --git a/valhalla/jawn/src/lib/proxy/ProviderClient.ts b/valhalla/jawn/src/lib/proxy/ProviderClient.ts index a181f68cbe..752f233de3 100644 --- a/valhalla/jawn/src/lib/proxy/ProviderClient.ts +++ b/valhalla/jawn/src/lib/proxy/ProviderClient.ts @@ -66,6 +66,45 @@ export function buildTargetUrl(originalUrl: URL, apiBase: string): URL { return new URL(`${apiBaseUrl.origin}${pathname}${originalUrl.search}`); } +// Hard cap so a hostile or misconfigured provider cannot make the proxy wait +// for an unbounded amount of time before retrying. +const MAX_RETRY_AFTER_MS = 60_000; + +/** + * Parse an RFC 7231 `Retry-After` header value into milliseconds. + * + * Accepts either a non-negative integer number of seconds or an HTTP-date. + * Returns `null` when the header is absent, malformed, or already in the past + * — in which case the caller should fall back to its default backoff strategy. + * The returned value is clamped to {@link MAX_RETRY_AFTER_MS} to prevent abuse. + */ +export function parseRetryAfter( + value: string | null | undefined, + now: number = Date.now() +): number | null { + if (!value) return null; + const trimmed = value.trim(); + if (trimmed === "") return null; + + // delta-seconds form (RFC 7231 §7.1.3 first alternative) + if (/^\d+$/.test(trimmed)) { + const seconds = Number.parseInt(trimmed, 10); + if (Number.isNaN(seconds) || seconds < 0) return null; + return Math.min(seconds * 1000, MAX_RETRY_AFTER_MS); + } + + // HTTP-date form (RFC 7231 §7.1.3 second alternative) + const date = Date.parse(trimmed); + if (Number.isNaN(date)) return null; + const delta = date - now; + if (delta <= 0) return null; + return Math.min(delta, MAX_RETRY_AFTER_MS); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + export async function callProviderWithRetry( callProps: CallProps, retryOptions: RetryOptions @@ -82,6 +121,18 @@ export async function callProviderWithRetry( lastResponse = res; // Throw an error if the status code is 429 if (res.status === 429 || res.status === 500 || res.status === 522) { + // For 429 the provider can explicitly tell us how long to wait via + // the Retry-After header. Honour it as a minimum delay before the + // next retry kicks in; async-retry's exponential backoff still + // applies on top, so this only ever lengthens the wait. + if (res.status === 429) { + const retryAfterMs = parseRetryAfter( + res.headers.get("retry-after") + ); + if (retryAfterMs !== null && retryAfterMs > 0) { + await sleep(retryAfterMs); + } + } throw new Error(`Status code ${res.status}`); } return res; diff --git a/valhalla/jawn/src/lib/proxy/__tests__/ProviderClient.test.ts b/valhalla/jawn/src/lib/proxy/__tests__/ProviderClient.test.ts new file mode 100644 index 0000000000..384300d9a8 --- /dev/null +++ b/valhalla/jawn/src/lib/proxy/__tests__/ProviderClient.test.ts @@ -0,0 +1,50 @@ +import { parseRetryAfter } from "../ProviderClient"; + +describe("parseRetryAfter", () => { + // Frozen reference point so the HTTP-date cases are deterministic. + const NOW = Date.UTC(2026, 4, 16, 12, 0, 0); // 2026-05-16T12:00:00Z + + it("returns null for missing or empty values", () => { + expect(parseRetryAfter(null)).toBeNull(); + expect(parseRetryAfter(undefined)).toBeNull(); + expect(parseRetryAfter("")).toBeNull(); + expect(parseRetryAfter(" ")).toBeNull(); + }); + + it("parses delta-seconds form into milliseconds", () => { + expect(parseRetryAfter("0")).toBe(0); + expect(parseRetryAfter("1")).toBe(1_000); + expect(parseRetryAfter("30")).toBe(30_000); + }); + + it("trims surrounding whitespace before parsing seconds", () => { + expect(parseRetryAfter(" 5 ")).toBe(5_000); + }); + + it("caps very large delta-seconds at 60 seconds", () => { + // 1 hour Retry-After should be clamped so a hostile provider cannot freeze + // the proxy for an unbounded amount of time. + expect(parseRetryAfter("3600")).toBe(60_000); + }); + + it("rejects non-numeric, non-date strings", () => { + expect(parseRetryAfter("soon")).toBeNull(); + expect(parseRetryAfter("1.5")).toBeNull(); + expect(parseRetryAfter("-1")).toBeNull(); + }); + + it("parses HTTP-date form into milliseconds from `now`", () => { + const fiveSecondsFromNow = new Date(NOW + 5_000).toUTCString(); + expect(parseRetryAfter(fiveSecondsFromNow, NOW)).toBe(5_000); + }); + + it("returns null for HTTP-date values in the past", () => { + const tenSecondsAgo = new Date(NOW - 10_000).toUTCString(); + expect(parseRetryAfter(tenSecondsAgo, NOW)).toBeNull(); + }); + + it("caps HTTP-date values far in the future at 60 seconds", () => { + const oneHourFromNow = new Date(NOW + 3_600_000).toUTCString(); + expect(parseRetryAfter(oneHourFromNow, NOW)).toBe(60_000); + }); +});