From dd8a724039c1470dafa2b0594e4ed5849fed8b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 04:55:47 +0200 Subject: [PATCH] fix(api): floor Retry-After at one second on a refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resetAt` is written by Postgres as `NOW() + interval` and subtracted from the app process's clock, so a row that still refuses can read as already expired here. The old floor let that render as `Retry-After: 0`, which tells a client to retry immediately into the ceiling that just turned it away — a contradiction of the status it rides on, and a hot loop for anything that honours the header. One second is the honest minimum on a 429. The rounding-up rule and the reason for it are unchanged; only the floor moves. --- src/lib/__tests__/rate-limit-headers.test.ts | 7 +++++-- src/lib/rate-limit-context.ts | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/lib/__tests__/rate-limit-headers.test.ts b/src/lib/__tests__/rate-limit-headers.test.ts index 51938a67c..248cd2aad 100644 --- a/src/lib/__tests__/rate-limit-headers.test.ts +++ b/src/lib/__tests__/rate-limit-headers.test.ts @@ -48,13 +48,16 @@ describe("rateLimitResponseHeaders", () => { expect(headers["Retry-After"]).toBe("1"); }); - it("never names a delay in the past", () => { + it("names at least a second even when the window already reads as expired", () => { const now = Date.UTC(2026, 0, 1, 12, 0, 0); const headers = rateLimitResponseHeaders( { limit: 10, remaining: 0, resetAt: now - 5_000 }, now, ); - expect(headers["Retry-After"]).toBe("0"); + // `resetAt` is the database clock and `now` is this process's, so a row + // that still refuses can read as expired here. Zero would say "retry now" + // beside a refusal and turn the client into a hot loop. + expect(headers["Retry-After"]).toBe("1"); }); it("keeps X-RateLimit-Reset in the ISO form already on the wire", () => { diff --git a/src/lib/rate-limit-context.ts b/src/lib/rate-limit-context.ts index 75df1b2a3..fd1181233 100644 --- a/src/lib/rate-limit-context.ts +++ b/src/lib/rate-limit-context.ts @@ -80,9 +80,11 @@ export function capturedRateLimit(): RateLimitSnapshot | null { * * `Retry-After` is whole seconds and rounds UP, per RFC 9110 §10.2.3: a client * that waits the value it is given must find the window already rolled over, - * which flooring cannot promise. It never goes below zero — a window that - * expired between the verdict and the response says "retry now" rather than - * naming a time in the past. + * which flooring cannot promise. It never goes below one second: `resetAt` + * comes from the database clock and the subtraction happens on the app + * process's, so a window can read as already expired here while the row still + * refuses; and zero on a refusal means "retry now", which contradicts the + * status it rides on and invites a hot loop. * * `X-RateLimit-Reset` keeps the ISO-8601 instant it has always carried. It is * the wrong convention for the header name and every other deployment uses @@ -96,7 +98,7 @@ export function rateLimitResponseHeaders( ): Record { return { "Retry-After": String( - Math.max(0, Math.ceil((snapshot.resetAt - now) / 1000)), + Math.max(1, Math.ceil((snapshot.resetAt - now) / 1000)), ), "X-RateLimit-Limit": String(snapshot.limit), "X-RateLimit-Remaining": String(snapshot.remaining),