From c640056724d48dffe2b32f227804c7975858c8d3 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 19 Oct 2026 23:47:49 -0400 Subject: [PATCH 1/3] feat(core): recognize HTTP 429 and surface the server's Retry-After A throttled request surfaced as the SDK's bare "Error POSTing to endpoint", indistinguishable from any other non-OK status. Annotate a 429 on the same request funnel the #2318 timeout diagnostics use, so all three clients say it is rate limiting, which request was throttled, and when the server asked to retry. The SDK's SdkHttpError drops response headers, so Retry-After comes from the fetch tracker. On the web path the upstream fetch runs in the backend, which relays the header on the typed send error rather than leaving the browser to race a fetch event against the send response. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- .../web/src/test/core/mcp/rateLimit.test.ts | 214 ++++++++++++++ .../mcp/inspectorClient-rate-limit.test.ts | 263 ++++++++++++++++++ core/mcp/inspectorClient.ts | 31 ++- core/mcp/rateLimit.ts | 173 ++++++++++++ core/mcp/remote/node/remote-session.ts | 18 ++ core/mcp/remote/node/server.ts | 7 + core/mcp/remote/remoteClientTransport.ts | 1 + core/mcp/remote/types.ts | 5 + 8 files changed, 705 insertions(+), 7 deletions(-) create mode 100644 clients/web/src/test/core/mcp/rateLimit.test.ts create mode 100644 clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts create mode 100644 core/mcp/rateLimit.ts diff --git a/clients/web/src/test/core/mcp/rateLimit.test.ts b/clients/web/src/test/core/mcp/rateLimit.test.ts new file mode 100644 index 0000000000..3e26174639 --- /dev/null +++ b/clients/web/src/test/core/mcp/rateLimit.test.ts @@ -0,0 +1,214 @@ +import { describe, it, expect } from "vitest"; +import { + SdkError, + SdkErrorCode, + SdkHttpError, +} from "@modelcontextprotocol/client"; +import { + annotateRateLimit, + isRateLimitError, + parseRetryAfter, + rateLimitResponseOf, + retryAfterSince, + type AnnotatedRateLimitData, +} from "@inspector/core/mcp/rateLimit.js"; +import type { FetchRequestEntryBase } from "@inspector/core/mcp/types.js"; + +const NOW = Date.parse("2026-10-19T12:00:00Z"); + +function entry( + overrides: Partial = {}, +): FetchRequestEntryBase { + return { + id: "fetch-1", + timestamp: new Date(NOW), + method: "POST", + url: "http://localhost/mcp", + requestHeaders: {}, + responseStatus: 429, + responseHeaders: { "Retry-After": "30" }, + ...overrides, + }; +} + +function throttled(data: Record = {}): SdkHttpError { + return new SdkHttpError( + SdkErrorCode.ClientHttpNotImplemented, + "Error POSTing to endpoint: slow down", + { status: 429, statusText: "Too Many Requests", ...data }, + ); +} + +describe("rateLimitResponseOf", () => { + it("records a 429's Retry-After, whatever its casing, and when it was sent", () => { + expect(rateLimitResponseOf(entry())).toEqual({ + retryAfter: "30", + sentAt: NOW, + }); + }); + + it("records a 429 with no Retry-After", () => { + expect(rateLimitResponseOf(entry({ responseHeaders: {} }))).toEqual({ + sentAt: NOW, + }); + }); + + it("ignores anything that is not a 429", () => { + expect(rateLimitResponseOf(entry({ responseStatus: 503 }))).toBeUndefined(); + expect( + rateLimitResponseOf(entry({ responseStatus: undefined })), + ).toBeUndefined(); + }); +}); + +describe("retryAfterSince", () => { + it("returns the header of a 429 sent at or after the bound", () => { + const latest = { retryAfter: "30", sentAt: NOW }; + expect(retryAfterSince(latest, NOW)).toBe("30"); + expect(retryAfterSince(latest, NOW - 1)).toBe("30"); + }); + + it("drops a 429 that predates the request", () => { + expect(retryAfterSince({ retryAfter: "30", sentAt: NOW }, NOW + 1)).toBe( + undefined, + ); + }); + + it("returns nothing when no 429 was recorded", () => { + expect(retryAfterSince(undefined, NOW)).toBeUndefined(); + }); +}); + +describe("parseRetryAfter", () => { + it("reads delta-seconds", () => { + expect(parseRetryAfter("30", NOW)).toBe(30_000); + expect(parseRetryAfter(" 0 ", NOW)).toBe(0); + }); + + it("reads an HTTP-date as the delay from now", () => { + expect(parseRetryAfter("Mon, 19 Oct 2026 12:01:30 GMT", NOW)).toBe(90_000); + }); + + it("clamps a date already past to zero", () => { + expect(parseRetryAfter("Mon, 19 Oct 2026 11:00:00 GMT", NOW)).toBe(0); + }); + + it("treats a missing or malformed value as absent", () => { + expect(parseRetryAfter(undefined, NOW)).toBeUndefined(); + expect(parseRetryAfter("soon", NOW)).toBeUndefined(); + // `Date.parse` alone reads these as dates. + expect(parseRetryAfter("-5", NOW)).toBeUndefined(); + expect(parseRetryAfter("1.5", NOW)).toBeUndefined(); + expect(parseRetryAfter("2026-10-19T12:01:30Z", NOW)).toBeUndefined(); + // The right shape, but no real date. + expect(parseRetryAfter("Mon, 99 Foo 2026 12:00:00 GMT", NOW)).toBe( + undefined, + ); + }); +}); + +describe("isRateLimitError", () => { + it("matches only an SdkHttpError with status 429", () => { + expect(isRateLimitError(throttled())).toBe(true); + expect(isRateLimitError(throttled({ status: 503 }))).toBe(false); + expect( + isRateLimitError(new SdkError(SdkErrorCode.RequestTimeout, "timed out")), + ).toBe(false); + expect( + isRateLimitError(Object.assign(new Error("x"), { status: 429 })), + ).toBe(false); + expect(isRateLimitError(undefined)).toBe(false); + }); +}); + +describe("annotateRateLimit", () => { + it("names the method and the retry delay, keeping class, code, status and cause", () => { + const original = throttled(); + const annotated = annotateRateLimit(original, "tools/call", "30", NOW); + + expect(SdkHttpError.isInstance(annotated)).toBe(true); + const error = annotated as SdkHttpError; + expect(error.message).toBe( + "Rate limited by the server (HTTP 429) on tools/call. " + + "The server asked to retry after 30s. " + + "Error POSTing to endpoint: slow down", + ); + expect(error.code).toBe(SdkErrorCode.ClientHttpNotImplemented); + expect(error.status).toBe(429); + expect(error.statusText).toBe("Too Many Requests"); + expect(error.cause).toBe(original); + const data = error.data as AnnotatedRateLimitData; + expect(data.method).toBe("tools/call"); + expect(data.retryAfter).toBe("30"); + expect(data.retryAfterMs).toBe(30_000); + }); + + it("prefers a Retry-After the backend relayed on the error", () => { + const annotated = annotateRateLimit( + throttled({ retryAfter: "5" }), + "tools/list", + "30", + NOW, + ) as SdkHttpError; + expect(annotated.message).toContain("retry after 5s"); + expect((annotated.data as AnnotatedRateLimitData).retryAfter).toBe("5"); + }); + + it("ignores a relayed retryAfter that is not a string", () => { + const annotated = annotateRateLimit( + throttled({ retryAfter: 5 }), + "tools/list", + "30", + NOW, + ) as SdkHttpError; + expect(annotated.message).toContain("retry after 30s"); + }); + + it("says when the server sent no Retry-After", () => { + const annotated = annotateRateLimit( + throttled(), + "tools/list", + undefined, + NOW, + ) as SdkHttpError; + expect(annotated.message).toContain( + "The server sent no Retry-After header.", + ); + const data = annotated.data as AnnotatedRateLimitData; + expect(data.retryAfter).toBeUndefined(); + expect(data.retryAfterMs).toBeUndefined(); + }); + + it("reports an unparseable Retry-After verbatim rather than guessing", () => { + const annotated = annotateRateLimit( + throttled(), + "tools/list", + "soon", + NOW, + ) as SdkHttpError; + expect(annotated.message).toContain( + "The server sent an unparseable Retry-After (soon).", + ); + const data = annotated.data as AnnotatedRateLimitData; + expect(data.retryAfter).toBe("soon"); + expect(data.retryAfterMs).toBeUndefined(); + }); + + it("defaults the clock to now", () => { + const annotated = annotateRateLimit( + throttled(), + "tools/list", + "1", + ) as SdkHttpError; + expect(annotated.message).toContain("retry after 1s"); + }); + + it("returns anything that is not a 429 untouched", () => { + const unavailable = throttled({ status: 503 }); + expect(annotateRateLimit(unavailable, "tools/list", "30", NOW)).toBe( + unavailable, + ); + const timeout = new SdkError(SdkErrorCode.RequestTimeout, "timed out"); + expect(annotateRateLimit(timeout, "tools/list", "30", NOW)).toBe(timeout); + }); +}); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts new file mode 100644 index 0000000000..0a0e1379e2 --- /dev/null +++ b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts @@ -0,0 +1,263 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + createServer, + type IncomingMessage, + type Server, + type ServerResponse, +} from "node:http"; +import { serve, type ServerType } from "@hono/node-server"; +import { SdkErrorCode, SdkHttpError } from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; +import { createRemoteTransport } from "@inspector/core/mcp/remote/createRemoteTransport.js"; +import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js"; +import { + eraToVersionNegotiation, + type CreateTransport, +} from "@inspector/core/mcp/types.js"; +import type { AnnotatedRateLimitData } from "@inspector/core/mcp/rateLimit.js"; + +/** + * Rate-limit recognition against a real connection (#2442). + * + * A server that answers `initialize` and then throttles `tools/list` with a + * `429`. The SDK's `SdkHttpError` keeps the status but drops the response + * headers, so the `Retry-After` in the message can only have come from the + * fetch tracker — directly on a Node transport, and relayed on the send error + * by the web backend on the remote one. Both are driven end to end here. + * + * Hand-rolled rather than a composable test server because the SDK's own + * server cannot be configured to answer one method with a 429. + */ + +interface ToolsListReply { + status: number; + headers?: Record; + body: string; +} + +interface ThrottlingServer { + url: string; + setToolsListReply: (reply: ToolsListReply) => void; + stop: () => Promise; +} + +const THROTTLED: ToolsListReply = { + status: 429, + headers: { "retry-after": "30" }, + body: "slow down", +}; + +function startThrottlingServer(): Promise { + let toolsListReply = THROTTLED; + + const handler = async (req: IncomingMessage, res: ServerResponse) => { + // No standalone notification stream; nothing here needs one. + if (req.method === "GET") { + res.writeHead(405).end(); + return; + } + if (req.method === "DELETE") { + res.writeHead(200).end(); + return; + } + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(chunk as Buffer); + const body = JSON.parse(Buffer.concat(chunks).toString()) as { + id?: unknown; + method: string; + }; + if (body.id === undefined) { + res.writeHead(202).end(); + return; + } + if (body.method === "tools/list") { + res.writeHead(toolsListReply.status, { + "content-type": "text/plain", + ...toolsListReply.headers, + }); + res.end(toolsListReply.body); + return; + } + const result = + body.method === "initialize" + ? { + protocolVersion: "2025-06-18", + capabilities: { tools: {} }, + serverInfo: { name: "throttling-server", version: "1.0.0" }, + } + : {}; + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ jsonrpc: "2.0", id: body.id, result })); + }; + + const server: Server = createServer((req, res) => { + void handler(req, res); + }); + + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + /* v8 ignore next -- listen() on a fresh server always yields an AddressInfo */ + const port = typeof address === "object" && address ? address.port : 0; + resolve({ + url: `http://127.0.0.1:${port}/mcp`, + setToolsListReply: (reply) => { + toolsListReply = reply; + }, + stop: () => + new Promise((done) => { + server.closeAllConnections(); + server.close(() => done()); + }), + }); + }); + }); +} + +/** The web backend on an ephemeral port, as the browser's remote transport sees it. */ +function startRemoteBackend(): Promise<{ + server: ServerType; + transport: CreateTransport; +}> { + const { app, authToken } = createRemoteApp({ + initialConfig: { defaultEnvironment: {} }, + }); + return new Promise((resolve, reject) => { + const server = serve( + { fetch: app.fetch, port: 0, hostname: "127.0.0.1" }, + (info) => { + resolve({ + server, + transport: createRemoteTransport({ + baseUrl: `http://127.0.0.1:${info.port}`, + authToken, + }), + }); + }, + ); + server.on("error", reject); + }); +} + +async function rejectionOf(promise: Promise): Promise { + try { + await promise; + } catch (err) { + return err; + } + throw new Error("expected the request to reject"); +} + +describe("InspectorClient rate-limit recognition (#2442)", () => { + let client: InspectorClient | null = null; + let upstream: ThrottlingServer | null = null; + let backend: ServerType | null = null; + + async function connectTo( + url: string, + transport: CreateTransport = createTransportNode, + ): Promise { + client = new InspectorClient( + { type: "streamable-http", url }, + { + environment: { transport }, + // Pinned legacy so the connect is exactly `initialize`. + versionNegotiation: eraToVersionNegotiation("legacy"), + }, + ); + await client.connect(); + return client; + } + + afterEach(async () => { + try { + await client?.disconnect(); + } catch { + // Teardown only — the assertions already ran. + } + client = null; + await upstream?.stop(); + upstream = null; + if (backend) { + const closing = backend; + await new Promise((done) => closing.close(() => done())); + backend = null; + } + }); + + function expectThrottled(caught: unknown): AnnotatedRateLimitData { + expect(SdkHttpError.isInstance(caught)).toBe(true); + const error = caught as SdkHttpError; + // Same class, code and status as the SDK's own error: only the message + // and the data grew. + expect(error.code).toBe(SdkErrorCode.ClientHttpNotImplemented); + expect(error.status).toBe(429); + return error.data as AnnotatedRateLimitData; + } + + it("names the throttled request and the server's Retry-After", async () => { + upstream = await startThrottlingServer(); + const c = await connectTo(upstream.url); + + const caught = await rejectionOf(c.listTools()); + + const data = expectThrottled(caught); + expect((caught as Error).message).toBe( + "Rate limited by the server (HTTP 429) on tools/list. " + + "The server asked to retry after 30s. " + + "Error POSTing to endpoint: slow down", + ); + expect(data.method).toBe("tools/list"); + expect(data.retryAfter).toBe("30"); + expect(data.retryAfterMs).toBe(30_000); + expect(SdkHttpError.isInstance((caught as Error).cause)).toBe(true); + }); + + it("says so when the server sent no Retry-After", async () => { + upstream = await startThrottlingServer(); + upstream.setToolsListReply({ status: 429, body: "slow down" }); + const c = await connectTo(upstream.url); + + const caught = await rejectionOf(c.listTools()); + + const data = expectThrottled(caught); + expect((caught as Error).message).toContain( + "The server sent no Retry-After header.", + ); + expect(data.retryAfter).toBeUndefined(); + }); + + it("leaves a non-429 failure exactly as the SDK threw it", async () => { + upstream = await startThrottlingServer(); + upstream.setToolsListReply({ + status: 503, + headers: { "retry-after": "30" }, + body: "down for maintenance", + }); + const c = await connectTo(upstream.url); + + const caught = await rejectionOf(c.listTools()); + + expect(SdkHttpError.isInstance(caught)).toBe(true); + expect((caught as SdkHttpError).status).toBe(503); + expect((caught as Error).message).toBe( + "Error POSTing to endpoint: down for maintenance", + ); + }); + + it("relays the Retry-After through the web backend", async () => { + upstream = await startThrottlingServer(); + const started = await startRemoteBackend(); + backend = started.server; + const c = await connectTo(upstream.url, started.transport); + + const caught = await rejectionOf(c.listTools()); + + const data = expectThrottled(caught); + expect((caught as Error).message).toMatch( + /^Rate limited by the server \(HTTP 429\) on tools\/list\. The server asked to retry after 30s\. /, + ); + expect(data.retryAfter).toBe("30"); + }); +}); diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 818c4f60eb..10f6187cef 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -296,6 +296,12 @@ import { type NotificationStreamState, type OutstandingRequest, } from "./connectionDiagnostics.js"; +import { + annotateRateLimit, + rateLimitResponseOf, + retryAfterSince, + type RateLimitResponse, +} from "./rateLimit.js"; import { OAuthManager, type OAuthManagerConfig } from "./oauthManager.js"; import { RemoteClientTransport } from "./remote/remoteClientTransport.js"; @@ -555,6 +561,9 @@ export class InspectorClient extends InspectorClientEventTarget { // `GET` on Streamable HTTP). Both per-session, reset with the maps above. private lastResponse: LastResponse | undefined; private notificationStream: TrackedNotificationStream | undefined; + // The most recent `429` a transport fetch drew, kept for its `Retry-After` + // (#2442). Per-session, reset with the diagnostics above. + private lastRateLimitResponse: RateLimitResponse | undefined; /** True when the cached transport was built with an OAuth authProvider attached. */ private transportHasAuthProvider = false; /** Dedupes concurrent ambient auth challenges (reason + scopes). */ @@ -1071,7 +1080,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.clientInfo, Object.keys(clientOptions).length > 0 ? clientOptions : undefined, ); - this.annotateSdkRequestTimeouts(this.client); + this.annotateSdkRequestErrors(this.client); if (this.tasksCapabilityAdvertised) { this.bindReceiverTasks(); } @@ -1080,7 +1089,8 @@ export class InspectorClient extends InspectorClientEventTarget { /** * Decorate the SDK Client's `request` so a per-request timeout rejects with * the connection's state at that moment instead of a bare `Request timed - * out` (#2318). + * out` (#2318), and a `429` rejects as rate limiting with the server's + * `Retry-After` instead of a bare `Error POSTing to endpoint` (#2442). * * `Protocol.request` is the one funnel every SDK verb goes through — * `listTools`, `callTool`, `readResource`, the handshake's `initialize`, @@ -1098,19 +1108,23 @@ export class InspectorClient extends InspectorClientEventTarget { * implementation signature serves both at runtime; the single cast on the * way back is what the overloads cost. */ - private annotateSdkRequestTimeouts(client: Client): void { + private annotateSdkRequestErrors(client: Client): void { type SdkRequest = Client["request"]; const sdkRequest: SdkRequest = client.request.bind(client); const decorated = async ( ...args: Parameters ): Promise => { + // A 429 is matched to this request by time: only one drawn by a fetch + // sent after the request started can be the response that failed it. + const sentAt = Date.now(); try { return await sdkRequest(...args); } catch (err) { - throw annotateRequestTimeout( - err, - args[0].method, - this.getConnectionDiagnostics(), + const method = args[0].method; + throw annotateRateLimit( + annotateRequestTimeout(err, method, this.getConnectionDiagnostics()), + method, + retryAfterSince(this.lastRateLimitResponse, sentAt), ); } }; @@ -1673,6 +1687,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.taskProgress.clear(); this.lastResponse = undefined; this.notificationStream = undefined; + this.lastRateLimitResponse = undefined; this.dispatchConnectionDiagnosticsChange(); // Per-session for the same reason: both name entries of the PREVIOUS // server's list. Cleared here as well as in `disconnect()` because the @@ -1813,6 +1828,8 @@ export class InspectorClient extends InspectorClientEventTarget { onFetchRequest: (entry: FetchRequestEntryBase) => { this.dispatchFetchRequest({ ...entry, category: "transport" }); this.noteTransportStream(entry); + this.lastRateLimitResponse = + rateLimitResponseOf(entry) ?? this.lastRateLimitResponse; }, onFetchResponseBody: (id: string, body: string) => { this.dispatchFetchRequestBodyUpdate(id, body); diff --git a/core/mcp/rateLimit.ts b/core/mcp/rateLimit.ts new file mode 100644 index 0000000000..c20235d7fa --- /dev/null +++ b/core/mcp/rateLimit.ts @@ -0,0 +1,173 @@ +/** + * Rate-limit recognition: the one place that turns a server's `HTTP 429` into + * a message saying the server is throttling, and when it says to retry + * (#2442). + * + * Before this, a throttled request surfaced as the SDK's + * `Error POSTing to endpoint: ` — the same shape as every other non-OK + * status, so telling "the server is down" from "the server is pacing me" was + * left to the reader. Both facts needed to say more were already in-process: + * the error carries the status, and the fetch tracker recorded the response's + * `Retry-After` header. + * + * The header has to come from the tracker because the SDK's `SdkHttpError` + * keeps the status and the body but none of the response headers. Where the + * upstream fetch runs in the web backend, the backend reads the same header + * off its own tracked fetch and relays it on the send error as `retryAfter` + * (see `RemoteHttpErrorData`), so the browser does not depend on a fetch + * event racing the send's HTTP response across two connections. + * + * Everything here is pure. `InspectorClient` owns the bookkeeping, the same + * split `connectionDiagnostics.ts` makes for request timeouts. + * + * Deliberately narrow: only a 429 is recognized. A 503 also carries + * `Retry-After` but means "unavailable", which is exactly the case this exists + * to tell apart from throttling. + */ + +import { + SdkHttpError, + type SdkHttpErrorData, +} from "@modelcontextprotocol/client"; +import { findHeader } from "./fetchTracking.js"; +import { formatDuration } from "./connectionDiagnostics.js"; +import type { FetchRequestEntryBase } from "./types.js"; + +/** `429 Too Many Requests` (RFC 6585). */ +export const HTTP_TOO_MANY_REQUESTS = 429; + +/** A rate-limited response the fetch tracker recorded. */ +export interface RateLimitResponse { + /** The raw `Retry-After` header, when the server sent one. */ + retryAfter?: string; + /** Epoch ms at which the request that drew it went out. */ + sentAt: number; +} + +/** + * The rate-limit facts in a tracked fetch, or `undefined` when the response + * was not a 429 (or the fetch failed before any response arrived). + */ +export function rateLimitResponseOf( + entry: FetchRequestEntryBase, +): RateLimitResponse | undefined { + if (entry.responseStatus !== HTTP_TOO_MANY_REQUESTS) return undefined; + const retryAfter = findHeader(entry.responseHeaders, "retry-after"); + return { + ...(retryAfter !== undefined && { retryAfter }), + sentAt: entry.timestamp.getTime(), + }; +} + +/** + * The latest recorded 429's `Retry-After`, when that 429 answered a request + * sent at or after `since`. The bound is what keeps a header from a 429 + * minutes ago from being reported against a request it never answered. + */ +export function retryAfterSince( + latest: RateLimitResponse | undefined, + since: number, +): string | undefined { + if (!latest || latest.sentAt < since) return undefined; + return latest.retryAfter; +} + +/** + * The IMF-fixdate form of an HTTP-date (`Sun, 06 Nov 1994 08:49:37 GMT`), + * which RFC 9110 §5.6.7 requires senders to use. Matched before `Date.parse` + * because `Date.parse` is lenient past usefulness: it reads `-5` as the year + * 5 BC, which would report a malformed header as "retry now". + */ +const IMF_FIXDATE = + /^[A-Z][a-z]{2}, \d{2} [A-Z][a-z]{2} \d{4} \d{2}:\d{2}:\d{2} GMT$/; + +/** + * A `Retry-After` value as a delay in ms from `now` (RFC 9110 §10.2.3): either + * a non-negative integer count of seconds, or an HTTP-date. A date already in + * the past is a delay of `0`. Anything else is `undefined` — a malformed header + * is reported verbatim rather than guessed at. + */ +export function parseRetryAfter( + value: string | undefined, + now: number, +): number | undefined { + if (value === undefined) return undefined; + const trimmed = value.trim(); + if (/^\d+$/.test(trimmed)) return Number(trimmed) * 1000; + if (!IMF_FIXDATE.test(trimmed)) return undefined; + const at = Date.parse(trimmed); + // A well-formed shape can still name no real date (`Mon, 99 Foo 2026 …`). + if (Number.isNaN(at)) return undefined; + return Math.max(0, at - now); +} + +/** Whether `err` is the SDK's HTTP error for a `429 Too Many Requests`. */ +export function isRateLimitError(err: unknown): err is SdkHttpError { + return SdkHttpError.isInstance(err) && err.status === HTTP_TOO_MANY_REQUESTS; +} + +/** + * The data carried by an annotated rate-limit error: the SDK's own HTTP data, + * plus the method that was throttled and the retry timing when the server + * gave one. Exposed so a consumer that wants the structured form (a countdown, + * a CLI's JSON output) can read it off `error.data` instead of parsing the + * message. + */ +export interface AnnotatedRateLimitData extends SdkHttpErrorData { + method: string; + /** The raw `Retry-After` header, when the server sent one. */ + retryAfter?: string; + /** The parsed delay, when `retryAfter` was well-formed. */ + retryAfterMs?: number; +} + +function relayedRetryAfter(err: SdkHttpError): string | undefined { + const value = err.data.retryAfter; + return typeof value === "string" ? value : undefined; +} + +function describeRetry( + retryAfter: string | undefined, + retryAfterMs: number | undefined, +): string { + if (retryAfterMs !== undefined) { + return `The server asked to retry after ${formatDuration(retryAfterMs)}.`; + } + if (retryAfter !== undefined) { + return `The server sent an unparseable Retry-After (${retryAfter}).`; + } + return "The server sent no Retry-After header."; +} + +/** + * Rebuild a 429 `SdkHttpError` with a message that names it as rate limiting, + * says which request was throttled, and gives the retry timing when the server + * sent one. + * + * `retryAfter` is the header the caller tracked; a value the backend relayed on + * the error itself takes precedence, since it was read off the very response + * that produced this error. + * + * Anything that is not a 429 is returned untouched, so this is safe to apply + * blindly on the rejection path of every request. The result keeps the + * original's code and HTTP data, so a status or code check downstream sees + * what it saw before; the original is kept as `cause`. + */ +export function annotateRateLimit( + err: unknown, + method: string, + retryAfter: string | undefined, + now: number = Date.now(), +): unknown { + if (!isRateLimitError(err)) return err; + const header = relayedRetryAfter(err) ?? retryAfter; + const retryAfterMs = parseRetryAfter(header, now); + const data: AnnotatedRateLimitData = { + ...err.data, + method, + ...(header !== undefined && { retryAfter: header }), + ...(retryAfterMs !== undefined && { retryAfterMs }), + }; + const message = `Rate limited by the server (HTTP 429) on ${method}. ${describeRetry(header, retryAfterMs)} ${err.message}`; + return new SdkHttpError(err.code, message, data, { cause: err }); +} diff --git a/core/mcp/remote/node/remote-session.ts b/core/mcp/remote/node/remote-session.ts index cb0b16f890..c337659245 100644 --- a/core/mcp/remote/node/remote-session.ts +++ b/core/mcp/remote/node/remote-session.ts @@ -11,6 +11,11 @@ import { AuthChallengeError } from "../../../auth/challenge.js"; import type { RemoteAuthProviderHandle } from "./tokenAuthProvider.js"; import type { RemoteAuthState } from "../types.js"; import { progressTokenOf, waitForProgressToken } from "../progressToken.js"; +import { + rateLimitResponseOf, + retryAfterSince, + type RateLimitResponse, +} from "../../rateLimit.js"; export interface SessionEvent { type: RemoteEvent["type"]; @@ -47,6 +52,8 @@ export class RemoteSession { private readonly requestWaits = new Map(); private authProviderHandle: RemoteAuthProviderHandle | null = null; private appliedProtocolVersion: string | undefined; + /** The most recent `429` an upstream fetch drew, for its `Retry-After` (#2442). */ + private lastRateLimitResponse: RateLimitResponse | undefined; /** A protocol version is a dated token (`2025-11-25`, `2026-07-28`, …). */ private static readonly PROTOCOL_VERSION_PATTERN = /^[A-Za-z0-9._-]{1,64}$/; @@ -310,6 +317,8 @@ export class RemoteSession { } onFetchRequest(entry: FetchRequestEntryBase): void { + this.lastRateLimitResponse = + rateLimitResponseOf(entry) ?? this.lastRateLimitResponse; this.pushEvent({ type: "fetch_request", data: { @@ -322,6 +331,15 @@ export class RemoteSession { }); } + /** + * The `Retry-After` of the latest upstream `429` drawn by a fetch sent at or + * after `since` — read by the send route, so the header rides back on the + * send's own error rather than on a fetch event racing it (#2442). + */ + retryAfterSince(since: number): string | undefined { + return retryAfterSince(this.lastRateLimitResponse, since); + } + onFetchResponseBody(id: string, responseBody: string): void { this.pushEvent({ type: "fetch_request_body_update", diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 0c677fb367..154ad507dc 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -30,6 +30,7 @@ import { createTransportNode } from "../../node/transport.js"; import { createProxyFetch } from "../../node/proxyFetch.js"; import { OAUTH_TIMEOUT_WIRE_CODE } from "../../../auth/requestTimeout.js"; import { redactUrlQuery } from "../../fetchTracking.js"; +import { isRateLimitError } from "../../rateLimit.js"; import type { RemoteConnectRequest, RemoteSendRequest, @@ -1045,6 +1046,8 @@ export function createRemoteApp( // on this request and everything after it (#1935). session.applyProtocolVersion(protocolVersion); + // Bounds which upstream 429 can be this send's (#2442). + const sendStartedAt = Date.now(); session.beginSend(); const requestId = requestIdForSendWait(message); let responseWait: Promise | undefined; @@ -1115,6 +1118,9 @@ export function createRemoteApp( // browser rebuilds an `SdkHttpError` the era probe can classify (#2660). if (err instanceof SdkHttpError) { const text = err.data.text; + const retryAfter = isRateLimitError(err) + ? session.retryAfterSince(sendStartedAt) + : undefined; return c.json({ ok: false, kind: "transport_error", @@ -1126,6 +1132,7 @@ export function createRemoteApp( statusText: err.statusText, }), ...(typeof text === "string" && { text }), + ...(retryAfter !== undefined && { retryAfter }), }, }); } diff --git a/core/mcp/remote/remoteClientTransport.ts b/core/mcp/remote/remoteClientTransport.ts index 9f9a5462f2..6339ebbda2 100644 --- a/core/mcp/remote/remoteClientTransport.ts +++ b/core/mcp/remote/remoteClientTransport.ts @@ -182,6 +182,7 @@ function toSdkHttpError( status: http.status, ...(http.statusText !== undefined && { statusText: http.statusText }), ...(http.text !== undefined && { text: http.text }), + ...(http.retryAfter !== undefined && { retryAfter: http.retryAfter }), }); } diff --git a/core/mcp/remote/types.ts b/core/mcp/remote/types.ts index 006d16d04a..6caa918d47 100644 --- a/core/mcp/remote/types.ts +++ b/core/mcp/remote/types.ts @@ -118,6 +118,11 @@ export interface RemoteHttpErrorData { statusText?: string; /** The upstream response body, which the probe parses for a JSON-RPC error. */ text?: string; + /** + * The upstream `Retry-After` header on a `429`, which the SDK's error does + * not carry; read off the backend's own tracked fetch (#2442). + */ + retryAfter?: string; } export type RemoteSendResponse = From ea35ae36d6e286354f17ef25e8aefda264ca50e2 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 20 Oct 2026 00:00:07 -0400 Subject: [PATCH 2/3] fix(core): correlate each 429's Retry-After by JSON-RPC id, not by time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "The latest 429 since the request started" let a concurrent request's response overwrite another's: request A, whose 429 body was still in flight, reported request B's Retry-After. File each 429's header under the request id(s) in its tracked request body, and stamp it onto the error at the point that knows which message failed — trackSendFailure directly, the send route on the web backend. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- .../web/src/test/core/mcp/rateLimit.test.ts | 168 ++++++++++++------ .../mcp/inspectorClient-rate-limit.test.ts | 94 +++++++++- core/mcp/inspectorClient.ts | 25 ++- core/mcp/rateLimit.ts | 132 +++++++++----- core/mcp/remote/node/remote-session.ts | 23 +-- core/mcp/remote/node/server.ts | 14 +- 6 files changed, 330 insertions(+), 126 deletions(-) diff --git a/clients/web/src/test/core/mcp/rateLimit.test.ts b/clients/web/src/test/core/mcp/rateLimit.test.ts index 3e26174639..2cdf8870c9 100644 --- a/clients/web/src/test/core/mcp/rateLimit.test.ts +++ b/clients/web/src/test/core/mcp/rateLimit.test.ts @@ -6,10 +6,12 @@ import { } from "@modelcontextprotocol/client"; import { annotateRateLimit, + attachRetryAfter, isRateLimitError, + MAX_UNCLAIMED_RATE_LIMITS, parseRetryAfter, - rateLimitResponseOf, - retryAfterSince, + RateLimitLedger, + requestIdsOf, type AnnotatedRateLimitData, } from "@inspector/core/mcp/rateLimit.js"; import type { FetchRequestEntryBase } from "@inspector/core/mcp/types.js"; @@ -25,6 +27,11 @@ function entry( method: "POST", url: "http://localhost/mcp", requestHeaders: {}, + requestBody: JSON.stringify({ + jsonrpc: "2.0", + id: 7, + method: "tools/list", + }), responseStatus: 429, responseHeaders: { "Retry-After": "30" }, ...overrides, @@ -39,43 +46,115 @@ function throttled(data: Record = {}): SdkHttpError { ); } -describe("rateLimitResponseOf", () => { - it("records a 429's Retry-After, whatever its casing, and when it was sent", () => { - expect(rateLimitResponseOf(entry())).toEqual({ - retryAfter: "30", - sentAt: NOW, - }); +describe("requestIdsOf", () => { + it("reads a single request's id, string or number", () => { + expect( + requestIdsOf('{"jsonrpc":"2.0","id":7,"method":"tools/list"}'), + ).toEqual([7]); + expect(requestIdsOf('{"jsonrpc":"2.0","id":"a","method":"x"}')).toEqual([ + "a", + ]); }); - it("records a 429 with no Retry-After", () => { - expect(rateLimitResponseOf(entry({ responseHeaders: {} }))).toEqual({ - sentAt: NOW, - }); + it("reads every request in a batch, skipping notifications and junk", () => { + expect( + requestIdsOf( + JSON.stringify([ + { jsonrpc: "2.0", id: 1, method: "a" }, + { jsonrpc: "2.0", method: "notifications/x" }, + { jsonrpc: "2.0", id: 2, result: {} }, + { jsonrpc: "2.0", id: null, method: "b" }, + null, + { jsonrpc: "2.0", id: 3, method: "c" }, + ]), + ), + ).toEqual([1, 3]); }); - it("ignores anything that is not a 429", () => { - expect(rateLimitResponseOf(entry({ responseStatus: 503 }))).toBeUndefined(); - expect( - rateLimitResponseOf(entry({ responseStatus: undefined })), - ).toBeUndefined(); + it("finds nothing in a missing or non-JSON body", () => { + expect(requestIdsOf(undefined)).toEqual([]); + expect(requestIdsOf("not json")).toEqual([]); + expect(requestIdsOf("42")).toEqual([]); }); }); -describe("retryAfterSince", () => { - it("returns the header of a 429 sent at or after the bound", () => { - const latest = { retryAfter: "30", sentAt: NOW }; - expect(retryAfterSince(latest, NOW)).toBe("30"); - expect(retryAfterSince(latest, NOW - 1)).toBe("30"); +describe("RateLimitLedger", () => { + it("files a 429's header, whatever its casing, under its request id, once", () => { + const ledger = new RateLimitLedger(); + ledger.record(entry()); + expect(ledger.take(7)).toBe("30"); + expect(ledger.take(7)).toBeUndefined(); }); - it("drops a 429 that predates the request", () => { - expect(retryAfterSince({ retryAfter: "30", sentAt: NOW }, NOW + 1)).toBe( - undefined, + it("keeps each overlapping request's own header", () => { + const ledger = new RateLimitLedger(); + ledger.record(entry()); + ledger.record( + entry({ + requestBody: JSON.stringify({ jsonrpc: "2.0", id: 8, method: "x" }), + responseHeaders: { "retry-after": "5" }, + }), ); + expect(ledger.take(7)).toBe("30"); + expect(ledger.take(8)).toBe("5"); }); - it("returns nothing when no 429 was recorded", () => { - expect(retryAfterSince(undefined, NOW)).toBeUndefined(); + it("files a 429 with no header as having none", () => { + const ledger = new RateLimitLedger(); + ledger.record(entry({ responseHeaders: {} })); + expect(ledger.take(7)).toBeUndefined(); + }); + + it("ignores anything that is not a 429", () => { + const ledger = new RateLimitLedger(); + ledger.record(entry({ responseStatus: 503 })); + ledger.record(entry({ responseStatus: undefined })); + expect(ledger.take(7)).toBeUndefined(); + }); + + it("drops the oldest unclaimed 429 past its cap", () => { + const ledger = new RateLimitLedger(); + for (let id = 0; id <= MAX_UNCLAIMED_RATE_LIMITS; id++) { + ledger.record( + entry({ + requestBody: JSON.stringify({ jsonrpc: "2.0", id, method: "x" }), + }), + ); + } + expect(ledger.take(0)).toBeUndefined(); + expect(ledger.take(1)).toBe("30"); + expect(ledger.take(MAX_UNCLAIMED_RATE_LIMITS)).toBe("30"); + }); + + it("forgets everything on clear", () => { + const ledger = new RateLimitLedger(); + ledger.record(entry()); + ledger.clear(); + expect(ledger.take(7)).toBeUndefined(); + }); +}); + +describe("attachRetryAfter", () => { + it("stamps the header onto a 429's data", () => { + const err = throttled(); + attachRetryAfter(err, "30"); + expect(err.data.retryAfter).toBe("30"); + }); + + it("keeps a header already relayed on the error", () => { + const err = throttled({ retryAfter: "5" }); + attachRetryAfter(err, "30"); + expect(err.data.retryAfter).toBe("5"); + }); + + it("leaves a non-429, or a missing header, alone", () => { + const unavailable = throttled({ status: 503 }); + attachRetryAfter(unavailable, "30"); + expect(unavailable.data.retryAfter).toBeUndefined(); + const err = throttled(); + attachRetryAfter(err, undefined); + expect(err.data).not.toHaveProperty("retryAfter"); + expect(() => attachRetryAfter(undefined, "30")).not.toThrow(); }); }); @@ -123,8 +202,8 @@ describe("isRateLimitError", () => { describe("annotateRateLimit", () => { it("names the method and the retry delay, keeping class, code, status and cause", () => { - const original = throttled(); - const annotated = annotateRateLimit(original, "tools/call", "30", NOW); + const original = throttled({ retryAfter: "30" }); + const annotated = annotateRateLimit(original, "tools/call", NOW); expect(SdkHttpError.isInstance(annotated)).toBe(true); const error = annotated as SdkHttpError; @@ -143,32 +222,21 @@ describe("annotateRateLimit", () => { expect(data.retryAfterMs).toBe(30_000); }); - it("prefers a Retry-After the backend relayed on the error", () => { - const annotated = annotateRateLimit( - throttled({ retryAfter: "5" }), - "tools/list", - "30", - NOW, - ) as SdkHttpError; - expect(annotated.message).toContain("retry after 5s"); - expect((annotated.data as AnnotatedRateLimitData).retryAfter).toBe("5"); - }); - - it("ignores a relayed retryAfter that is not a string", () => { + it("ignores a retryAfter on the data that is not a string", () => { const annotated = annotateRateLimit( throttled({ retryAfter: 5 }), "tools/list", - "30", NOW, ) as SdkHttpError; - expect(annotated.message).toContain("retry after 30s"); + expect(annotated.message).toContain( + "The server sent no Retry-After header.", + ); }); it("says when the server sent no Retry-After", () => { const annotated = annotateRateLimit( throttled(), "tools/list", - undefined, NOW, ) as SdkHttpError; expect(annotated.message).toContain( @@ -181,9 +249,8 @@ describe("annotateRateLimit", () => { it("reports an unparseable Retry-After verbatim rather than guessing", () => { const annotated = annotateRateLimit( - throttled(), + throttled({ retryAfter: "soon" }), "tools/list", - "soon", NOW, ) as SdkHttpError; expect(annotated.message).toContain( @@ -196,19 +263,16 @@ describe("annotateRateLimit", () => { it("defaults the clock to now", () => { const annotated = annotateRateLimit( - throttled(), + throttled({ retryAfter: "1" }), "tools/list", - "1", ) as SdkHttpError; expect(annotated.message).toContain("retry after 1s"); }); it("returns anything that is not a 429 untouched", () => { const unavailable = throttled({ status: 503 }); - expect(annotateRateLimit(unavailable, "tools/list", "30", NOW)).toBe( - unavailable, - ); + expect(annotateRateLimit(unavailable, "tools/list", NOW)).toBe(unavailable); const timeout = new SdkError(SdkErrorCode.RequestTimeout, "timed out"); - expect(annotateRateLimit(timeout, "tools/list", "30", NOW)).toBe(timeout); + expect(annotateRateLimit(timeout, "tools/list", NOW)).toBe(timeout); }); }); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts index 0a0e1379e2..86e826732a 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts @@ -27,7 +27,8 @@ import type { AnnotatedRateLimitData } from "@inspector/core/mcp/rateLimit.js"; * by the web backend on the remote one. Both are driven end to end here. * * Hand-rolled rather than a composable test server because the SDK's own - * server cannot be configured to answer one method with a 429. + * server cannot be configured to answer one method with a 429, let alone hold + * one 429's body back while another overtakes it. */ interface ToolsListReply { @@ -39,9 +40,19 @@ interface ToolsListReply { interface ThrottlingServer { url: string; setToolsListReply: (reply: ToolsListReply) => void; + /** + * The methods of the overlapping pair (`prompts/list`, `resources/list`), in + * the order they arrived. The first is answered `429` + `Retry-After: 30` + * with its body held back; the second `429` + `Retry-After: 5` at once. + */ + overlapArrivals: () => string[]; + /** Finish the held first response of the overlapping pair. */ + releaseFirstOverlap: () => void; stop: () => Promise; } +const OVERLAPPING = new Set(["prompts/list", "resources/list"]); + const THROTTLED: ToolsListReply = { status: 429, headers: { "retry-after": "30" }, @@ -50,6 +61,8 @@ const THROTTLED: ToolsListReply = { function startThrottlingServer(): Promise { let toolsListReply = THROTTLED; + const overlapArrivals: string[] = []; + let releaseFirstOverlap = () => {}; const handler = async (req: IncomingMessage, res: ServerResponse) => { // No standalone notification stream; nothing here needs one. @@ -79,11 +92,28 @@ function startThrottlingServer(): Promise { res.end(toolsListReply.body); return; } + if (OVERLAPPING.has(body.method)) { + overlapArrivals.push(body.method); + const first = overlapArrivals.length === 1; + res.writeHead(429, { + "content-type": "text/plain", + "retry-after": first ? "30" : "5", + }); + if (first) { + // Headers out now, so the 429 is tracked; the SDK's error waits on + // the body, which only the test releases. + res.flushHeaders(); + releaseFirstOverlap = () => res.end("first"); + } else { + res.end("second"); + } + return; + } const result = body.method === "initialize" ? { protocolVersion: "2025-06-18", - capabilities: { tools: {} }, + capabilities: { tools: {}, prompts: {}, resources: {} }, serverInfo: { name: "throttling-server", version: "1.0.0" }, } : {}; @@ -105,6 +135,8 @@ function startThrottlingServer(): Promise { setToolsListReply: (reply) => { toolsListReply = reply; }, + overlapArrivals: () => [...overlapArrivals], + releaseFirstOverlap: () => releaseFirstOverlap(), stop: () => new Promise((done) => { server.closeAllConnections(); @@ -140,6 +172,21 @@ function startRemoteBackend(): Promise<{ }); } +/** Poll until `predicate` holds; the failure names what never became true. */ +async function waitUntil( + what: string, + predicate: () => boolean, + timeoutMs = 5000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error(`timed out waiting for ${what}`); + } + await new Promise((r) => setTimeout(r, 10)); + } +} + async function rejectionOf(promise: Promise): Promise { try { await promise; @@ -260,4 +307,47 @@ describe("InspectorClient rate-limit recognition (#2442)", () => { ); expect(data.retryAfter).toBe("30"); }); + + /** + * Two requests throttled at once, the first one's response overtaken by + * the second's. Each must report its own response's `Retry-After`; picking + * "the latest 429" would hand the first request the second's 5s. + */ + async function expectOverlapCorrelated(c: InspectorClient): Promise { + const first = rejectionOf(c.listPrompts()); + await waitUntil( + "the first request to arrive", + () => upstream!.overlapArrivals().length === 1, + ); + const second = await rejectionOf(c.listResources()); + upstream!.releaseFirstOverlap(); + const firstCaught = await first; + + expect(upstream!.overlapArrivals()).toEqual([ + "prompts/list", + "resources/list", + ]); + expect(expectThrottled(firstCaught).retryAfter).toBe("30"); + expect((firstCaught as Error).message).toContain( + "on prompts/list. The server asked to retry after 30s.", + ); + expect(expectThrottled(second).retryAfter).toBe("5"); + expect((second as Error).message).toContain( + "on resources/list. The server asked to retry after 5s.", + ); + } + + it("gives each of two overlapping 429s its own Retry-After", async () => { + upstream = await startThrottlingServer(); + await expectOverlapCorrelated(await connectTo(upstream.url)); + }); + + it("gives each of two overlapping 429s its own Retry-After through the web backend", async () => { + upstream = await startThrottlingServer(); + const started = await startRemoteBackend(); + backend = started.server; + await expectOverlapCorrelated( + await connectTo(upstream.url, started.transport), + ); + }); }); diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 10f6187cef..e9b026901b 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -298,9 +298,8 @@ import { } from "./connectionDiagnostics.js"; import { annotateRateLimit, - rateLimitResponseOf, - retryAfterSince, - type RateLimitResponse, + attachRetryAfter, + RateLimitLedger, } from "./rateLimit.js"; import { OAuthManager, type OAuthManagerConfig } from "./oauthManager.js"; import { RemoteClientTransport } from "./remote/remoteClientTransport.js"; @@ -561,9 +560,10 @@ export class InspectorClient extends InspectorClientEventTarget { // `GET` on Streamable HTTP). Both per-session, reset with the maps above. private lastResponse: LastResponse | undefined; private notificationStream: TrackedNotificationStream | undefined; - // The most recent `429` a transport fetch drew, kept for its `Retry-After` - // (#2442). Per-session, reset with the diagnostics above. - private lastRateLimitResponse: RateLimitResponse | undefined; + // Each tracked `429`'s `Retry-After`, by the JSON-RPC id that drew it, until + // that id's failed send claims it (#2442). Per-session, reset with the + // diagnostics above. + private readonly rateLimits = new RateLimitLedger(); /** True when the cached transport was built with an OAuth authProvider attached. */ private transportHasAuthProvider = false; /** Dedupes concurrent ambient auth challenges (reason + scopes). */ @@ -1114,9 +1114,6 @@ export class InspectorClient extends InspectorClientEventTarget { const decorated = async ( ...args: Parameters ): Promise => { - // A 429 is matched to this request by time: only one drawn by a fetch - // sent after the request started can be the response that failed it. - const sentAt = Date.now(); try { return await sdkRequest(...args); } catch (err) { @@ -1124,7 +1121,6 @@ export class InspectorClient extends InspectorClientEventTarget { throw annotateRateLimit( annotateRequestTimeout(err, method, this.getConnectionDiagnostics()), method, - retryAfterSince(this.lastRateLimitResponse, sentAt), ); } }; @@ -1192,6 +1188,10 @@ export class InspectorClient extends InspectorClientEventTarget { // saw as unanswered, in every timeout message and Connection Info // row, until the session resets (#2318). if (isRequestTimeoutError(error)) return; + // The SDK's 429 error carries no headers; this is the one point that + // knows which request it failed, so it is where that request's own + // `Retry-After` goes onto it (#2442). + attachRetryAfter(error, this.rateLimits.take(message.id)); if (this.outstandingRequests.delete(message.id)) { this.dispatchConnectionDiagnosticsChange(); } @@ -1687,7 +1687,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.taskProgress.clear(); this.lastResponse = undefined; this.notificationStream = undefined; - this.lastRateLimitResponse = undefined; + this.rateLimits.clear(); this.dispatchConnectionDiagnosticsChange(); // Per-session for the same reason: both name entries of the PREVIOUS // server's list. Cleared here as well as in `disconnect()` because the @@ -1828,8 +1828,7 @@ export class InspectorClient extends InspectorClientEventTarget { onFetchRequest: (entry: FetchRequestEntryBase) => { this.dispatchFetchRequest({ ...entry, category: "transport" }); this.noteTransportStream(entry); - this.lastRateLimitResponse = - rateLimitResponseOf(entry) ?? this.lastRateLimitResponse; + this.rateLimits.record(entry); }, onFetchResponseBody: (id: string, body: string) => { this.dispatchFetchRequestBodyUpdate(id, body); diff --git a/core/mcp/rateLimit.ts b/core/mcp/rateLimit.ts index c20235d7fa..1bbfe637d9 100644 --- a/core/mcp/rateLimit.ts +++ b/core/mcp/rateLimit.ts @@ -11,14 +11,20 @@ * `Retry-After` header. * * The header has to come from the tracker because the SDK's `SdkHttpError` - * keeps the status and the body but none of the response headers. Where the - * upstream fetch runs in the web backend, the backend reads the same header - * off its own tracked fetch and relays it on the send error as `retryAfter` - * (see `RemoteHttpErrorData`), so the browser does not depend on a fetch - * event racing the send's HTTP response across two connections. + * keeps the status and the body but none of the response headers. It is + * matched to its error by JSON-RPC id, not by time: a {@link RateLimitLedger} + * files each 429's header under the request id(s) in the tracked request body, + * and the code that sees the failed send — which knows the message it was + * sending — takes that id's header and stamps it onto the error's data as + * `retryAfter` ({@link attachRetryAfter}). Two requests throttled at once + * therefore each report their own server's word, whatever order their + * responses land in. * - * Everything here is pure. `InspectorClient` owns the bookkeeping, the same - * split `connectionDiagnostics.ts` makes for request timeouts. + * Where the upstream fetch runs in the web backend, the backend does the + * stamping from its own ledger and relays `retryAfter` on the send error (see + * `RemoteHttpErrorData`), so the browser does not depend on a fetch event + * racing the send's HTTP response across two connections. Either way, by the + * time {@link annotateRateLimit} sees the error, the header is on it. * * Deliberately narrow: only a 429 is recognized. A 503 also carries * `Retry-After` but means "unavailable", which is exactly the case this exists @@ -36,40 +42,72 @@ import type { FetchRequestEntryBase } from "./types.js"; /** `429 Too Many Requests` (RFC 6585). */ export const HTTP_TOO_MANY_REQUESTS = 429; -/** A rate-limited response the fetch tracker recorded. */ -export interface RateLimitResponse { - /** The raw `Retry-After` header, when the server sent one. */ - retryAfter?: string; - /** Epoch ms at which the request that drew it went out. */ - sentAt: number; +/** + * The JSON-RPC request ids in a tracked request body: one for a single + * request, several for a batch, none for a notification, a response, or a + * body that is not JSON. The tracker records the body redacted, but redaction + * masks only sensitive string fields, so `id` survives it. + */ +export function requestIdsOf(body: string | undefined): (string | number)[] { + if (body === undefined) return []; + let parsed: unknown; + try { + parsed = JSON.parse(body); + } catch { + return []; + } + const messages: unknown[] = Array.isArray(parsed) ? parsed : [parsed]; + const ids: (string | number)[] = []; + for (const message of messages) { + if (typeof message !== "object" || message === null) continue; + if (!("method" in message) || !("id" in message)) continue; + const { id } = message; + if (typeof id === "string" || typeof id === "number") ids.push(id); + } + return ids; } /** - * The rate-limit facts in a tracked fetch, or `undefined` when the response - * was not a 429 (or the fetch failed before any response arrived). + * How many unclaimed 429s a ledger holds before it drops the oldest. A 429 is + * normally claimed within the same tick its send fails; one that never is (a + * send aborted mid-flight is not reported as a failure) would otherwise stay + * for the rest of the session. */ -export function rateLimitResponseOf( - entry: FetchRequestEntryBase, -): RateLimitResponse | undefined { - if (entry.responseStatus !== HTTP_TOO_MANY_REQUESTS) return undefined; - const retryAfter = findHeader(entry.responseHeaders, "retry-after"); - return { - ...(retryAfter !== undefined && { retryAfter }), - sentAt: entry.timestamp.getTime(), - }; -} +export const MAX_UNCLAIMED_RATE_LIMITS = 100; /** - * The latest recorded 429's `Retry-After`, when that 429 answered a request - * sent at or after `since`. The bound is what keeps a header from a 429 - * minutes ago from being reported against a request it never answered. + * Each tracked 429's `Retry-After`, filed under the JSON-RPC id of the request + * that drew it, until the failed send for that id claims it. */ -export function retryAfterSince( - latest: RateLimitResponse | undefined, - since: number, -): string | undefined { - if (!latest || latest.sentAt < since) return undefined; - return latest.retryAfter; +export class RateLimitLedger { + // `undefined` is a real value here: a 429 that carried no header. + private readonly byRequestId = new Map(); + + /** File a tracked fetch, if it was a 429. Anything else is ignored. */ + record(entry: FetchRequestEntryBase): void { + if (entry.responseStatus !== HTTP_TOO_MANY_REQUESTS) return; + const retryAfter = findHeader(entry.responseHeaders, "retry-after"); + for (const id of requestIdsOf(entry.requestBody)) { + this.byRequestId.delete(id); + this.byRequestId.set(id, retryAfter); + if (this.byRequestId.size > MAX_UNCLAIMED_RATE_LIMITS) { + const [oldest] = this.byRequestId.keys(); + /* v8 ignore next -- size > 0 was just checked, so a first key exists */ + if (oldest !== undefined) this.byRequestId.delete(oldest); + } + } + } + + /** The `Retry-After` filed for `id`, removed as it is read. */ + take(id: string | number): string | undefined { + const retryAfter = this.byRequestId.get(id); + this.byRequestId.delete(id); + return retryAfter; + } + + clear(): void { + this.byRequestId.clear(); + } } /** @@ -121,11 +159,27 @@ export interface AnnotatedRateLimitData extends SdkHttpErrorData { retryAfterMs?: number; } -function relayedRetryAfter(err: SdkHttpError): string | undefined { +function retryAfterOn(err: SdkHttpError): string | undefined { const value = err.data.retryAfter; return typeof value === "string" ? value : undefined; } +/** + * Stamp a 429's own `Retry-After` onto its error's data, where + * {@link annotateRateLimit} reads it. A header already there wins: on the + * browser's remote transport the backend relayed it from the upstream + * response itself, which nothing on this side can know better. Anything that + * is not a 429, or a missing header, leaves the error untouched. + */ +export function attachRetryAfter( + err: unknown, + retryAfter: string | undefined, +): void { + if (!isRateLimitError(err) || retryAfter === undefined) return; + if (retryAfterOn(err) !== undefined) return; + err.data.retryAfter = retryAfter; +} + function describeRetry( retryAfter: string | undefined, retryAfterMs: number | undefined, @@ -144,9 +198,8 @@ function describeRetry( * says which request was throttled, and gives the retry timing when the server * sent one. * - * `retryAfter` is the header the caller tracked; a value the backend relayed on - * the error itself takes precedence, since it was read off the very response - * that produced this error. + * The header is read off the error's own data, where {@link attachRetryAfter} + * (or the backend relay) put it. * * Anything that is not a 429 is returned untouched, so this is safe to apply * blindly on the rejection path of every request. The result keeps the @@ -156,11 +209,10 @@ function describeRetry( export function annotateRateLimit( err: unknown, method: string, - retryAfter: string | undefined, now: number = Date.now(), ): unknown { if (!isRateLimitError(err)) return err; - const header = relayedRetryAfter(err) ?? retryAfter; + const header = retryAfterOn(err); const retryAfterMs = parseRetryAfter(header, now); const data: AnnotatedRateLimitData = { ...err.data, diff --git a/core/mcp/remote/node/remote-session.ts b/core/mcp/remote/node/remote-session.ts index c337659245..e81675d603 100644 --- a/core/mcp/remote/node/remote-session.ts +++ b/core/mcp/remote/node/remote-session.ts @@ -11,11 +11,7 @@ import { AuthChallengeError } from "../../../auth/challenge.js"; import type { RemoteAuthProviderHandle } from "./tokenAuthProvider.js"; import type { RemoteAuthState } from "../types.js"; import { progressTokenOf, waitForProgressToken } from "../progressToken.js"; -import { - rateLimitResponseOf, - retryAfterSince, - type RateLimitResponse, -} from "../../rateLimit.js"; +import { RateLimitLedger } from "../../rateLimit.js"; export interface SessionEvent { type: RemoteEvent["type"]; @@ -52,8 +48,8 @@ export class RemoteSession { private readonly requestWaits = new Map(); private authProviderHandle: RemoteAuthProviderHandle | null = null; private appliedProtocolVersion: string | undefined; - /** The most recent `429` an upstream fetch drew, for its `Retry-After` (#2442). */ - private lastRateLimitResponse: RateLimitResponse | undefined; + /** Each upstream `429`'s `Retry-After`, by the JSON-RPC id that drew it (#2442). */ + private readonly rateLimits = new RateLimitLedger(); /** A protocol version is a dated token (`2025-11-25`, `2026-07-28`, …). */ private static readonly PROTOCOL_VERSION_PATTERN = /^[A-Za-z0-9._-]{1,64}$/; @@ -317,8 +313,7 @@ export class RemoteSession { } onFetchRequest(entry: FetchRequestEntryBase): void { - this.lastRateLimitResponse = - rateLimitResponseOf(entry) ?? this.lastRateLimitResponse; + this.rateLimits.record(entry); this.pushEvent({ type: "fetch_request", data: { @@ -332,12 +327,12 @@ export class RemoteSession { } /** - * The `Retry-After` of the latest upstream `429` drawn by a fetch sent at or - * after `since` — read by the send route, so the header rides back on the - * send's own error rather than on a fetch event racing it (#2442). + * The `Retry-After` of the upstream `429` that answered request `id` — read + * by the send route, so the header rides back on the send's own error rather + * than on a fetch event racing it (#2442). */ - retryAfterSince(since: number): string | undefined { - return retryAfterSince(this.lastRateLimitResponse, since); + takeRetryAfter(id: string | number): string | undefined { + return this.rateLimits.take(id); } onFetchResponseBody(id: string, responseBody: string): void { diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 154ad507dc..576672445b 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -1046,8 +1046,6 @@ export function createRemoteApp( // on this request and everything after it (#1935). session.applyProtocolVersion(protocolVersion); - // Bounds which upstream 429 can be this send's (#2442). - const sendStartedAt = Date.now(); session.beginSend(); const requestId = requestIdForSendWait(message); let responseWait: Promise | undefined; @@ -1118,9 +1116,15 @@ export function createRemoteApp( // browser rebuilds an `SdkHttpError` the era probe can classify (#2660). if (err instanceof SdkHttpError) { const text = err.data.text; - const retryAfter = isRateLimitError(err) - ? session.retryAfterSince(sendStartedAt) - : undefined; + // The 429's own header, by this send's JSON-RPC id (#2442). Read off + // the message rather than `requestId`, which is unset for a request + // the route deliberately does not wait on (`subscriptions/listen`). + const sentId = "id" in message ? message.id : undefined; + const retryAfter = + isRateLimitError(err) && + (typeof sentId === "string" || typeof sentId === "number") + ? session.takeRetryAfter(sentId) + : undefined; return c.json({ ok: false, kind: "transport_error", From 4c02f4b92ed7f1c87042740c857374b8d2c41556 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 20 Oct 2026 00:45:00 -0400 Subject: [PATCH 3/3] fix(core): annotate 429s on the raw-wire path; reject impossible dates A modern-era tool call against a server advertising the tasks extension goes through ext-tasks' raw-wire channel, whose host hook applied only the timeout annotation, so its 429 kept the generic message. Both paths now share one annotateRequestError, and the hook is renamed annotateError. Date.parse rolls an impossible IMF-fixdate (31 Nov) over to a real one, so a Retry-After is accepted only if it prints back exactly as sent. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- .../extension/tasks/rawWireChannel.test.ts | 15 ++-- .../web/src/test/core/mcp/rateLimit.test.ts | 9 ++- .../mcp/inspectorClient-rate-limit.test.ts | 75 ++++++++++++++++++- core/extension/tasks/rawWireChannel.ts | 14 ++-- core/mcp/inspectorClient.ts | 23 ++++-- core/mcp/rateLimit.ts | 9 ++- 6 files changed, 117 insertions(+), 28 deletions(-) diff --git a/clients/web/src/test/core/extension/tasks/rawWireChannel.test.ts b/clients/web/src/test/core/extension/tasks/rawWireChannel.test.ts index eb2300cd41..af9a831691 100644 --- a/clients/web/src/test/core/extension/tasks/rawWireChannel.test.ts +++ b/clients/web/src/test/core/extension/tasks/rawWireChannel.test.ts @@ -23,7 +23,7 @@ function setup(overrides: Partial = {}) { transport: () => transport, defaultTimeoutMs: () => 1_000, resetTimeoutOnProgress: () => true, - annotateTimeout: (error) => error, + annotateError: (error) => error, ...overrides, }); return { channel, sent, transport }; @@ -100,8 +100,8 @@ describe("RawWireChannel", () => { it("times out with the annotated timeout as its cause, re-armed by progress", async () => { vi.useFakeTimers(); - const annotateTimeout = vi.fn((error: unknown) => error); - const { channel, sent } = setup({ annotateTimeout }); + const annotateError = vi.fn((error: unknown) => error); + const { channel, sent } = setup({ annotateError }); const promise = channel.dispatch({ method: "tools/call", params: { _meta: { progressToken: "p" } }, @@ -121,10 +121,7 @@ describe("RawWireChannel", () => { error instanceof DispatchError && (error.cause as { code?: unknown }).code !== undefined, ); - expect(annotateTimeout).toHaveBeenCalledWith( - expect.anything(), - "tools/call", - ); + expect(annotateError).toHaveBeenCalledWith(expect.anything(), "tools/call"); // stdio-style transport: the timeout reaches the server as a cancel. expect(sent[1]).toMatchObject({ method: "notifications/cancelled" }); }); @@ -166,8 +163,8 @@ describe("RawWireChannel", () => { it("annotates a send failure and rejects everything on rejectAll", async () => { const boom = new Error("closed"); - const annotateTimeout = vi.fn(() => boom); - const failing = setup({ annotateTimeout }); + const annotateError = vi.fn(() => boom); + const failing = setup({ annotateError }); failing.transport.send.mockRejectedValueOnce("raw failure"); await expect( failing.channel.dispatch({ method: "tasks/get" }), diff --git a/clients/web/src/test/core/mcp/rateLimit.test.ts b/clients/web/src/test/core/mcp/rateLimit.test.ts index 2cdf8870c9..8e3b628b9f 100644 --- a/clients/web/src/test/core/mcp/rateLimit.test.ts +++ b/clients/web/src/test/core/mcp/rateLimit.test.ts @@ -179,7 +179,14 @@ describe("parseRetryAfter", () => { expect(parseRetryAfter("-5", NOW)).toBeUndefined(); expect(parseRetryAfter("1.5", NOW)).toBeUndefined(); expect(parseRetryAfter("2026-10-19T12:01:30Z", NOW)).toBeUndefined(); - // The right shape, but no real date. + // The right shape, but no real date: `Date.parse` rolls an impossible + // day over (31 Nov → 1 Dec) and ignores a weekday that does not match. + expect(parseRetryAfter("Tue, 31 Nov 2026 12:00:00 GMT", NOW)).toBe( + undefined, + ); + expect(parseRetryAfter("Tue, 19 Oct 2026 12:01:30 GMT", NOW)).toBe( + undefined, + ); expect(parseRetryAfter("Mon, 99 Foo 2026 12:00:00 GMT", NOW)).toBe( undefined, ); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts index 86e826732a..ff21cd2617 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-rate-limit.test.ts @@ -84,6 +84,14 @@ function startThrottlingServer(): Promise { res.writeHead(202).end(); return; } + if (body.method === "tools/call") { + res.writeHead(THROTTLED.status, { + "content-type": "text/plain", + ...THROTTLED.headers, + }); + res.end(THROTTLED.body); + return; + } if (body.method === "tools/list") { res.writeHead(toolsListReply.status, { "content-type": "text/plain", @@ -109,6 +117,30 @@ function startThrottlingServer(): Promise { } return; } + if (body.method === "server/discover") { + // Only a modern-era connect sends this; the legacy-pinned cases never do. + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + jsonrpc: "2.0", + id: body.id, + result: { + resultType: "complete", + ttlMs: 0, + cacheScope: "public", + supportedVersions: ["2026-07-28"], + // The tasks extension is what routes a modern `tools/call` onto + // ext-tasks' raw-wire channel instead of the SDK's `request`. + capabilities: { + tools: {}, + extensions: { "io.modelcontextprotocol/tasks": {} }, + }, + serverInfo: { name: "throttling-server", version: "1.0.0" }, + }, + }), + ); + return; + } const result = body.method === "initialize" ? { @@ -204,13 +236,15 @@ describe("InspectorClient rate-limit recognition (#2442)", () => { async function connectTo( url: string, transport: CreateTransport = createTransportNode, + era: "legacy" | "modern" = "legacy", ): Promise { client = new InspectorClient( { type: "streamable-http", url }, { environment: { transport }, - // Pinned legacy so the connect is exactly `initialize`. - versionNegotiation: eraToVersionNegotiation("legacy"), + // Pinned, so the connect is exactly `initialize` (legacy) or + // `server/discover` (modern) and nothing else. + versionNegotiation: eraToVersionNegotiation(era), }, ); await client.connect(); @@ -350,4 +384,41 @@ describe("InspectorClient rate-limit recognition (#2442)", () => { await connectTo(upstream.url, started.transport), ); }); + + /** + * Against a modern server advertising the tasks extension, a tool call + * bypasses the SDK's `Client.request` for the ext-tasks raw-wire channel, + * so this is the case that proves the annotation is applied on that path + * too, not only on the SDK funnel. (`tools/list` is no use here: it takes + * the SDK funnel in both eras, and so does `tools/call` without the + * extension.) + */ + async function expectModernThrottled(c: InspectorClient): Promise { + expect(c.getProtocolEra()).toBe("modern"); + const caught = await rejectionOf( + c.callTool({ name: "echo", inputSchema: { type: "object" } }, {}), + ); + const data = expectThrottled(caught); + expect((caught as Error).message).toMatch( + /^Rate limited by the server \(HTTP 429\) on tools\/call\. The server asked to retry after 30s\. /, + ); + expect(data.method).toBe("tools/call"); + expect(data.retryAfterMs).toBe(30_000); + } + + it("annotates a modern-era 429, which takes the raw-wire path", async () => { + upstream = await startThrottlingServer(); + await expectModernThrottled( + await connectTo(upstream.url, createTransportNode, "modern"), + ); + }); + + it("annotates a modern-era 429 through the web backend", async () => { + upstream = await startThrottlingServer(); + const started = await startRemoteBackend(); + backend = started.server; + await expectModernThrottled( + await connectTo(upstream.url, started.transport, "modern"), + ); + }); }); diff --git a/core/extension/tasks/rawWireChannel.ts b/core/extension/tasks/rawWireChannel.ts index 455062c5fc..e2c4017151 100644 --- a/core/extension/tasks/rawWireChannel.ts +++ b/core/extension/tasks/rawWireChannel.ts @@ -47,9 +47,9 @@ export interface RawWireChannelHost { defaultTimeoutMs(): number; /** Whether progress on a request's token re-arms its timeout. */ resetTimeoutOnProgress(): boolean; - /** Decorate a timeout the way an SDK request's timeout is (#2318); any - * other error is returned untouched. */ - annotateTimeout(error: unknown, method: string): unknown; + /** Decorate a failed request's error the way an SDK request's is — a + * timeout (#2318), a `429` (#2442); any other error is returned untouched. */ + annotateError(error: unknown, method: string): unknown; } interface PendingRawRequest { @@ -182,7 +182,7 @@ export class RawWireChannel { `Raw MCP request "${message.method}" timed out after ${timeoutMs} ms`, false, { - cause: this.host.annotateTimeout( + cause: this.host.annotateError( new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { timeout: timeoutMs, }), @@ -232,10 +232,10 @@ export class RawWireChannel { pending.cleanup(); // The browser's remote transport awaits the response inside `send`, // so its relay wait can expire here first, as the SDK's timeout - // shape; annotate it exactly as the local timer above does (a - // non-timeout error passes through untouched). + // shape; annotate it exactly as the local timer above does. A `429` + // fails the send here too, and is annotated the same way (#2442). reject( - this.host.annotateTimeout( + this.host.annotateError( error instanceof Error ? error : new Error(String(error)), message.method, ), diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index e9b026901b..83dfc14eea 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -536,8 +536,7 @@ export class InspectorClient extends InspectorClientEventTarget { transport: () => this.transport, defaultTimeoutMs: () => this.requestTimeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC, resetTimeoutOnProgress: () => this.resetTimeoutOnProgress, - annotateTimeout: (error, method) => - annotateRequestTimeout(error, method, this.getConnectionDiagnostics()), + annotateError: (error, method) => this.annotateRequestError(error, method), }); private readonly dispatchTaskRequest: RawClientDispatch = ( request, @@ -1117,16 +1116,26 @@ export class InspectorClient extends InspectorClientEventTarget { try { return await sdkRequest(...args); } catch (err) { - const method = args[0].method; - throw annotateRateLimit( - annotateRequestTimeout(err, method, this.getConnectionDiagnostics()), - method, - ); + throw this.annotateRequestError(err, args[0].method); } }; client.request = decorated as SdkRequest; } + /** + * The one place a failed request's error is decorated, shared by the SDK + * funnel above and the raw-wire channel that bypasses it (modern-era tool + * calls and task operations), so a caller sees the same error whichever + * path its request took: a timeout with the connection's state (#2318), a + * `429` named as rate limiting (#2442), anything else untouched. + */ + private annotateRequestError(err: unknown, method: string): unknown { + return annotateRateLimit( + annotateRequestTimeout(err, method, this.getConnectionDiagnostics()), + method, + ); + } + /** * @param base - the composed OAuth-path fetch, deadline innermost (#2319). * diff --git a/core/mcp/rateLimit.ts b/core/mcp/rateLimit.ts index 1bbfe637d9..6696e79e4e 100644 --- a/core/mcp/rateLimit.ts +++ b/core/mcp/rateLimit.ts @@ -134,8 +134,13 @@ export function parseRetryAfter( if (/^\d+$/.test(trimmed)) return Number(trimmed) * 1000; if (!IMF_FIXDATE.test(trimmed)) return undefined; const at = Date.parse(trimmed); - // A well-formed shape can still name no real date (`Mon, 99 Foo 2026 …`). - if (Number.isNaN(at)) return undefined; + // A well-formed shape can still name no real date. `Mon, 99 Foo 2026 …` is + // `NaN`, but `Tue, 31 Nov 2026 …` silently rolls over to 1 December, so the + // parse is accepted only if it prints back as exactly what was sent — + // `toUTCString` emits IMF-fixdate, so a real date always does. + if (Number.isNaN(at) || new Date(at).toUTCString() !== trimmed) { + return undefined; + } return Math.max(0, at - now); }