fix(api): dress only the 429s a limiter actually refused

The request-scoped capture kept the last verdict of any kind and fell back
to it when nothing had refused. That fallback fires in exactly one state —
no limiter refused this request — which is the definition of "this 429 came
from somewhere else". So a daily AI budget that is spent until midnight, an
hourly generation quota, and a provider-side 429 relayed onward all went out
carrying our own bucket's numbers: a remainder that says there is room left
and a Retry-After measured against an unrelated window. On the AI paths the
captured window can also expire while the provider call runs, so the dressed
delay could land at zero and tell the client to retry immediately into the
ceiling that had just refused it.

The cell now keeps refusals only. A request every bucket let through reports
nothing, and a 429 from another cause leaves undressed, which is correct:
there is no bucket to describe. The two-limiter rule is unchanged — the last
refusal wins, and a refusal still outranks a pass.

The unit case that pinned the fallback is turned round, and both directions
now run through the real wrapper: a limiter refusal gets the headers, a 429
raised without one gets none. The integration file gains the two shapes it
was missing — a route that consults two buckets must report the one that
refused, and a route whose 429 comes from the daily budget must carry no
rate-limit headers at all.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 04:55:19 +02:00
1 parent 21dd7c7e48
commit dd3844a7a2
5 files changed
+305 -23

No files matched your search

@@ -0,0 +1,191 @@
/**
* Which 429s `apiHandler` is allowed to dress with rate-limit headers.
*
* The wrapper attaches `Retry-After` and the `X-RateLimit` triple to any 429
* that leaves a handler without them, and it takes the numbers from the
* request-scoped limiter capture. That is only honest when a limiter actually
* refused: an AI budget that is spent for the day, an hourly generation quota
* and a provider-side 429 relayed onward are all 429s no bucket produced, and
* describing them with a bucket that still has room tells the client to retry
* into a ceiling that has not moved.
*
* So both directions are pinned here: a refusal from the limiter gets the
* headers, a 429 from anything else gets none.
*/
import { NextRequest, NextResponse } from "next/server";
import { describe, expect, it, vi } from "vitest";
// --- Mocks must be hoisted before importing the module under test. ---
vi.mock("@/lib/db", () => ({
prisma: {},
}));
vi.mock("@/lib/auth/session", () => ({
getSession: vi.fn(),
}));
vi.mock("@/lib/auth/hmac", () => ({
hashToken: vi.fn(),
}));
vi.mock("@/lib/auth/audit", () => ({
auditLog: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("@/lib/logging/transports", () => ({
emitIfSampled: vi.fn(),
}));
vi.mock("next/headers", () => ({
headers: vi.fn(async () => ({ get: () => null })),
cookies: vi.fn(async () => ({
get: () => undefined,
set: () => {},
delete: () => {},
})),
}));
import { apiHandler } from "@/lib/api-handler";
import { captureRateLimitResult } from "@/lib/rate-limit-context";
const RATE_LIMIT_HEADERS = [
"Retry-After",
"X-RateLimit-Limit",
"X-RateLimit-Remaining",
"X-RateLimit-Reset",
] as const;
function refusal(): NextResponse {
return NextResponse.json({ data: null, error: "Too many" }, { status: 429 });
}
function request(): NextRequest {
return new NextRequest("http://localhost/api/test", { method: "POST" });
}
describe("apiHandler — 429 header dressing", () => {
it("dresses a 429 the limiter refused", async () => {
const resetAt = Date.now() + 30_000;
const route = apiHandler(async (_request: NextRequest) => {
captureRateLimitResult({
allowed: false,
limit: 60,
remaining: 0,
resetAt,
});
return refusal();
});
const res = await route(request());
expect(res.status).toBe(429);
expect(res.headers.get("X-RateLimit-Limit")).toBe("60");
expect(res.headers.get("X-RateLimit-Remaining")).toBe("0");
expect(res.headers.get("X-RateLimit-Reset")).toBe(
new Date(resetAt).toISOString(),
);
expect(Number(res.headers.get("Retry-After"))).toBeGreaterThan(0);
});
it("leaves a 429 no limiter produced undressed", async () => {
const route = apiHandler(async (_request: NextRequest) => refusal());
const res = await route(request());
expect(res.status).toBe(429);
for (const name of RATE_LIMIT_HEADERS) {
expect(res.headers.get(name)).toBeNull();
}
});
it("leaves a 429 undressed when every bucket the handler consulted passed", async () => {
// The shape of the AI surfaces: a per-minute limiter lets the request
// through, then a daily budget or an upstream provider refuses it. The
// window the limiter cleared says nothing about when the budget resets.
const route = apiHandler(async (_request: NextRequest) => {
captureRateLimitResult({
allowed: true,
limit: 10,
remaining: 9,
resetAt: Date.now() + 60_000,
});
return refusal();
});
const res = await route(request());
expect(res.status).toBe(429);
for (const name of RATE_LIMIT_HEADERS) {
expect(res.headers.get(name)).toBeNull();
}
});
it("reports the bucket that refused when the handler consulted two", async () => {
const resetAt = Date.now() + 45_000;
const route = apiHandler(async (_request: NextRequest) => {
captureRateLimitResult({
allowed: true,
limit: 600,
remaining: 599,
resetAt: Date.now() + 3_600_000,
});
captureRateLimitResult({
allowed: false,
limit: 5,
remaining: 0,
resetAt,
});
return refusal();
});
const res = await route(request());
expect(res.headers.get("X-RateLimit-Limit")).toBe("5");
expect(res.headers.get("X-RateLimit-Reset")).toBe(
new Date(resetAt).toISOString(),
);
});
it("keeps the headers a handler attached itself", async () => {
const route = apiHandler(async (_request: NextRequest) => {
captureRateLimitResult({
allowed: false,
limit: 60,
remaining: 0,
resetAt: Date.now() + 30_000,
});
return NextResponse.json(
{ data: null, error: "Too many" },
{
status: 429,
headers: { "Retry-After": "900", "X-RateLimit-Limit": "3" },
},
);
});
const res = await route(request());
expect(res.headers.get("Retry-After")).toBe("900");
expect(res.headers.get("X-RateLimit-Limit")).toBe("3");
});
it("leaves a non-429 response alone even after a refusal was captured", async () => {
const route = apiHandler(async (_request: NextRequest) => {
captureRateLimitResult({
allowed: false,
limit: 60,
remaining: 0,
resetAt: Date.now() + 30_000,
});
return NextResponse.json({ data: { ok: true }, error: null });
});
const res = await route(request());
expect(res.status).toBe(200);
for (const name of RATE_LIMIT_HEADERS) {
expect(res.headers.get(name)).toBeNull();
}
});
});
+10 -10
View File
@@ -7,9 +7,10 @@
* plain integers, and the reset instant in the ISO form it has always carried.
*
* The capture half is what lets `apiHandler` dress a 429 that a handler built
* without headers. Its one interesting rule is that a refusal outranks a pass:
* a handler that clears one bucket and is refused by the next must report the
* bucket that refused it.
* without headers. It keeps refusals and nothing else: a handler that clears
* one bucket and is refused by the next reports the bucket that refused it,
* and a request every bucket let through reports nothing at all, because a
* 429 it ends up answering came from some other ceiling.
*/
import { describe, expect, it } from "vitest";
@@ -78,7 +79,7 @@ describe("request-scoped rate-limit capture", () => {
expect(capturedRateLimit()).toBeNull();
});
it("reports the last verdict when every bucket passed", () => {
it("reports nothing when every bucket passed, so a 429 from elsewhere stays undressed", () => {
runWithRateLimitCapture(() => {
captureRateLimitResult({
allowed: true,
@@ -92,12 +93,11 @@ describe("request-scoped rate-limit capture", () => {
remaining: 8,
resetAt: 2_000,
});
expect(capturedRateLimit()).toEqual({
allowed: true,
limit: 9,
remaining: 8,
resetAt: 2_000,
});
// Passing every bucket is exactly the state in which a 429 did not come
// from the limiter. Rendering the passing verdict would put
// `X-RateLimit-Remaining: 8` beside a refusal and a delay measured
// against a window that has nothing to do with it.
expect(capturedRateLimit()).toBeNull();
});
});
+5
View File
@@ -380,6 +380,11 @@ export function apiHandler<T extends (...args: any[]) => Promise<Response>>(
* `Retry-After` is the presence test rather than a flag: a handler that built
* its own headers already carries it (they come from the same builder), so
* this leaves that response exactly as it was written.
*
* The cell holds refusals only, so a 429 no limiter produced — a spent daily
* AI budget, an hourly generation quota, a provider-side 429 relayed onward —
* reads null here and goes out undressed. There is no bucket to describe, and
* one that still has room would name a delay against the wrong window.
*/
function attachRateLimitHeaders(response: NextResponse): void {
if (response.status !== 429) return;
+17 -9
View File
@@ -1,5 +1,5 @@
/**
* Request-scoped capture of the rate-limiter's last verdict, plus the one
* Request-scoped capture of the rate-limiter's refusals, plus the one
* builder that turns a verdict into response headers — a LEAF module.
*
* It lives apart from `rate-limit.ts` on purpose. Nearly two hundred route
@@ -29,7 +29,6 @@ export interface RateLimitSnapshot {
}
interface RateLimitCapture {
last: RateLimitSnapshot | null;
denied: RateLimitSnapshot | null;
}
@@ -41,30 +40,39 @@ const storage = new AsyncLocalStorage<RateLimitCapture>();
* {@link capturedRateLimit} reads null.
*/
export function runWithRateLimitCapture<T>(fn: () => T): T {
return storage.run({ last: null, denied: null }, fn);
return storage.run({ denied: null }, fn);
}
/**
* Record a limiter verdict for the request in flight.
*
* A denied verdict wins over an allowed one: a handler that consults two
* buckets and is refused by the second must report the bucket that actually
* refused it, not the one it passed.
* Only refusals are kept. A verdict that let the request through describes a
* bucket that still has room, and there is no 429 it could honestly explain:
* if the response ends up being a 429 anyway, some other ceiling produced it.
* The last refusal wins, since a handler that consults two buckets refuses at
* the first denial it acts on.
*/
export function captureRateLimitResult(
result: RateLimitSnapshot & { allowed: boolean },
): void {
const store = storage.getStore();
if (!store) return;
store.last = result;
if (!result.allowed) store.denied = result;
}
/** The verdict to render for this request, or null when none was taken. */
/**
* The refusal to render for this request, or null when no limiter refused it.
*
* Null is the answer for a request every bucket let through, so a 429 raised
* by something else — a daily AI budget, an hourly generation quota, a
* provider-side 429 relayed onward — is left undressed rather than labelled
* with a bucket that has room left and a delay measured against an unrelated
* window.
*/
export function capturedRateLimit(): RateLimitSnapshot | null {
const store = storage.getStore();
if (!store) return null;
return store.denied ?? store.last;
return store.denied;
}
/**