From 93965936ba220dd88ea52925f96b8c2e2e915b03 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 11 Sep 2026 05:42:00 -0400 Subject: [PATCH] docs: describe the shipped timeout architecture (#2319 round 14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three comments still said transport-internal discovery has no client-side wrapper, or that the wrapper must never be applied to the transport fetch. Round 11 made both false, and the second actively tells a caller to do the opposite of what InspectorClient does — while hiding that isExempt is the safety-critical part. - createRemoteFetch's marker comment now gives the real reason the reconstruction matters: both ends run the same budget, so this path is the backend winning the race, most plausibly a backgrounded tab throttling setTimeout while the server's fires on schedule. Which deadline fired must not decide whether the error names its endpoint. - The body-buffering comment now says buffering is the second reason isExempt is not optional on a mixed chain, since an exempt request skips it and an SSE body stays open rather than being drained. - The matching test comment is reframed as testing the proxy boundary independently of which deadline wins. The PR description is rewritten around what is actually shipped: both chains bounded and differing only in the exemption, the per-request envelope that replaced the fetchTimeoutMs option, and the three mutation-verified tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YFnM6RqB4nXqNysgUYmjmq Signed-off-by: cliffhall --- .../test/core/mcp/remote/createRemoteFetch.test.ts | 10 ++++++---- core/auth/requestTimeout.ts | 7 ++++--- core/mcp/remote/createRemoteFetch.ts | 11 ++++++++--- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/clients/web/src/test/core/mcp/remote/createRemoteFetch.test.ts b/clients/web/src/test/core/mcp/remote/createRemoteFetch.test.ts index 37e71d9d..27a42d61 100644 --- a/clients/web/src/test/core/mcp/remote/createRemoteFetch.test.ts +++ b/clients/web/src/test/core/mcp/remote/createRemoteFetch.test.ts @@ -272,10 +272,12 @@ describe("createRemoteFetch", () => { } it("rebuilds an OAuthRequestTimeoutError from the route's marker", async () => { - // The one path where a timeout reaches the browser without a client-side - // wrapper having fired first: the discovery the SDK runs from inside the - // transport has no wrapper at all, so the backend's deadline is the only - // one, and it arrives as an ordinary error response. + // The proxy boundary in isolation: whichever end's deadline fires, a + // timeout that comes back as an ordinary error response must still + // reconstruct as a typed one. Both ends run the same budget in + // production, so this path is the backend winning the race — a + // backgrounded tab throttling `setTimeout` is the plausible case — and + // which one fired must not decide whether the error names its endpoint. const remoteFetch = remoteReturning( 504, JSON.stringify({ diff --git a/core/auth/requestTimeout.ts b/core/auth/requestTimeout.ts index d74bff90..28efdbc4 100644 --- a/core/auth/requestTimeout.ts +++ b/core/auth/requestTimeout.ts @@ -391,9 +391,10 @@ export function withOAuthRequestTimeout( // `response.json()` would hang with nothing watching it (Copilot). Every // response on this path is a small, finite document — metadata, // a registration, a token — so buffering it under the same deadline - // bounds the whole exchange. This is the other reason the wrapper must - // never be applied to the transport fetch, whose bodies are streams that - // are supposed to stay open. + // bounds the whole exchange. It is also the second reason `isExempt` is + // not optional on a mixed chain: an exempt request skips this buffering + // entirely, which is what lets an SSE body stay open rather than being + // drained into memory here. const body = await Promise.race([response.arrayBuffer(), abandoned]); return rebuildResponse(response, body); } catch (err) { diff --git a/core/mcp/remote/createRemoteFetch.ts b/core/mcp/remote/createRemoteFetch.ts index 144f6681..d0757956 100644 --- a/core/mcp/remote/createRemoteFetch.ts +++ b/core/mcp/remote/createRemoteFetch.ts @@ -184,9 +184,14 @@ export function createRemoteFetch(options: RemoteFetchOptions): typeof fetch { // rebuild the typed error from its marker so the `instanceof` checks // downstream — most importantly the one that lets a stalled probe escape // `withRfc8414OidcCompat` — still see a timeout for what it is (#2319, - // Copilot). This is the only path on which such a timeout can reach the - // browser without a client-side wrapper having fired first: the discovery - // the SDK runs from inside the transport has no wrapper at all. + // Copilot). + // + // Both ends run the same budget, so the client's race usually settles + // first and this path is not taken. It matters when the backend's timer + // wins anyway — most plausibly a backgrounded tab, where the browser + // throttles `setTimeout` while the server's fires on schedule. Without + // this, which of two equal deadlines happened to fire would decide + // whether the error carried its endpoint. let parsed: unknown; try { parsed = JSON.parse(text);