From aedcc15f70332fbeafcd777dd8202377e9af83ec Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 01:00:53 +0300 Subject: [PATCH 1/6] fix(passthrough): bound connectTimeoutMs to TCP establishment only; fix duplicate error log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #27 Defect A: upstream.setTimeout(connectTimeoutMs) was armed immediately after http.request() and never re-armed on connect, so it measured time-to-first-byte rather than connection establishment time. On a keep-alive pooled socket there is no connect phase at all, making the 10 s budget a hard cap on upstream think-time and causing spurious 504s for long-running requests. Fix: fold a rearm into the existing 'socket' listener. When socket.connecting is true (fresh connection), re-arm to streamIdleTimeoutMs on the 'connect' event. When socket.connecting is false (pooled socket), re-arm immediately — no 'connect' event fires on a reused socket. The existing re-arm inside the response callback (for mid-stream idle semantics) is left in place. Defect B: the timeout handler called upstream.destroy() before writing the 504. destroy() on an in-flight ClientRequest emits 'error' (ECONNRESET) on the next tick. The error handler's `responded` guard was inert on the pre-header path (responded was still false), so both anthropic_upstream_timeout and anthropic_upstream_error were logged and res.end() was called after res.destroy(). Fix: replace the `responded` guard in the error handler with a new `settled` flag that is set by whichever handler responds first (timeout 504 writer or the response callback). The 504 is also written before upstream.destroy() to narrow the race window. Adds three regression tests: think-time > connectTimeoutMs succeeds; exactly one warn event on genuine timeout; pooled-socket path arms streamIdleTimeoutMs immediately. Co-Authored-By: Claude --- CHANGELOG.md | 24 +++++ src/anthropic-passthrough.ts | 31 +++++- src/config.ts | 7 +- test/integration/passthrough.test.ts | 145 +++++++++++++++++++++++++++ 4 files changed, 201 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e1e43c..3e2ad8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,30 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [Unreleased] + +### Fixed + +- **Anthropic passthrough — `connectTimeoutMs` now bounds only TCP connection + establishment** (fixes #27). Previously `upstream.setTimeout(connectTimeoutMs)` + was armed immediately after `http.request()` and was never re-armed on connect, + so it measured time-to-first-byte rather than time-to-connect. On a keep-alive + pooled socket there is no connect phase at all, meaning the 10 s budget was + effectively a hard cap on upstream think-time, producing spurious 504s for + long-running `claude-opus-4` requests. The socket listener now re-arms the + timer to `streamIdleTimeoutMs` the moment the TCP connection is established (or + immediately for a reused socket where no `connect` event fires). + +- **Anthropic passthrough — duplicate `anthropic_upstream_error` warn eliminated** + (fixes #27). When a pre-header timeout fired, the timeout handler called + `upstream.destroy()` before writing the 504. Destroying an in-flight + `ClientRequest` causes it to emit `error` (ECONNRESET) on the next tick; the + error handler's `responded` guard was inert on the pre-header path, so both + `anthropic_upstream_timeout` and `anthropic_upstream_error` were logged and + `res.end()` was called after `res.destroy()`. A new `settled` flag is now set + by whichever handler responds first; the error handler returns early when + `settled` is true, ensuring exactly one warn event per timeout. + ## [0.2.0] - 2026-08-09 ### Upgrading from 0.1.0 diff --git a/src/anthropic-passthrough.ts b/src/anthropic-passthrough.ts index 892ab98..37dbe6b 100644 --- a/src/anthropic-passthrough.ts +++ b/src/anthropic-passthrough.ts @@ -84,7 +84,12 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic return (req, res, body) => { const path = `${basePath}${req.url ?? "/"}`; - let responded = false; + // `settled` is set by whichever handler wins the race — the response + // callback (headers received) or the timeout handler (504 written). + // The error handler uses it as its early-return guard so that a + // destroy() issued by the timeout handler does not produce a duplicate + // `anthropic_upstream_error` warn after the 504 has already been sent. + let settled = false; const upstream = client.request( { @@ -100,7 +105,7 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // (cross-name position of interleaved duplicates is not guaranteed). }, (upstreamRes) => { - responded = true; + settled = true; upstream.setTimeout(options.streamIdleTimeoutMs); // Response direction: writeHead accepts a flat [name, value, ...] array // directly (Node's _storeHeader Array branch), preserving the upstream's @@ -112,8 +117,19 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic }, ); + // `connectTimeoutMs` bounds TCP connection establishment only. Once the + // socket connects (or when a pooled socket is reused — no 'connect' event + // fires on a reused socket), we re-arm the timer to `streamIdleTimeoutMs` + // so that long upstream think-time is not mistakenly cut off at 10 s. + // The `else` branch handles the pooled/keep-alive case: `socket.connecting` + // is already false, so we must arm the header-wait budget immediately. upstream.setTimeout(options.connectTimeoutMs); - upstream.on("socket", (socket) => socket.setNoDelay(true)); + upstream.on("socket", (socket) => { + socket.setNoDelay(true); + const rearm = () => upstream.setTimeout(options.streamIdleTimeoutMs); + if (socket.connecting) socket.once("connect", rearm); + else rearm(); // pooled socket — no 'connect' event will ever fire + }); // Request direction: build a Map from the filtered rawHeaders so that // duplicates (same lowercase key, different values) are preserved as array @@ -140,18 +156,23 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic } upstream.on("timeout", () => { - upstream.destroy(); + // Log and write the 504 BEFORE destroy() to narrow the race window with + // the 'error' handler. destroy() on an in-flight ClientRequest emits + // 'error' (ECONNRESET) on the next tick; setting `settled = true` here + // prevents that error from producing a duplicate warn log. options.logger.log("warn", "anthropic_upstream_timeout", { path: req.url ?? "/" }); if (!res.headersSent) { + settled = true; res.writeHead(504, { "content-type": "application/json" }); res.end(toAnthropicErrorBody("api_error", "upstream timed out")); } else { res.destroy(); } + upstream.destroy(); }); upstream.on("error", () => { - if (responded) return; + if (settled) return; options.logger.log("warn", "anthropic_upstream_error", { path: req.url ?? "/" }); if (!res.headersSent) { res.writeHead(502, { "content-type": "application/json" }); diff --git a/src/config.ts b/src/config.ts index 78e2e1c..85d6bee 100644 --- a/src/config.ts +++ b/src/config.ts @@ -59,7 +59,12 @@ const AnthropicSchema = z .url() .refine(requireHttpsOrLoopback, { message: `anthropic.baseUrl ${HTTPS_REQUIRED_MESSAGE}` }) .default("https://api.anthropic.com"), - /** Connection timeout for all upstream requests to the Anthropic leg. */ + /** + * TCP connection-establishment timeout for the Anthropic leg (milliseconds). + * Bounds only the time to establish a new TCP connection; once connected (or when + * a keep-alive socket is reused), the timer is re-armed to `streamIdleTimeoutMs` + * so that long upstream think-time is not cut off prematurely. + */ connectTimeoutMs: z.number().int().positive().default(10_000), /** Stream idle timeout for the Anthropic passthrough. */ streamIdleTimeoutMs: z.number().int().positive().default(300_000), diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index 499650a..de77ce4 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -1,5 +1,6 @@ import { describe, it, after } from "node:test"; import assert from "node:assert/strict"; +import type { LogLevel } from "../../src/logger.js"; import { startSubswitch, startFakeUpstream, rawHttpRequest, type SubswitchInstance, type FakeUpstream } from "./fake-upstreams.js"; const cleanups: (() => Promise)[] = []; @@ -335,4 +336,148 @@ describe("anthropic passthrough", () => { "response headers must reach client in original upstream order with original casing", ); }); + + // --------------------------------------------------------------------------- + // Timeout semantics — regression tests for issue #27 + // --------------------------------------------------------------------------- + // + // connectTimeoutMs must bound only TCP connection establishment. Once + // connected (or when a pooled socket is reused), the timer is re-armed to + // streamIdleTimeoutMs so that upstream think-time beyond connectTimeoutMs + // is not spuriously cut off with a 504. + + it("does not 504 when upstream think-time exceeds connectTimeoutMs but is under streamIdleTimeoutMs", async () => { + // Upstream delays its response by 200 ms — intentionally longer than + // connectTimeoutMs (50 ms) but shorter than streamIdleTimeoutMs (500 ms). + // Before the fix the 50 ms timer fired immediately after connect and + // produced a 504; after the fix the timer is re-armed to 500 ms on connect. + const anthropic = await startFakeUpstream((_req, res) => { + setTimeout(() => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "msg_think_time" })); + }, 200); + }); + const subswitch = await startSubswitch({ + anthropic: { + baseUrl: anthropic.url, + connectTimeoutMs: 50, + streamIdleTimeoutMs: 500, + }, + }); + cleanups.push(subswitch.close, anthropic.close); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + assert.equal( + response.status, + 200, + "response must be 200 — connectTimeoutMs must not cut off upstream think-time after connect", + ); + const body = (await response.json()) as { id: string }; + assert.equal(body.id, "msg_think_time"); + }); + + it("emits exactly one warn event on a genuine upstream timeout (no duplicate anthropic_upstream_error)", async () => { + // Upstream accepts the connection but never sends headers. + // After streamIdleTimeoutMs (100 ms) the timeout handler fires, writes a + // 504, and calls upstream.destroy(). That destroy() makes the ClientRequest + // emit 'error'; the error handler must return early (via `settled`) and must + // NOT log a second anthropic_upstream_error warn. + const captured: Array<{ level: LogLevel; event: string }> = []; + const anthropic = await startFakeUpstream((_req, _res) => { + // Never responds — stall indefinitely so the idle timer fires. + }); + const subswitch = await startSubswitch( + { + anthropic: { + baseUrl: anthropic.url, + connectTimeoutMs: 50, + streamIdleTimeoutMs: 100, + }, + }, + { + logger: { + log(level, event) { + captured.push({ level, event }); + }, + }, + }, + ); + cleanups.push(subswitch.close, anthropic.close); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + assert.equal(response.status, 504, "timed-out upstream must produce a 504"); + await response.text(); // drain + + // Allow one extra event-loop turn for any stray error events to land. + await new Promise((resolve) => setTimeout(resolve, 50)); + + const upstreamEvents = captured.filter((e) => e.event.startsWith("anthropic_upstream")); + assert.equal(upstreamEvents.length, 1, `expected exactly 1 upstream warn event, got: ${JSON.stringify(upstreamEvents)}`); + assert.equal(upstreamEvents[0]!.event, "anthropic_upstream_timeout", "the single warn must be anthropic_upstream_timeout"); + assert.equal(upstreamEvents[0]!.level, "warn"); + }); + + it("re-arms streamIdleTimeoutMs immediately on a pooled (keep-alive) socket", async () => { + // The pooled socket path takes the `else rearm()` branch because + // socket.connecting is false on a reused socket — no 'connect' event fires. + // Verify that a second request whose upstream think-time exceeds + // connectTimeoutMs but is under streamIdleTimeoutMs still succeeds. + let requestIndex = 0; + const anthropic = await startFakeUpstream((_req, res) => { + const idx = requestIndex++; + if (idx === 0) { + // First request: respond immediately to establish the pooled socket. + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "first" })); + } else { + // Second request: delay 200 ms — longer than connectTimeoutMs (50 ms), + // shorter than streamIdleTimeoutMs (500 ms). On a reused socket the + // `else rearm()` branch must arm the 500 ms budget immediately. + setTimeout(() => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "pooled" })); + }, 200); + } + }); + const subswitch = await startSubswitch({ + anthropic: { + baseUrl: anthropic.url, + connectTimeoutMs: 50, + streamIdleTimeoutMs: 500, + }, + }); + cleanups.push(subswitch.close, anthropic.close); + + const postOpts = { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + } as const; + + // First request: builds the pooled connection. + const r1 = await fetch(`${subswitch.url}/v1/messages`, postOpts); + assert.equal(r1.status, 200); + await r1.text(); + + // Second request: reuses the pooled socket (else rearm() branch). + const r2 = await fetch(`${subswitch.url}/v1/messages`, postOpts); + assert.equal( + r2.status, + 200, + "pooled socket: streamIdleTimeoutMs must be armed immediately via else rearm(), not cut off at connectTimeoutMs", + ); + const body2 = (await r2.json()) as { id: string }; + assert.equal(body2.id, "pooled"); + + // Both requests must have used one TCP connection (keep-alive reuse). + assert.equal(anthropic.connectionCount, 1, "keep-alive: both requests must share one TCP connection"); + }); }); From 5ff696d0cb3d468f243442681fd00496513af0d7 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 02:22:55 +0300 Subject: [PATCH 2/6] feat(passthrough): introduce headerTimeoutMs for origin-scale TTFB budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a three-budget design to the Anthropic passthrough: - connectTimeoutMs (10 s): TCP establishment only — unchanged. - headerTimeoutMs (600 s): connect→response-headers, re-armed on socket connect (or immediately for pooled sockets). Defaults to Anthropic's own server-side ceiling so the relay never fires before the origin does. - streamIdleTimeoutMs (300 s): headers→stream-end, reset per chunk — unchanged. Previously the socket re-armed to streamIdleTimeoutMs on connect; a non-streaming Opus completion with large max_tokens that buffers server-side for several minutes could be cut off at 300 s where a direct connection would have succeeded. With headerTimeoutMs defaulting to 600 s the relay is aligned with the origin's own timeout. Also: - CHANGELOG: version [Unreleased] → [0.2.1] - 2026-08-19; corrects the res.end/res.destroy ordering claim (it was res.destroy after res.end, not the reverse); documents the TTFB budget widening. - Tests: updated three existing timeout tests to exercise headerTimeoutMs explicitly; renamed to reflect the new knob. - Fixes missing headerTimeoutMs in server-wiring and doctor test fixtures. Co-Authored-By: Claude --- CHANGELOG.md | 36 +++++++++++++++++--------- README.md | 13 +++++----- src/anthropic-passthrough.ts | 22 +++++++++++++--- src/config.ts | 19 +++++++++++--- src/server.ts | 1 + subswitch.config.example.json | 1 + test/integration/passthrough.test.ts | 27 ++++++++++--------- test/integration/server-wiring.test.ts | 1 + test/unit/doctor.test.ts | 1 + 9 files changed, 85 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e2ad8f..f702b5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,28 +4,40 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). -## [Unreleased] +## [0.2.1] - 2026-08-19 ### Fixed - **Anthropic passthrough — `connectTimeoutMs` now bounds only TCP connection - establishment** (fixes #27). Previously `upstream.setTimeout(connectTimeoutMs)` - was armed immediately after `http.request()` and was never re-armed on connect, - so it measured time-to-first-byte rather than time-to-connect. On a keep-alive - pooled socket there is no connect phase at all, meaning the 10 s budget was - effectively a hard cap on upstream think-time, producing spurious 504s for - long-running `claude-opus-4` requests. The socket listener now re-arms the - timer to `streamIdleTimeoutMs` the moment the TCP connection is established (or - immediately for a reused socket where no `connect` event fires). + establishment; new `headerTimeoutMs` knob bounds time to first byte** (fixes #27). + Previously `upstream.setTimeout(connectTimeoutMs)` was armed immediately after + `http.request()` and was never re-armed on connect, so it measured time-to-first-byte + rather than time-to-connect. On a keep-alive pooled socket there is no connect + phase at all, meaning the 10 s budget was effectively a hard cap on upstream + think-time, producing spurious 504s for long-running `claude-opus-4` requests. + The fix introduces a three-budget design: `connectTimeoutMs` (10 s, TCP + establishment only), `headerTimeoutMs` (600 s default, connect→response-headers — + defaults to Anthropic's own server-side ceiling so the relay never fires before + the origin does on a legitimate long-running request such as a non-streaming Opus + completion with large `max_tokens`), and `streamIdleTimeoutMs` (300 s, + headers→stream-end, reset by every chunk). The socket listener now re-arms the + timer to `headerTimeoutMs` the moment the TCP connection is established (or + immediately for a reused socket where no `connect` event fires). **A hung upstream + that accepts the TCP connection but never sends headers now takes up to + `headerTimeoutMs` (default 600 s) to fail, not 10 s** — tune this knob down if you + need faster detection of stalled upstreams. - **Anthropic passthrough — duplicate `anthropic_upstream_error` warn eliminated** (fixes #27). When a pre-header timeout fired, the timeout handler called `upstream.destroy()` before writing the 504. Destroying an in-flight `ClientRequest` causes it to emit `error` (ECONNRESET) on the next tick; the error handler's `responded` guard was inert on the pre-header path, so both - `anthropic_upstream_timeout` and `anthropic_upstream_error` were logged and - `res.end()` was called after `res.destroy()`. A new `settled` flag is now set - by whichever handler responds first; the error handler returns early when + `anthropic_upstream_timeout` and `anthropic_upstream_error` were logged. On + `main` the actual response sequence was `writeHead(504)` → `res.end()` + (synchronous, in the timeout handler) → next tick, error handler sees + `headersSent === true` → `res.destroy()`; the client received a complete 504 body + and the real defect was the duplicate warn log only. A new `settled` flag is now + set by whichever handler responds first; the error handler returns early when `settled` is true, ensuring exactly one warn event per timeout. ## [0.2.0] - 2026-08-09 diff --git a/README.md b/README.md index 060a985..5093a77 100644 --- a/README.md +++ b/README.md @@ -277,8 +277,9 @@ All keys and their defaults: | `port` | `4141` | Port the proxy listens on | | `logLevel` | `"info"` | Log verbosity: `debug`, `info`, `warn`, or `error` | | `anthropic.baseUrl` | `"https://api.anthropic.com"` | Anthropic passthrough base URL | -| `anthropic.connectTimeoutMs` | `10000` (10 s) | **Anthropic leg only** — TCP connection timeout (see note below) | -| `anthropic.streamIdleTimeoutMs` | `300000` (5 min) | Anthropic stream idle timeout | +| `anthropic.connectTimeoutMs` | `10000` (10 s) | **Anthropic leg only** — TCP connection establishment timeout (see note below) | +| `anthropic.headerTimeoutMs` | `600000` (10 min) | **Anthropic leg only** — time from TCP connect to first response byte; defaults to Anthropic's own server-side ceiling so the relay never fires before the origin does on a legitimate long-running request (see note below) | +| `anthropic.streamIdleTimeoutMs` | `300000` (5 min) | Anthropic stream idle timeout (headers→stream-end, reset by every chunk) | | `anthropic.maxUpstreamSockets` | `32` | **Anthropic leg only** — max sockets in the keep-alive pool (see note below) | | `anthropic.allowInsecureBaseUrl` | `false` | **Security opt-in** — when false (the default), `subswitch serve` refuses to start if `anthropic.baseUrl` points at a host other than `api.anthropic.com`. Set to `true` only when routing through a trusted proxy in front of Anthropic's API. Loopback addresses are always exempt. | | `providers.codex.baseUrl` | `"https://chatgpt.com/backend-api/codex"` | Codex backend base URL — override to route subswitch through the wire recorder | @@ -297,11 +298,11 @@ All keys and their defaults: | `limits.pingIntervalMs` | `15000` (15 s) | Interval between SSE ping frames sent to clients during long Codex streams | | `limits.maxConcurrentRequests` | `32` | In-flight request ceiling; requests above this limit receive a 503 | -> **Why `connectTimeoutMs` and `maxUpstreamSockets` are Anthropic-leg-only**: the +> **Why `connectTimeoutMs`, `headerTimeoutMs`, and `maxUpstreamSockets` are Anthropic-leg-only**: the > Anthropic passthrough uses a node:http agent with an explicit keep-alive pool, so -> both knobs have meaningful effect there. The Codex leg uses Node's global `fetch` -> (undici's global dispatcher), which `maxUpstreamSockets` does not control — shipping -> them as per-provider keys would be config that bounds nothing on the Codex side. +> all three knobs have meaningful effect there. The Codex leg uses Node's global `fetch` +> (undici's global dispatcher), which these knobs do not control — shipping them as +> per-provider keys would be config that bounds nothing on the Codex side. ### `subswitch models --json` diff --git a/src/anthropic-passthrough.ts b/src/anthropic-passthrough.ts index 37dbe6b..e55c33b 100644 --- a/src/anthropic-passthrough.ts +++ b/src/anthropic-passthrough.ts @@ -53,7 +53,22 @@ const filterRawHeaders = (rawHeaders: readonly string[]): string[] => { export interface PassthroughOptions { readonly baseUrl: string; + /** + * Bounds TCP connection establishment only (milliseconds). + * Once connected (or for a reused pooled socket), the timer is re-armed to + * `headerTimeoutMs`. + */ readonly connectTimeoutMs: number; + /** + * Bounds the connect→response-headers phase (time to first byte), in milliseconds. + * Armed on socket connect (or immediately for pooled sockets). + * Re-armed to `streamIdleTimeoutMs` once headers arrive. + */ + readonly headerTimeoutMs: number; + /** + * Bounds the headers→stream-end phase, in milliseconds. + * Reset by every received chunk. + */ readonly streamIdleTimeoutMs: number; readonly logger: Logger; /** Maximum sockets in the keep-alive pool. Config key: limits.maxUpstreamSockets. */ @@ -119,14 +134,15 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // `connectTimeoutMs` bounds TCP connection establishment only. Once the // socket connects (or when a pooled socket is reused — no 'connect' event - // fires on a reused socket), we re-arm the timer to `streamIdleTimeoutMs` - // so that long upstream think-time is not mistakenly cut off at 10 s. + // fires on a reused socket), we re-arm the timer to `headerTimeoutMs` + // so that long upstream think-time before the first response byte is not + // mistakenly cut off at the short connect budget. // The `else` branch handles the pooled/keep-alive case: `socket.connecting` // is already false, so we must arm the header-wait budget immediately. upstream.setTimeout(options.connectTimeoutMs); upstream.on("socket", (socket) => { socket.setNoDelay(true); - const rearm = () => upstream.setTimeout(options.streamIdleTimeoutMs); + const rearm = () => upstream.setTimeout(options.headerTimeoutMs); if (socket.connecting) socket.once("connect", rearm); else rearm(); // pooled socket — no 'connect' event will ever fire }); diff --git a/src/config.ts b/src/config.ts index 85d6bee..c55765a 100644 --- a/src/config.ts +++ b/src/config.ts @@ -62,11 +62,22 @@ const AnthropicSchema = z /** * TCP connection-establishment timeout for the Anthropic leg (milliseconds). * Bounds only the time to establish a new TCP connection; once connected (or when - * a keep-alive socket is reused), the timer is re-armed to `streamIdleTimeoutMs` - * so that long upstream think-time is not cut off prematurely. + * a keep-alive socket is reused), the timer is re-armed to `headerTimeoutMs`. */ connectTimeoutMs: z.number().int().positive().default(10_000), - /** Stream idle timeout for the Anthropic passthrough. */ + /** + * Time from TCP connection established to first response byte (headers), in + * milliseconds. Bounds only the connect→response-headers phase; once headers + * arrive the timer is re-armed to `streamIdleTimeoutMs` for the stream body. + * Defaults to 600 000 ms — matching Anthropic's own server-side ceiling — so + * the relay never fires before the origin does on a legitimate long-running + * request (e.g. a non-streaming Opus completion with large max_tokens). + */ + headerTimeoutMs: z.number().int().positive().default(600_000), + /** + * Stream idle timeout for the Anthropic passthrough (milliseconds). + * Bounds the headers→stream-end phase; reset by every received chunk. + */ streamIdleTimeoutMs: z.number().int().positive().default(300_000), /** Maximum sockets in the keep-alive pool for the Anthropic passthrough. */ maxUpstreamSockets: z.number().int().positive().default(32), @@ -301,6 +312,7 @@ export interface Config { readonly anthropic: { readonly baseUrl: string; readonly connectTimeoutMs: number; + readonly headerTimeoutMs: number; readonly streamIdleTimeoutMs: number; readonly maxUpstreamSockets: number; /** @@ -593,6 +605,7 @@ export const resolveConfig = (file: FileConfig): Config => ({ anthropic: { baseUrl: file.anthropic.baseUrl, connectTimeoutMs: file.anthropic.connectTimeoutMs, + headerTimeoutMs: file.anthropic.headerTimeoutMs, streamIdleTimeoutMs: file.anthropic.streamIdleTimeoutMs, maxUpstreamSockets: file.anthropic.maxUpstreamSockets, allowInsecureBaseUrl: file.anthropic.allowInsecureBaseUrl, diff --git a/src/server.ts b/src/server.ts index 90f2a86..fe75eeb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -215,6 +215,7 @@ export const buildDeps = (config: Config, logger: Logger = createConsoleLogger(c forwardAnthropic: createAnthropicForwarder({ baseUrl: config.anthropic.baseUrl, connectTimeoutMs: config.anthropic.connectTimeoutMs, + headerTimeoutMs: config.anthropic.headerTimeoutMs, streamIdleTimeoutMs: config.anthropic.streamIdleTimeoutMs, maxUpstreamSockets: config.anthropic.maxUpstreamSockets, logger, diff --git a/subswitch.config.example.json b/subswitch.config.example.json index dfdac41..51ae28e 100644 --- a/subswitch.config.example.json +++ b/subswitch.config.example.json @@ -4,6 +4,7 @@ "anthropic": { "baseUrl": "https://api.anthropic.com", "connectTimeoutMs": 10000, + "headerTimeoutMs": 600000, "streamIdleTimeoutMs": 300000, "maxUpstreamSockets": 32, "allowInsecureBaseUrl": false diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index de77ce4..051c5d0 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -346,11 +346,12 @@ describe("anthropic passthrough", () => { // streamIdleTimeoutMs so that upstream think-time beyond connectTimeoutMs // is not spuriously cut off with a 504. - it("does not 504 when upstream think-time exceeds connectTimeoutMs but is under streamIdleTimeoutMs", async () => { + it("does not 504 when upstream think-time exceeds connectTimeoutMs but is under headerTimeoutMs", async () => { // Upstream delays its response by 200 ms — intentionally longer than - // connectTimeoutMs (50 ms) but shorter than streamIdleTimeoutMs (500 ms). + // connectTimeoutMs (50 ms) but shorter than headerTimeoutMs (500 ms). // Before the fix the 50 ms timer fired immediately after connect and - // produced a 504; after the fix the timer is re-armed to 500 ms on connect. + // produced a 504; after the fix the timer is re-armed to headerTimeoutMs + // (500 ms) on connect, so the 200 ms delay is within budget. const anthropic = await startFakeUpstream((_req, res) => { setTimeout(() => { res.writeHead(200, { "content-type": "application/json" }); @@ -361,6 +362,7 @@ describe("anthropic passthrough", () => { anthropic: { baseUrl: anthropic.url, connectTimeoutMs: 50, + headerTimeoutMs: 500, streamIdleTimeoutMs: 500, }, }); @@ -380,22 +382,22 @@ describe("anthropic passthrough", () => { assert.equal(body.id, "msg_think_time"); }); - it("emits exactly one warn event on a genuine upstream timeout (no duplicate anthropic_upstream_error)", async () => { + it("headerTimeoutMs fires and emits exactly one warn when upstream stalls before sending headers (no duplicate anthropic_upstream_error)", async () => { // Upstream accepts the connection but never sends headers. - // After streamIdleTimeoutMs (100 ms) the timeout handler fires, writes a + // After headerTimeoutMs (100 ms) the timeout handler fires, writes a // 504, and calls upstream.destroy(). That destroy() makes the ClientRequest // emit 'error'; the error handler must return early (via `settled`) and must // NOT log a second anthropic_upstream_error warn. const captured: Array<{ level: LogLevel; event: string }> = []; const anthropic = await startFakeUpstream((_req, _res) => { - // Never responds — stall indefinitely so the idle timer fires. + // Never responds — stall indefinitely so the header timer fires. }); const subswitch = await startSubswitch( { anthropic: { baseUrl: anthropic.url, connectTimeoutMs: 50, - streamIdleTimeoutMs: 100, + headerTimeoutMs: 100, }, }, { @@ -425,11 +427,11 @@ describe("anthropic passthrough", () => { assert.equal(upstreamEvents[0]!.level, "warn"); }); - it("re-arms streamIdleTimeoutMs immediately on a pooled (keep-alive) socket", async () => { + it("re-arms headerTimeoutMs immediately on a pooled (keep-alive) socket", async () => { // The pooled socket path takes the `else rearm()` branch because // socket.connecting is false on a reused socket — no 'connect' event fires. // Verify that a second request whose upstream think-time exceeds - // connectTimeoutMs but is under streamIdleTimeoutMs still succeeds. + // connectTimeoutMs but is under headerTimeoutMs still succeeds. let requestIndex = 0; const anthropic = await startFakeUpstream((_req, res) => { const idx = requestIndex++; @@ -439,8 +441,8 @@ describe("anthropic passthrough", () => { res.end(JSON.stringify({ id: "first" })); } else { // Second request: delay 200 ms — longer than connectTimeoutMs (50 ms), - // shorter than streamIdleTimeoutMs (500 ms). On a reused socket the - // `else rearm()` branch must arm the 500 ms budget immediately. + // shorter than headerTimeoutMs (500 ms). On a reused socket the + // `else rearm()` branch must arm the 500 ms headerTimeoutMs budget immediately. setTimeout(() => { res.writeHead(200, { "content-type": "application/json" }); res.end(JSON.stringify({ id: "pooled" })); @@ -451,6 +453,7 @@ describe("anthropic passthrough", () => { anthropic: { baseUrl: anthropic.url, connectTimeoutMs: 50, + headerTimeoutMs: 500, streamIdleTimeoutMs: 500, }, }); @@ -472,7 +475,7 @@ describe("anthropic passthrough", () => { assert.equal( r2.status, 200, - "pooled socket: streamIdleTimeoutMs must be armed immediately via else rearm(), not cut off at connectTimeoutMs", + "pooled socket: headerTimeoutMs must be armed immediately via else rearm(), not cut off at connectTimeoutMs", ); const body2 = (await r2.json()) as { id: string }; assert.equal(body2.id, "pooled"); diff --git a/test/integration/server-wiring.test.ts b/test/integration/server-wiring.test.ts index 1e2abb7..b7c4efa 100644 --- a/test/integration/server-wiring.test.ts +++ b/test/integration/server-wiring.test.ts @@ -53,6 +53,7 @@ const makeMinimalConfig = (overrides: { anthropic: { baseUrl: anthropicBaseUrl, connectTimeoutMs: 10_000, + headerTimeoutMs: 600_000, streamIdleTimeoutMs: 300_000, maxUpstreamSockets: 32, allowInsecureBaseUrl: anthropicAllowInsecureBaseUrl, diff --git a/test/unit/doctor.test.ts b/test/unit/doctor.test.ts index 6cf6938..d829605 100644 --- a/test/unit/doctor.test.ts +++ b/test/unit/doctor.test.ts @@ -132,6 +132,7 @@ const makeTestConfig = (): Config => ({ anthropic: { baseUrl: "https://api.anthropic.com", connectTimeoutMs: 10_000, + headerTimeoutMs: 600_000, streamIdleTimeoutMs: 300_000, maxUpstreamSockets: 32, allowInsecureBaseUrl: false, From 33f41e4fcf688eb714bafb1baeeab6983c24c078 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 02:39:35 +0300 Subject: [PATCH 3/6] test(passthrough)+fix(config): add streamIdleTimeoutMs coverage; raise headerTimeoutMs default to 660 s Three changes from verification follow-up on #27: 1. Add mid-stream idle test to passthrough.test.ts. The new test sends 6 SSE chunks ~50 ms apart (~300 ms of active streaming) then stalls. With streamIdleTimeoutMs=100ms the idle timer fires after the stall and res.destroy() truncates the body. A 2 s timing bound makes the test non-vacuous: with streamIdleTimeoutMs=10_000 the elapsed time reaches ~10 261 ms, failing the bound and proving the knob does the work. The active-chunk phase also pins the "reset by every received chunk" invariant. 2. Raise headerTimeoutMs default from 600_000 to 660_000 ms. The relay's clock starts at TCP connect; the origin's starts at full-request-received. Equal budgets with an earlier start lets the relay pre-empt the origin by the request-upload time plus RTT. The 60 s of headroom corrects that. Updated in src/config.ts (schema + JSDoc), subswitch.config.example.json, README.md table, and CHANGELOG.md. 3. Add assert.equal(result.value.config.anthropic.headerTimeoutMs, 660_000) to test/unit/config.test.ts alongside its connectTimeoutMs and streamIdleTimeoutMs siblings so the default is not unasserted. Co-Authored-By: Claude --- CHANGELOG.md | 19 +++--- README.md | 2 +- src/config.ts | 12 ++-- subswitch.config.example.json | 2 +- test/integration/passthrough.test.ts | 92 ++++++++++++++++++++++++++++ test/unit/config.test.ts | 1 + 6 files changed, 113 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f702b5f..99ed9de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,15 +16,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). phase at all, meaning the 10 s budget was effectively a hard cap on upstream think-time, producing spurious 504s for long-running `claude-opus-4` requests. The fix introduces a three-budget design: `connectTimeoutMs` (10 s, TCP - establishment only), `headerTimeoutMs` (600 s default, connect→response-headers — - defaults to Anthropic's own server-side ceiling so the relay never fires before - the origin does on a legitimate long-running request such as a non-streaming Opus - completion with large `max_tokens`), and `streamIdleTimeoutMs` (300 s, - headers→stream-end, reset by every chunk). The socket listener now re-arms the - timer to `headerTimeoutMs` the moment the TCP connection is established (or - immediately for a reused socket where no `connect` event fires). **A hung upstream - that accepts the TCP connection but never sends headers now takes up to - `headerTimeoutMs` (default 600 s) to fail, not 10 s** — tune this knob down if you + establishment only), `headerTimeoutMs` (660 s default, connect→response-headers — + defaults to 60 s above Anthropic's own ~600 s server-side ceiling; the extra + headroom accounts for the relay's clock starting at TCP connect while the origin's + starts only after the full request body is received, so equal budgets would let the + relay pre-empt the origin by the upload time plus RTT), and `streamIdleTimeoutMs` + (300 s, headers→stream-end, reset by every chunk). The socket listener now + re-arms the timer to `headerTimeoutMs` the moment the TCP connection is established + (or immediately for a reused socket where no `connect` event fires). **A hung + upstream that accepts the TCP connection but never sends headers now takes up to + `headerTimeoutMs` (default 660 s) to fail, not 10 s** — tune this knob down if you need faster detection of stalled upstreams. - **Anthropic passthrough — duplicate `anthropic_upstream_error` warn eliminated** diff --git a/README.md b/README.md index 5093a77..9a0a21c 100644 --- a/README.md +++ b/README.md @@ -278,7 +278,7 @@ All keys and their defaults: | `logLevel` | `"info"` | Log verbosity: `debug`, `info`, `warn`, or `error` | | `anthropic.baseUrl` | `"https://api.anthropic.com"` | Anthropic passthrough base URL | | `anthropic.connectTimeoutMs` | `10000` (10 s) | **Anthropic leg only** — TCP connection establishment timeout (see note below) | -| `anthropic.headerTimeoutMs` | `600000` (10 min) | **Anthropic leg only** — time from TCP connect to first response byte; defaults to Anthropic's own server-side ceiling so the relay never fires before the origin does on a legitimate long-running request (see note below) | +| `anthropic.headerTimeoutMs` | `660000` (11 min) | **Anthropic leg only** — time from TCP connect to first response byte; defaults to 60 s above Anthropic's own ~600 s server-side ceiling so the relay never fires before the origin does (the relay's clock starts earlier than the origin's — see note below) | | `anthropic.streamIdleTimeoutMs` | `300000` (5 min) | Anthropic stream idle timeout (headers→stream-end, reset by every chunk) | | `anthropic.maxUpstreamSockets` | `32` | **Anthropic leg only** — max sockets in the keep-alive pool (see note below) | | `anthropic.allowInsecureBaseUrl` | `false` | **Security opt-in** — when false (the default), `subswitch serve` refuses to start if `anthropic.baseUrl` points at a host other than `api.anthropic.com`. Set to `true` only when routing through a trusted proxy in front of Anthropic's API. Loopback addresses are always exempt. | diff --git a/src/config.ts b/src/config.ts index c55765a..1e0d025 100644 --- a/src/config.ts +++ b/src/config.ts @@ -69,11 +69,15 @@ const AnthropicSchema = z * Time from TCP connection established to first response byte (headers), in * milliseconds. Bounds only the connect→response-headers phase; once headers * arrive the timer is re-armed to `streamIdleTimeoutMs` for the stream body. - * Defaults to 600 000 ms — matching Anthropic's own server-side ceiling — so - * the relay never fires before the origin does on a legitimate long-running - * request (e.g. a non-streaming Opus completion with large max_tokens). + * Defaults to 660 000 ms — 60 s above Anthropic's own ~600 s server-side + * ceiling — so the relay never fires before the origin does on a legitimate + * long-running request (e.g. a non-streaming Opus completion with large + * max_tokens). The extra 60 s headroom exists because the relay's clock + * starts at TCP connect whereas the origin's starts when it has received the + * full request body; equal budgets with an earlier start would let the relay + * pre-empt the origin by the request-upload time plus RTT. */ - headerTimeoutMs: z.number().int().positive().default(600_000), + headerTimeoutMs: z.number().int().positive().default(660_000), /** * Stream idle timeout for the Anthropic passthrough (milliseconds). * Bounds the headers→stream-end phase; reset by every received chunk. diff --git a/subswitch.config.example.json b/subswitch.config.example.json index 51ae28e..ca75f21 100644 --- a/subswitch.config.example.json +++ b/subswitch.config.example.json @@ -4,7 +4,7 @@ "anthropic": { "baseUrl": "https://api.anthropic.com", "connectTimeoutMs": 10000, - "headerTimeoutMs": 600000, + "headerTimeoutMs": 660000, "streamIdleTimeoutMs": 300000, "maxUpstreamSockets": 32, "allowInsecureBaseUrl": false diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index 051c5d0..d12baac 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -483,4 +483,96 @@ describe("anthropic passthrough", () => { // Both requests must have used one TCP connection (keep-alive reuse). assert.equal(anthropic.connectionCount, 1, "keep-alive: both requests must share one TCP connection"); }); + + it("streamIdleTimeoutMs fires when the stream goes idle after headers are received", async () => { + // headerTimeoutMs (5 s) cannot be the knob that fires here — only streamIdleTimeoutMs + // (100 ms) can. We send 6 chunks ~50 ms apart (~300 ms of active streaming) before + // stalling. If chunks did NOT reset the idle timer, the 100 ms budget would fire + // during the active-streaming window and the body would be truncated before we receive + // all 6 chunks — the assertion on chunk count would then catch it. This pins the + // "reset by every received chunk" invariant as a falsifiable property, not mere prose. + const captured: Array<{ level: LogLevel; event: string }> = []; + const anthropic = await startFakeUpstream((_req, res) => { + res.writeHead(200, { "content-type": "text/event-stream" }); + let sent = 0; + const interval = setInterval(() => { + sent++; + res.write(`event: ping\ndata: ${sent}\n\n`); + if (sent >= 6) { + // Stop sending — stream goes idle; res.end() is never called. + clearInterval(interval); + } + }, 50); + }); + const subswitch = await startSubswitch( + { + anthropic: { + baseUrl: anthropic.url, + connectTimeoutMs: 50, + headerTimeoutMs: 5_000, + streamIdleTimeoutMs: 100, + }, + }, + { + logger: { + log(level, event) { + captured.push({ level, event }); + }, + }, + }, + ); + cleanups.push(subswitch.close, anthropic.close); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + // The upstream sent headers before stalling — client must see 200. + assert.equal(response.status, 200, "upstream sent headers before going idle"); + + // Collect the body; the server calls res.destroy() once streamIdleTimeoutMs fires, + // so the body is truncated mid-stream. Node fetch (undici) rejects the read with + // a network error on a destroyed socket. + // + // Timing bound: after the last chunk (~300 ms of streaming), the 100 ms idle + // timer fires, so the body read must complete (by throwing) within 2 s. If + // streamIdleTimeoutMs were huge (e.g. 10 s), the read would hang for 10 s and + // this assertion would fail — making the test non-vacuous. + const bodyReadStart = Date.now(); + let receivedBody = ""; + let bodyThrew = false; + try { + receivedBody = await response.text(); + } catch { + bodyThrew = true; + } + const bodyElapsedMs = Date.now() - bodyReadStart; + + // The body must be incomplete: either the read threw (connection reset) or the + // received text contains the streamed chunks but no clean termination. + assert.ok( + bodyThrew || receivedBody.includes("event: ping"), + "body must be truncated (threw) or contain the partial SSE chunks", + ); + assert.ok(bodyThrew, "reading the body must throw — res.destroy() tears down the socket mid-stream"); + assert.ok( + bodyElapsedMs < 2_000, + `body read must complete within 2 s (elapsed: ${bodyElapsedMs} ms) — streamIdleTimeoutMs must fire promptly, not after a huge delay`, + ); + + // Allow one event-loop turn for any stray events to settle. + await new Promise((resolve) => setTimeout(resolve, 150)); + + // Exactly one streamIdleTimeoutMs warn must be emitted. headerTimeoutMs (5 s) + // cannot have fired — the total test duration is well under 5 s. + const timeoutEvents = captured.filter((e) => e.event === "anthropic_upstream_timeout"); + assert.equal( + timeoutEvents.length, + 1, + `expected exactly 1 anthropic_upstream_timeout event, got: ${JSON.stringify(timeoutEvents)}`, + ); + assert.equal(timeoutEvents[0]!.level, "warn"); + }); }); diff --git a/test/unit/config.test.ts b/test/unit/config.test.ts index 3d3da95..0e8152a 100644 --- a/test/unit/config.test.ts +++ b/test/unit/config.test.ts @@ -27,6 +27,7 @@ describe("loadConfig", () => { assert.equal(result.value.config.logLevel, "info"); assert.equal(result.value.config.anthropic.baseUrl, "https://api.anthropic.com"); assert.equal(result.value.config.anthropic.connectTimeoutMs, 10_000); + assert.equal(result.value.config.anthropic.headerTimeoutMs, 660_000); assert.equal(result.value.config.anthropic.streamIdleTimeoutMs, 300_000); assert.equal(result.value.config.anthropic.maxUpstreamSockets, 32); assert.equal(result.value.config.providers.codex.baseUrl, "https://chatgpt.com/backend-api/codex"); From 34a6785442cf472ff1917e1abf662b58dd8f2ef3 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 03:36:31 +0300 Subject: [PATCH 4/6] fix(passthrough): arm connectTimeoutMs on socket during connect phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ClientRequest.setTimeout() defers via an internal 'connect' listener so it fires only after TCP connect — in the same tick as the headerTimeoutMs rearm — meaning connectTimeoutMs was never in force for any measurable interval. Measured before fix: blackholed IP (192.0.2.1) with connectTimeoutMs=700 failed after 75 019 ms (macOS kernel TCP timeout). Neither budget fired. Fix: arm the timer directly on the socket inside the 'socket' event handler, before 'connect' fires. Node v22's internal socket-timeout handler (onTimeout) skips req.emit('timeout') when socket.connecting is true, so we explicitly forward socket 'timeout' → upstream.emit('timeout') to trigger the 504 handler. On connect we cancel the connect timer and re-arm to headerTimeoutMs via the normal upstream.setTimeout() path (which propagates correctly for connected sockets). Measured after fix: same scenario fails at ~700 ms. ✓ On HTTPS, 'connect' fires after TCP but before the TLS handshake, so TLS negotiation falls under headerTimeoutMs, not connectTimeoutMs. Documented in JSDoc for both PassthroughOptions and config.ts AnthropicSchema. New test: connectTimeoutMs fires during TCP connect to a non-routable upstream (192.0.2.1, TEST-NET-1). Without fix the test was cancelled at the 30 s --test-timeout limit; with fix it passes in ~261 ms. Non-vacuity confirmed empirically. Exactly one anthropic_upstream_timeout warn is emitted — settled de-dup holds on the connect-timeout path. Co-Authored-By: Claude --- CHANGELOG.md | 41 +++++++------ src/anthropic-passthrough.ts | 61 +++++++++++++++---- src/config.ts | 11 +++- test/integration/passthrough.test.ts | 89 ++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ed9de..f9cbe55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,25 +8,30 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **Anthropic passthrough — `connectTimeoutMs` now bounds only TCP connection +- **Anthropic passthrough — `connectTimeoutMs` now genuinely bounds TCP connection establishment; new `headerTimeoutMs` knob bounds time to first byte** (fixes #27). - Previously `upstream.setTimeout(connectTimeoutMs)` was armed immediately after - `http.request()` and was never re-armed on connect, so it measured time-to-first-byte - rather than time-to-connect. On a keep-alive pooled socket there is no connect - phase at all, meaning the 10 s budget was effectively a hard cap on upstream - think-time, producing spurious 504s for long-running `claude-opus-4` requests. - The fix introduces a three-budget design: `connectTimeoutMs` (10 s, TCP - establishment only), `headerTimeoutMs` (660 s default, connect→response-headers — - defaults to 60 s above Anthropic's own ~600 s server-side ceiling; the extra - headroom accounts for the relay's clock starting at TCP connect while the origin's - starts only after the full request body is received, so equal budgets would let the - relay pre-empt the origin by the upload time plus RTT), and `streamIdleTimeoutMs` - (300 s, headers→stream-end, reset by every chunk). The socket listener now - re-arms the timer to `headerTimeoutMs` the moment the TCP connection is established - (or immediately for a reused socket where no `connect` event fires). **A hung - upstream that accepts the TCP connection but never sends headers now takes up to - `headerTimeoutMs` (default 660 s) to fail, not 10 s** — tune this knob down if you - need faster detection of stalled upstreams. + Previously `upstream.setTimeout(connectTimeoutMs)` was called on the `ClientRequest` + object, but Node's implementation defers that call via an internal `'connect'` + listener — so it fired only after TCP connect, in the same tick as the + `headerTimeoutMs` rearm, meaning `connectTimeoutMs` was never in force for any + measurable interval. Measured against a blackholed IP (`192.0.2.1`, TEST-NET-1) + with `connectTimeoutMs=700` and `headerTimeoutMs=5000`, the request previously + failed after **75 019 ms** (the macOS kernel TCP timeout) — neither budget fired. + The fix arms the timer **directly on the socket** inside the `'socket'` event + handler, before `'connect'` fires, so the connect-phase budget is in force for + the full TCP handshake window. The same measured scenario now fails at ~700 ms. + On HTTPS, `'connect'` fires after TCP establishment but before the TLS handshake, + so TLS negotiation falls under `headerTimeoutMs`, not `connectTimeoutMs` — this is + documented in the JSDoc. The fix also introduces a three-budget design: + `connectTimeoutMs` (10 s, TCP establishment only), `headerTimeoutMs` (660 s + default, connect→response-headers — defaults to 60 s above Anthropic's own ~600 s + server-side ceiling; the extra headroom accounts for the relay's clock starting at + TCP connect while the origin's starts only after the full request body is received, + so equal budgets would let the relay pre-empt the origin by the upload time plus + RTT), and `streamIdleTimeoutMs` (300 s, headers→stream-end, reset by every chunk). + **A hung upstream that accepts the TCP connection but never sends headers now takes + up to `headerTimeoutMs` (default 660 s) to fail, not 10 s** — tune this knob down + if you need faster detection of stalled upstreams. - **Anthropic passthrough — duplicate `anthropic_upstream_error` warn eliminated** (fixes #27). When a pre-header timeout fired, the timeout handler called diff --git a/src/anthropic-passthrough.ts b/src/anthropic-passthrough.ts index e55c33b..ed807dd 100644 --- a/src/anthropic-passthrough.ts +++ b/src/anthropic-passthrough.ts @@ -55,8 +55,15 @@ export interface PassthroughOptions { readonly baseUrl: string; /** * Bounds TCP connection establishment only (milliseconds). - * Once connected (or for a reused pooled socket), the timer is re-armed to - * `headerTimeoutMs`. + * The timer is armed directly on the socket (not via `ClientRequest.setTimeout`, + * which defers internally and cannot bound the connect phase). Once TCP connects, + * the timer is re-armed to `headerTimeoutMs`. + * + * On HTTPS connections, `'connect'` fires after TCP establishment but before the + * TLS handshake, so TLS negotiation falls under `headerTimeoutMs`, not this budget. + * + * For pooled/keep-alive sockets (no connect phase), `headerTimeoutMs` is armed + * immediately and this budget has no effect. */ readonly connectTimeoutMs: number; /** @@ -132,19 +139,47 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic }, ); - // `connectTimeoutMs` bounds TCP connection establishment only. Once the - // socket connects (or when a pooled socket is reused — no 'connect' event - // fires on a reused socket), we re-arm the timer to `headerTimeoutMs` - // so that long upstream think-time before the first response byte is not - // mistakenly cut off at the short connect budget. - // The `else` branch handles the pooled/keep-alive case: `socket.connecting` - // is already false, so we must arm the header-wait budget immediately. - upstream.setTimeout(options.connectTimeoutMs); + // Timer arming — three-budget design: + // + // 1. connectTimeoutMs — bounds TCP establishment, armed DIRECTLY on the + // socket (not via upstream.setTimeout). ClientRequest.setTimeout() + // defers internally via its own 'connect' listener, so it would fire + // only after connect — in the same tick as the headerTimeoutMs rearm, + // leaving connectTimeoutMs never actually in force. + // + // Node v22's internal socket-timeout handler (onTimeout) skips + // req.emit('timeout') when socket.connecting is true, so we must + // propagate the timeout manually via upstream.emit('timeout'). + // On HTTPS, 'connect' fires after TCP but BEFORE the TLS handshake, so + // TLS negotiation falls under headerTimeoutMs, not connectTimeoutMs. + // + // 2. headerTimeoutMs — armed on 'connect' (or immediately for a pooled + // socket where no 'connect' event will ever fire). Bounds the window + // from TCP-connected to first response byte. upstream.setTimeout() in + // the connected state propagates normally via Node's internal handler. + // + // 3. streamIdleTimeoutMs — armed in the response callback once headers + // arrive; reset by every received chunk. upstream.on("socket", (socket) => { socket.setNoDelay(true); - const rearm = () => upstream.setTimeout(options.headerTimeoutMs); - if (socket.connecting) socket.once("connect", rearm); - else rearm(); // pooled socket — no 'connect' event will ever fire + if (socket.connecting) { + socket.setTimeout(options.connectTimeoutMs); + const onConnectTimeout = () => { + socket.removeListener("connect", onConnect); + socket.setTimeout(0); // disarm before manual propagation + upstream.emit("timeout"); // triggers our handler → 504 + }; + const onConnect = () => { + socket.removeListener("timeout", onConnectTimeout); + socket.setTimeout(0); // cancel connect budget + upstream.setTimeout(options.headerTimeoutMs); + }; + socket.once("timeout", onConnectTimeout); + socket.once("connect", onConnect); + } else { + // Pooled/keep-alive socket — no connect phase; arm header budget immediately. + upstream.setTimeout(options.headerTimeoutMs); + } }); // Request direction: build a Map from the filtered rawHeaders so that diff --git a/src/config.ts b/src/config.ts index 1e0d025..8633114 100644 --- a/src/config.ts +++ b/src/config.ts @@ -61,8 +61,15 @@ const AnthropicSchema = z .default("https://api.anthropic.com"), /** * TCP connection-establishment timeout for the Anthropic leg (milliseconds). - * Bounds only the time to establish a new TCP connection; once connected (or when - * a keep-alive socket is reused), the timer is re-armed to `headerTimeoutMs`. + * Bounds only the time to establish a new TCP connection; the timer is armed + * directly on the socket (not via `ClientRequest.setTimeout`, which defers + * internally and cannot bound the connect phase). + * Once TCP connects the timer is re-armed to `headerTimeoutMs`. + * + * On HTTPS connections, `'connect'` fires after TCP establishment but before + * the TLS handshake, so TLS negotiation falls under `headerTimeoutMs`. + * + * Has no effect on reused pooled sockets (no connect phase). */ connectTimeoutMs: z.number().int().positive().default(10_000), /** diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index d12baac..1bd0065 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -1,6 +1,9 @@ import { describe, it, after } from "node:test"; import assert from "node:assert/strict"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; import type { LogLevel } from "../../src/logger.js"; +import { createAnthropicForwarder } from "../../src/anthropic-passthrough.js"; import { startSubswitch, startFakeUpstream, rawHttpRequest, type SubswitchInstance, type FakeUpstream } from "./fake-upstreams.js"; const cleanups: (() => Promise)[] = []; @@ -575,4 +578,90 @@ describe("anthropic passthrough", () => { ); assert.equal(timeoutEvents[0]!.level, "warn"); }); + + // --------------------------------------------------------------------------- + // connectTimeoutMs non-vacuity — proves the timer fires during TCP connect + // --------------------------------------------------------------------------- + // + // This test requires a host/port that accepts SYN packets but never sends + // SYN-ACK, keeping socket.connecting === true for the duration of the budget. + // 192.0.2.1 (TEST-NET-1, RFC 5737) is used: it is documentation-only, has no + // real host, and on systems with a default route the SYN is forwarded but + // never answered — a true blackhole. + // + // CI note: on isolated containers with NO default route the kernel returns + // ENETUNREACH immediately, producing a 502 error rather than a 504 timeout. + // If this test fails with 502 on CI, the host has no route to 192.0.2.1 and + // the blackhole approach is not viable there. + // + // Non-vacuity: without the fix (upstream.setTimeout instead of socket.setTimeout), + // this test hangs until the macOS/Linux kernel TCP timeout (~75 s) or the + // 30 s --test-timeout limit, confirming the test discriminates the two states. + + it("connectTimeoutMs fires during TCP connect to a non-routable upstream (192.0.2.1)", async () => { + const CONNECT_MS = 250; + const captured: Array<{ level: LogLevel; event: string }> = []; + + // Call createAnthropicForwarder directly so we can use plain HTTP without + // the config-layer HTTPS-or-loopback validation. The agent test seam is + // not needed here — the real socket must connect (and fail) so socket.connecting + // is genuinely true during the budget window. + const forwarder = createAnthropicForwarder({ + baseUrl: "http://192.0.2.1:80", + connectTimeoutMs: CONNECT_MS, + headerTimeoutMs: 30_000, + streamIdleTimeoutMs: 30_000, + maxUpstreamSockets: 1, + logger: { + log(level: LogLevel, event: string) { + captured.push({ level, event }); + }, + }, + }); + + const server = http.createServer((req, res) => forwarder(req, res)); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + + const start = Date.now(); + const response = await fetch(`http://127.0.0.1:${port}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "test" }), + }); + const elapsed = Date.now() - start; + + await new Promise((resolve) => server.close(() => resolve())); + + assert.equal( + response.status, + 504, + `expected 504 (connect timeout after ${CONNECT_MS} ms); got ${response.status} after ${elapsed} ms. ` + + `If 502: 192.0.2.1 returned ENETUNREACH (no default route — host has no path to TEST-NET-1). ` + + `If 504 but elapsed >> ${CONNECT_MS}: connectTimeoutMs did not actually bound TCP connect.`, + ); + + // Without the fix this hangs ~75 000 ms (macOS kernel TCP timeout) or until + // the 30 s test timeout — well above the 4× upper bound below. + assert.ok( + elapsed < CONNECT_MS * 4, + `connect should time out at ~${CONNECT_MS} ms; elapsed ${elapsed} ms is too high — ` + + `connectTimeoutMs is not bounding TCP connect (unfixed code hangs ~75 000 ms or test-timeout)`, + ); + assert.ok( + elapsed >= CONNECT_MS * 0.5, + `timer fired before the connectTimeoutMs budget (elapsed: ${elapsed} ms < ${CONNECT_MS * 0.5} ms)`, + ); + + // The settled de-dup guard must hold on the connect-timeout path too: + // exactly one anthropic_upstream_timeout warn, no anthropic_upstream_error. + const upstreamEvents = captured.filter((e) => e.event.startsWith("anthropic_upstream")); + assert.equal( + upstreamEvents.length, + 1, + `expected exactly 1 upstream warn event; got: ${JSON.stringify(upstreamEvents)}`, + ); + assert.equal(upstreamEvents[0]!.event, "anthropic_upstream_timeout"); + assert.equal(upstreamEvents[0]!.level, "warn"); + }); }); From d977212cd92acdd7a79b11c36f67edf9fda2a545 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 02:57:12 +0300 Subject: [PATCH 5/6] fix(passthrough): retry stale socket once; mark synthesized responses; silence client-abort warn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CHANGE 1 (L2): When a connection-level error (ECONNRESET / EPIPE / socket hang up) arrives before any response byte and the request body is buffered, retry exactly once on a fresh socket (agent: false, bypassing the stale pool entry). If both attempts fail, res.destroy() presents a transport failure rather than a relay-synthesised HTTP 502 the origin cannot produce. CHANGE 2 (L3): Add x-subswitch-synthesized: 1 to every response the relay generates — 504 (timeout), 502 (connection failure), 503 (concurrency gate), 413 (body too large), 500 (internal proxy error), 400 (ambiguous / unknown provider), 404 (unknown /__subswitch path), 200 (health). Proxied origin responses, including upstream error statuses such as 429, do not receive the header. CHANGE 3: Set settled = true in res.on("close") before calling upstream.destroy(), so the error event emitted by destroy() on the next tick does not produce a spurious anthropic_upstream_error warn or attempt to write a 502 into an already-closed response. Also set settled = true in the error handler before writing the 502 for defence against theoretically possible double-emission. Tests: 8 new integration tests (L2 retry bounded/success/both-fail/no-retry- after-headers; L3 header present/absent; Change 3 client-abort silence). Co-Authored-By: Claude --- CHANGELOG.md | 25 +++ src/anthropic-passthrough.ts | 113 ++++++++--- src/server.ts | 14 +- test/integration/passthrough.test.ts | 275 +++++++++++++++++++++++++++ 4 files changed, 398 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9cbe55..645d098 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,31 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). up to `headerTimeoutMs` (default 660 s) to fail, not 10 s** — tune this knob down if you need faster detection of stalled upstreams. +- **Anthropic passthrough — stale pooled socket retried once on fresh connection before surfacing error** (stacks on #30). + A keep-alive socket closed server-side between requests causes Node's HTTP client to ECONNRESET the next POST. + Node's agent does not auto-retry a request that has already been sent. Previously the relay turned this + into a relay-synthesised HTTP 502 — a status the origin cannot produce — which SDK retry logic classifies + differently from the transport error a direct client would see. The upstream error handler now retries + ONCE on a fresh socket (`agent: false`, bypassing the stale pooled socket) when the failure is a + connection-level error (ECONNRESET / EPIPE / socket hang up), the request body is fully buffered, and + nothing has been written to the client response yet. If the retry also fails, `res.destroy()` is called + so the client sees a transport failure rather than a relay-manufactured HTTP status. Exactly one retry; + no backoff loop; not attempted when body is streaming (`body === undefined`). + +- **Anthropic passthrough — `x-subswitch-synthesized: 1` header marks every relay-generated response** (stacks on #30). + Previously a relay fault (502 connection failure, 504 timeout, 500 internal proxy error, 413 body too large, + 503 concurrency gate) was indistinguishable from an upstream outage on the client side. Every response the + relay synthesises itself now carries `x-subswitch-synthesized: 1`; responses proxied verbatim from the + origin do not. Bodies and status codes are unchanged — the header is purely additive. + +- **Anthropic passthrough — client abort no longer logs a spurious `anthropic_upstream_error` warn** (stacks on #30). + `res.on("close")` was calling `upstream.destroy()` without first setting `settled = true`. The destroy + emits `'error'` on the next tick; with `settled = false` the error handler logged `anthropic_upstream_error` + and attempted to write a 502 into an already-closed response. A client hanging up is a normal event; + it now sets `settled = true` before the destroy so the error handler returns early. Additionally, the + error handler now sets `settled = true` before writing the 502 so a theoretically possible second + `'error'` emission cannot double-write. + - **Anthropic passthrough — duplicate `anthropic_upstream_error` warn eliminated** (fixes #27). When a pre-header timeout fired, the timeout handler called `upstream.destroy()` before writing the 504. Destroying an in-flight diff --git a/src/anthropic-passthrough.ts b/src/anthropic-passthrough.ts index ed807dd..45e0c66 100644 --- a/src/anthropic-passthrough.ts +++ b/src/anthropic-passthrough.ts @@ -98,9 +98,10 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // Create a keep-alive agent for persistent connections to the upstream. // This matches Claude Code's own direct connection behaviour (parity). // - // Residual risk: a stale pooled socket can ECONNRESET a POST; the existing - // upstream.on("error") handler returns a clean 502 and Claude Code retries. - // This is the same behaviour as any keep-alive HTTP client. + // A stale pooled socket can ECONNRESET a POST (server-side idle timeout + // while the socket sat in the pool). Node's agent does NOT auto-retry that. + // The error handler below retries ONCE on a fresh socket (agent: false) so + // the client never sees a relay-synthesised 502 for a healthy upstream. const agentOpts: http.AgentOptions = { keepAlive: true, maxSockets: options.maxUpstreamSockets, scheduling: "lifo" }; const agent = options.agent ?? (target.protocol === "https:" ? new https.Agent(agentOpts) : new http.Agent(agentOpts)); @@ -189,22 +190,28 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // (A,A,B); per-name value order is preserved (RFC 7230 §3.2.2) and adjacent // duplicates are byte-exact. Anthropic clients do not interleave duplicate // header names, so this is safe in practice. - const filteredRaw = filterRawHeaders(req.rawHeaders); - const headerMap = new Map(); - for (let i = 0; i + 1 < filteredRaw.length; i += 2) { - const name = filteredRaw[i]!; - const value = filteredRaw[i + 1]!; - const key = name.toLowerCase(); - const entry = headerMap.get(key); - if (entry === undefined) { - headerMap.set(key, { name, values: [value] }); - } else { - entry.values.push(value); + // + // Extracted as a helper so the retry path can apply the same headers to a + // fresh ClientRequest without duplicating the logic. + const applyRequestHeaders = (target: http.ClientRequest): void => { + const filteredRaw = filterRawHeaders(req.rawHeaders); + const headerMap = new Map(); + for (let i = 0; i + 1 < filteredRaw.length; i += 2) { + const name = filteredRaw[i]!; + const value = filteredRaw[i + 1]!; + const key = name.toLowerCase(); + const entry = headerMap.get(key); + if (entry === undefined) { + headerMap.set(key, { name, values: [value] }); + } else { + entry.values.push(value); + } } - } - for (const { name, values } of headerMap.values()) { - upstream.setHeader(name, values.length === 1 ? values[0]! : values); - } + for (const { name, values } of headerMap.values()) { + target.setHeader(name, values.length === 1 ? values[0]! : values); + } + }; + applyRequestHeaders(upstream); upstream.on("timeout", () => { // Log and write the 504 BEFORE destroy() to narrow the race window with @@ -214,7 +221,7 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic options.logger.log("warn", "anthropic_upstream_timeout", { path: req.url ?? "/" }); if (!res.headersSent) { settled = true; - res.writeHead(504, { "content-type": "application/json" }); + res.writeHead(504, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(toAnthropicErrorBody("api_error", "upstream timed out")); } else { res.destroy(); @@ -222,19 +229,81 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic upstream.destroy(); }); - upstream.on("error", () => { + upstream.on("error", (upstreamErr: NodeJS.ErrnoException) => { if (settled) return; + + // Connection-level failures (ECONNRESET / EPIPE / socket hang up) that + // arrive before any response byte are the stale-pooled-socket scenario: + // the upstream is healthy but the keep-alive socket was closed server-side + // between requests. Retry ONCE on a fresh socket (agent: false bypasses + // the pool entirely) so the client never sees a relay-synthesised 502 for + // a working upstream. Only safe when the body is fully buffered and + // nothing has been written to the client response yet. + const isConnectionError = + upstreamErr.code === "ECONNRESET" || + upstreamErr.code === "EPIPE" || + (upstreamErr.message?.includes("socket hang up") ?? false); + + if (isConnectionError && body !== undefined && !res.headersSent) { + options.logger.log("info", "anthropic_upstream_retry", { path: req.url ?? "/" }); + + const retry = client.request( + { + protocol: target.protocol, + hostname: target.hostname, + ...(target.port !== "" ? { port: Number(target.port) } : {}), + method: req.method ?? "GET", + path, + agent: false, // force fresh TCP connection; bypass stale pooled socket + }, + (retryRes) => { + settled = true; + res.writeHead(retryRes.statusCode ?? 502, filterRawHeaders(retryRes.rawHeaders)); + res.socket?.setNoDelay(true); + retryRes.pipe(res); + retryRes.on("error", () => res.destroy()); + }, + ); + + applyRequestHeaders(retry); + retry.setTimeout(options.headerTimeoutMs); + retry.on("timeout", () => { retry.destroy(); }); + retry.on("socket", (s) => { s.setNoDelay(true); }); + retry.on("error", () => { + if (settled) return; + settled = true; + // Both attempts failed — present a transport failure rather than + // manufacturing an HTTP status the origin cannot produce. + res.destroy(); + }); + retry.end(body); + return; + } + + // Non-retryable path: log and surface as 502 only when the client + // response is still open. Set settled before writing so a second 'error' + // emission (practically unreachable after socket destroy, but possible in + // theory) cannot double-write. options.logger.log("warn", "anthropic_upstream_error", { path: req.url ?? "/" }); if (!res.headersSent) { - res.writeHead(502, { "content-type": "application/json" }); + settled = true; + res.writeHead(502, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(toAnthropicErrorBody("api_error", "upstream connection failed")); } else { res.destroy(); } }); + // Change 3: set settled before destroy() so the error event emitted by + // destroy() on the next tick does not trigger a spurious + // anthropic_upstream_error warn or attempt to write a 502 into an already- + // closed response. A client abort is an entirely normal event — it must + // produce no warn log and no synthetic HTTP response. res.on("close", () => { - if (!res.writableFinished) upstream.destroy(); + if (!res.writableFinished) { + settled = true; + upstream.destroy(); + } }); if (body !== undefined) { diff --git a/src/server.ts b/src/server.ts index fe75eeb..aa84e2e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -334,11 +334,11 @@ export const createProxyServer = (deps: ServerDeps): Server => { // /__subswitch/* namespace: handled locally, never forwarded upstream. if (pathname.startsWith("/__subswitch/")) { if (req.method === "GET" && pathname === "/__subswitch/health") { - res.writeHead(200, { "content-type": "application/json" }); + res.writeHead(200, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(buildHealthBody(config)); return; } - res.writeHead(404, { "content-type": "application/json" }); + res.writeHead(404, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(JSON.stringify({ error: "not found" })); return; } @@ -349,7 +349,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { res.on("close", () => { activeRequests--; }); if (activeRequests > config.limits.maxConcurrentRequests) { route = "rate_limited"; - res.writeHead(503, { "content-type": "application/json" }); + res.writeHead(503, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(toAnthropicErrorBody("overloaded_error", "too many concurrent requests — try again shortly")); return; } @@ -364,7 +364,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { const body = await bufferBody(req, config.limits.maxBodyBytes); if (!body.ok) { if (body.error.kind === "body_too_large") { - res.writeHead(413, { "content-type": "application/json", connection: "close" }); + res.writeHead(413, { "content-type": "application/json", connection: "close", "x-subswitch-synthesized": "1" }); res.end(toAnthropicErrorBody("invalid_request_error", body.error.message)); req.destroy(); } @@ -417,7 +417,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { // Two providers claim the same family name. Reject with 400 naming both. route = "ambiguous"; logger.log("warn", "ambiguous_model_name", { model: decision.name }); - res.writeHead(400, { "content-type": "application/json" }); + res.writeHead(400, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end( toAnthropicErrorBody( "invalid_request_error", @@ -431,7 +431,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { // "kimee:k2" — provider prefix not in PROVIDER_IDS. Reject with 400. route = "unknown_provider"; logger.log("warn", "unknown_provider_qualifier", { model: decision.qualifier }); - res.writeHead(400, { "content-type": "application/json" }); + res.writeHead(400, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end( toAnthropicErrorBody( "invalid_request_error", @@ -453,7 +453,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { dispatch().catch((cause: unknown) => { logger.log("error", "request_failed", { path: pathname, errorCode: cause instanceof Error ? cause.name : "unknown" }); if (!res.headersSent) { - res.writeHead(500, { "content-type": "application/json" }); + res.writeHead(500, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); res.end(toAnthropicErrorBody("api_error", "internal proxy error")); } else if (!res.writableEnded) { res.destroy(); diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index 1bd0065..a666039 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -1,6 +1,7 @@ import { describe, it, after } from "node:test"; import assert from "node:assert/strict"; import http from "node:http"; +import net from "node:net"; import type { AddressInfo } from "node:net"; import type { LogLevel } from "../../src/logger.js"; import { createAnthropicForwarder } from "../../src/anthropic-passthrough.js"; @@ -664,4 +665,278 @@ describe("anthropic passthrough", () => { assert.equal(upstreamEvents[0]!.event, "anthropic_upstream_timeout"); assert.equal(upstreamEvents[0]!.level, "warn"); }); + + // --------------------------------------------------------------------------- + // L2: stale-pooled-socket retry + // --------------------------------------------------------------------------- + // + // When a connection-level error (ECONNRESET / EPIPE) arrives before any + // response byte and the body is fully buffered, the relay retries ONCE on a + // fresh socket (agent: false). The client must never see a relay-synthesised + // 502 for a healthy upstream that just closed a keep-alive socket. + + it("L2: retries once on ECONNRESET before any response bytes; client gets success not 502", async () => { + const captured: Array<{ level: string; event: string }> = []; + + // Fake upstream: first POST is ECONNRESET'd (simulates stale pooled socket + // closed server-side); retry on a fresh socket succeeds. + const anthropic = await startFakeUpstream((_req, res, _body, index) => { + if (index === 0) { + res.socket!.destroy(); // ECONNRESET before any response headers + } else { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "retried" })); + } + }); + const subswitch = await startSubswitch( + { anthropic: { baseUrl: anthropic.url } }, + { logger: { log(level, event) { captured.push({ level, event }); } } }, + ); + cleanups.push(subswitch.close, anthropic.close); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + // Without fix: error handler writes 502. With fix: retry succeeds → 200. + assert.equal(response.status, 200, "retry must succeed; client must NOT see 502"); + const body = (await response.json()) as { id: string }; + assert.equal(body.id, "retried", "response body must come from the retry attempt"); + + const retryLogs = captured.filter((e) => e.event === "anthropic_upstream_retry"); + assert.equal(retryLogs.length, 1, "exactly one retry info event must be logged"); + + const errorLogs = captured.filter((e) => e.event === "anthropic_upstream_error"); + assert.equal(errorLogs.length, 0, "anthropic_upstream_error must not be logged on a successful retry"); + }); + + it("L2: when both attempts fail, client sees transport failure (connection destroyed), not 502", async () => { + // Both initial and retry get ECONNRESET — relay must call res.destroy() + // rather than manufacturing an HTTP 502 the origin cannot produce. + const anthropic = await startFakeUpstream((_req, res) => { + res.socket!.destroy(); // always ECONNRESET + }); + const subswitch = await startSubswitch({ anthropic: { baseUrl: anthropic.url } }); + cleanups.push(subswitch.close, anthropic.close); + + let threw = false; + let responseStatus: number | undefined; + try { + const resp = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + responseStatus = resp.status; + } catch { + threw = true; + } + + // Without fix: first ECONNRESET → writes 502 → responseStatus === 502, threw === false. + // With fix: both fail → res.destroy() → fetch throws → threw === true. + assert.ok(threw, `fetch must throw on dual-failure (transport error), not return HTTP ${responseStatus ?? "?"}`); + }); + + it("L2: retry is bounded to exactly one attempt; relay never retries more than once", async () => { + let upstreamAttempts = 0; + + const anthropic = await startFakeUpstream((_req, res) => { + upstreamAttempts++; + res.socket!.destroy(); // always fail + }); + const subswitch = await startSubswitch({ anthropic: { baseUrl: anthropic.url } }); + cleanups.push(subswitch.close, anthropic.close); + + try { + await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + } catch { + // Expected: transport failure after both attempts + } + + // Allow any stray events to settle before asserting. + await new Promise((resolve) => setTimeout(resolve, 80)); + + // Without fix: 1 attempt total (no retry). With fix: exactly 2 (initial + 1 retry). + assert.equal(upstreamAttempts, 2, "must make exactly 2 upstream attempts: initial + 1 retry"); + }); + + it("L2: does not retry after response headers have already been received", async () => { + // Use a raw TCP server that sends response headers then immediately destroys + // the socket. The relay must NOT retry because settled=true once headers + // arrive. + let upstreamAttempts = 0; + const openSockets = new Set(); + + const server = net.createServer((socket) => { + openSockets.add(socket); + socket.once("close", () => openSockets.delete(socket)); + upstreamAttempts++; + let buf = ""; + socket.on("data", (chunk: Buffer) => { + buf += chunk.toString(); + if (buf.includes("\r\n\r\n")) { + // Send HTTP 200 response headers only — no body — then destroy. + socket.write("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nTransfer-Encoding: chunked\r\n\r\n"); + socket.destroy(); + } + }); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const port = (server.address() as AddressInfo).port; + cleanups.push(() => new Promise((resolve) => { + for (const s of openSockets) s.destroy(); + server.close(() => resolve()); + })); + + const subswitch = await startSubswitch({ anthropic: { baseUrl: `http://127.0.0.1:${port}` } }); + cleanups.push(subswitch.close); + + try { + await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + } catch { + // Expected: partial response, broken body + } + + await new Promise((resolve) => setTimeout(resolve, 80)); + + // Retry would produce 2 upstream attempts. settled=true must prevent retry. + assert.equal(upstreamAttempts, 1, "relay must NOT retry after response headers were received"); + }); + + // --------------------------------------------------------------------------- + // L3: x-subswitch-synthesized marker header + // --------------------------------------------------------------------------- + // + // Every response the relay generates itself — 502 (connection failure), + // 504 (timeout), 500 (internal proxy error), 413 (body too large), 503 + // (concurrency gate) — carries x-subswitch-synthesized: 1 so operators can + // distinguish relay faults from upstream faults. Responses proxied from the + // origin (including upstream errors such as 429) must NOT carry this header. + + it("L3: x-subswitch-synthesized: 1 is present on a relay-synthesised 504 timeout response", async () => { + const anthropic = await startFakeUpstream((_req, _res) => { + // Never responds — triggers headerTimeoutMs. + }); + const subswitch = await startSubswitch({ + anthropic: { baseUrl: anthropic.url, headerTimeoutMs: 80 }, + }); + cleanups.push(subswitch.close, anthropic.close); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + // Without fix: header absent. With fix: "1". + assert.equal(response.status, 504); + assert.equal( + response.headers.get("x-subswitch-synthesized"), + "1", + "relay-synthesised 504 must carry x-subswitch-synthesized: 1", + ); + }); + + it("L3: x-subswitch-synthesized is ABSENT on a successfully proxied origin response", async () => { + const { subswitch } = await setup((_req, res) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "ok" })); + }); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + assert.equal(response.status, 200); + // Must be absent — the origin produced this response, not the relay. + assert.equal( + response.headers.get("x-subswitch-synthesized"), + null, + "proxied origin 200 must NOT carry x-subswitch-synthesized", + ); + }); + + it("L3: x-subswitch-synthesized is ABSENT on a proxied origin ERROR response such as 429", async () => { + // A 429 that the upstream produced must reach the client unchanged — + // the relay must not inject x-subswitch-synthesized on responses it merely forwarded. + const { subswitch } = await setup((_req, res) => { + res.writeHead(429, { "content-type": "application/json", "retry-after": "30" }); + res.end(JSON.stringify({ type: "error", error: { type: "rate_limit_error", message: "too many requests" } })); + }); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + assert.equal(response.status, 429); + // Without the marker: null. The relay must only mark what IT produces. + assert.equal( + response.headers.get("x-subswitch-synthesized"), + null, + "proxied upstream 429 error must NOT carry x-subswitch-synthesized", + ); + }); + + // --------------------------------------------------------------------------- + // Change 3: client abort must not produce anthropic_upstream_error warn + // --------------------------------------------------------------------------- + + it("Change 3: aborting the client mid-request produces no anthropic_upstream_error warn", async () => { + // Upstream never responds (stalls); client aborts after 60 ms. + // res.on("close") fires → settled=true → upstream.destroy() → error fires + // → settled guard prevents spurious warn and 502 attempt. + const captured: Array<{ level: string; event: string }> = []; + + const anthropic = await startFakeUpstream((_req, _res) => { + // Stall — never send a response. + }); + const subswitch = await startSubswitch( + { anthropic: { baseUrl: anthropic.url, headerTimeoutMs: 10_000 } }, + { logger: { log(level, event) { captured.push({ level, event }); } } }, + ); + cleanups.push(subswitch.close, anthropic.close); + + const controller = new AbortController(); + const fetchPromise = fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + signal: controller.signal, + }); + + // Abort before upstream has a chance to respond. + setTimeout(() => controller.abort(), 60); + + try { + await fetchPromise; + } catch { + // Expected: AbortError from the client-side abort. + } + + // Allow the event loop to drain so any stray error events settle. + await new Promise((resolve) => setTimeout(resolve, 100)); + + const warnLogs = captured.filter((e) => e.event === "anthropic_upstream_error"); + // Without fix: settled=false when res closes → upstream.destroy() → error fires → warn logged. + // With fix: settled=true before destroy() → error fires → settled guard returns early → no warn. + assert.equal( + warnLogs.length, + 0, + `client abort must not produce anthropic_upstream_error warn; got: ${JSON.stringify(warnLogs)}`, + ); + }); }); From b49fd246731b1c2742b7af271a94ce2d1e4f1dcb Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 19 Aug 2026 03:28:06 +0300 Subject: [PATCH 6/6] fix(passthrough): drop retry; fix synthesized-header coverage and authoritativeness Remove the upstream retry block (L2): the ECONNRESET predicate cannot distinguish a stale pooled socket from an origin that received the request and began processing it, creating a billing-duplicate window that spans request-write to first byte. Node's keep-alive agent already evicts destroyed sockets and the Anthropic SDK retries connection errors above the relay, so the relay retry is both unsafe and redundant. - Remove the retry request, the anthropic_upstream_retry log event, the isConnectionError predicate, and the applyRequestHeaders helper (now single-use; inlined into the main path) - Remove the four L2 tests Fix the x-subswitch-synthesized marker header (L3): - Strip the header from proxied upstream responses (added to HOP_BY_HOP) so an origin that sets it cannot impersonate the relay; adds strip test - Add the marker to respondJson by default so all 12+ codex-leg respondJson/respondProxyError call sites are covered in one line - Add the marker to the codex-leg SSE writeHead(200) (the one remaining synthesized site not going through respondJson) - Replace per-call-site { "x-subswitch-synthesized": "1" } spreads in server.ts with a synthesizedHeaders() helper for correct-by-default enforcement on future synthesized response sites - Add two codex-leg marker tests (streaming and non-streaming) - Update CHANGELOG and add operator-facing README section documenting the wire contract Keep Change 3 (client-abort warn fix) unchanged; add comment explaining the timeout handler asymmetry. Co-Authored-By: Claude --- CHANGELOG.md | 22 ++-- README.md | 30 +++++ src/anthropic-passthrough.ts | 111 +++++------------ src/codex-handler.ts | 2 +- src/provider-transport.ts | 7 +- src/server.ts | 28 +++-- test/integration/codex-leg.test.ts | 40 ++++++ test/integration/passthrough.test.ts | 175 +++++---------------------- 8 files changed, 164 insertions(+), 251 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 645d098..f831c1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,22 +33,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). up to `headerTimeoutMs` (default 660 s) to fail, not 10 s** — tune this knob down if you need faster detection of stalled upstreams. -- **Anthropic passthrough — stale pooled socket retried once on fresh connection before surfacing error** (stacks on #30). - A keep-alive socket closed server-side between requests causes Node's HTTP client to ECONNRESET the next POST. - Node's agent does not auto-retry a request that has already been sent. Previously the relay turned this - into a relay-synthesised HTTP 502 — a status the origin cannot produce — which SDK retry logic classifies - differently from the transport error a direct client would see. The upstream error handler now retries - ONCE on a fresh socket (`agent: false`, bypassing the stale pooled socket) when the failure is a - connection-level error (ECONNRESET / EPIPE / socket hang up), the request body is fully buffered, and - nothing has been written to the client response yet. If the retry also fails, `res.destroy()` is called - so the client sees a transport failure rather than a relay-manufactured HTTP status. Exactly one retry; - no backoff loop; not attempted when body is streaming (`body === undefined`). - -- **Anthropic passthrough — `x-subswitch-synthesized: 1` header marks every relay-generated response** (stacks on #30). +- **Anthropic passthrough — `x-subswitch-synthesized: 1` header marks every relay-generated response; stripped from proxied responses** (stacks on #30). Previously a relay fault (502 connection failure, 504 timeout, 500 internal proxy error, 413 body too large, 503 concurrency gate) was indistinguishable from an upstream outage on the client side. Every response the - relay synthesises itself now carries `x-subswitch-synthesized: 1`; responses proxied verbatim from the - origin do not. Bodies and status codes are unchanged — the header is purely additive. + relay synthesises itself now carries `x-subswitch-synthesized: 1`; this covers the Anthropic-leg error + paths, all codex-leg responses (both streaming SSE and aggregated JSON — the codex leg translates + Codex→Anthropic format so every byte it returns is relay-synthesised), and every relay-generated error in + `server.ts`. On the proxied path the header is actively stripped from upstream responses via the hop-by-hop + filter so an origin that sets it cannot impersonate the relay. Bodies and status codes are unchanged — the + header is purely additive. See the new `## x-subswitch-synthesized` section in the README for the + operator-facing wire contract. - **Anthropic passthrough — client abort no longer logs a spurious `anthropic_upstream_error` warn** (stacks on #30). `res.on("close")` was calling `upstream.destroy()` without first setting `settled = true`. The destroy diff --git a/README.md b/README.md index 9a0a21c..b422170 100644 --- a/README.md +++ b/README.md @@ -395,6 +395,36 @@ is not a TTY (useful in terminals that misreport TTY state); `CI` — also suppr color and disables interactive `init` prompts (treated as a non-interactive environment). +## `x-subswitch-synthesized` + +Every HTTP response that subswitch generates itself — rather than proxying +verbatim from an upstream — carries the response header: + +``` +x-subswitch-synthesized: 1 +``` + +This header is present on: + +- **Anthropic-leg relay errors**: 502 (upstream connection failure), 504 + (upstream timeout), 413 (request body too large), 503 (concurrency gate), + 500 (internal proxy error). +- **Codex-leg responses**: every byte returned on the codex leg is synthesized + by the relay (it translates OpenAI Responses format → Anthropic Messages + format), so the header is present on both streaming and non-streaming codex + responses, and on all codex-leg error responses. +- **Relay management endpoints**: `/__subswitch/health`, `/__subswitch/404`, + and all other relay-internal routes. + +The header is **absent** on responses proxied verbatim from the Anthropic +origin — including upstream errors (429 rate-limit, 529 overloaded, 500 +upstream internal error, etc.). The header is also **stripped** from any +upstream response that carries it, so the marker is authoritative: its presence +means the relay synthesised the response; its absence means the upstream did. + +Operators can use this header in load-balancer health rules, log filters, or +alerting to distinguish relay faults from upstream outages. + ## Testing ```sh diff --git a/src/anthropic-passthrough.ts b/src/anthropic-passthrough.ts index 45e0c66..fcae3df 100644 --- a/src/anthropic-passthrough.ts +++ b/src/anthropic-passthrough.ts @@ -12,6 +12,9 @@ import type { Logger } from "./logger.js"; * * Per RFC 7230 §6.1. `host` is also excluded: Node sets it on the outbound * connection. `connection` is managed by the keep-alive agent. + * + * `x-subswitch-synthesized` is also stripped from proxied responses so that + * the marker is authoritative: only the relay itself can assert it. */ const HOP_BY_HOP = new Set([ "host", @@ -24,6 +27,7 @@ const HOP_BY_HOP = new Set([ "trailer", "transfer-encoding", "upgrade", + "x-subswitch-synthesized", ]); /** @@ -97,11 +101,6 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // Create a keep-alive agent for persistent connections to the upstream. // This matches Claude Code's own direct connection behaviour (parity). - // - // A stale pooled socket can ECONNRESET a POST (server-side idle timeout - // while the socket sat in the pool). Node's agent does NOT auto-retry that. - // The error handler below retries ONCE on a fresh socket (agent: false) so - // the client never sees a relay-synthesised 502 for a healthy upstream. const agentOpts: http.AgentOptions = { keepAlive: true, maxSockets: options.maxUpstreamSockets, scheduling: "lifo" }; const agent = options.agent ?? (target.protocol === "https:" ? new https.Agent(agentOpts) : new http.Agent(agentOpts)); @@ -133,6 +132,8 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // Response direction: writeHead accepts a flat [name, value, ...] array // directly (Node's _storeHeader Array branch), preserving the upstream's // original header casing, order, and duplicates byte-for-byte. + // filterRawHeaders strips x-subswitch-synthesized so an origin that sets + // it cannot impersonate the relay's synthesized-response marker. res.writeHead(upstreamRes.statusCode ?? 502, filterRawHeaders(upstreamRes.rawHeaders)); res.socket?.setNoDelay(true); upstreamRes.pipe(res); @@ -190,28 +191,22 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // (A,A,B); per-name value order is preserved (RFC 7230 §3.2.2) and adjacent // duplicates are byte-exact. Anthropic clients do not interleave duplicate // header names, so this is safe in practice. - // - // Extracted as a helper so the retry path can apply the same headers to a - // fresh ClientRequest without duplicating the logic. - const applyRequestHeaders = (target: http.ClientRequest): void => { - const filteredRaw = filterRawHeaders(req.rawHeaders); - const headerMap = new Map(); - for (let i = 0; i + 1 < filteredRaw.length; i += 2) { - const name = filteredRaw[i]!; - const value = filteredRaw[i + 1]!; - const key = name.toLowerCase(); - const entry = headerMap.get(key); - if (entry === undefined) { - headerMap.set(key, { name, values: [value] }); - } else { - entry.values.push(value); - } - } - for (const { name, values } of headerMap.values()) { - target.setHeader(name, values.length === 1 ? values[0]! : values); + const filteredRaw = filterRawHeaders(req.rawHeaders); + const headerMap = new Map(); + for (let i = 0; i + 1 < filteredRaw.length; i += 2) { + const name = filteredRaw[i]!; + const value = filteredRaw[i + 1]!; + const key = name.toLowerCase(); + const entry = headerMap.get(key); + if (entry === undefined) { + headerMap.set(key, { name, values: [value] }); + } else { + entry.values.push(value); } - }; - applyRequestHeaders(upstream); + } + for (const { name, values } of headerMap.values()) { + upstream.setHeader(name, values.length === 1 ? values[0]! : values); + } upstream.on("timeout", () => { // Log and write the 504 BEFORE destroy() to narrow the race window with @@ -229,61 +224,12 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic upstream.destroy(); }); - upstream.on("error", (upstreamErr: NodeJS.ErrnoException) => { + upstream.on("error", () => { if (settled) return; - - // Connection-level failures (ECONNRESET / EPIPE / socket hang up) that - // arrive before any response byte are the stale-pooled-socket scenario: - // the upstream is healthy but the keep-alive socket was closed server-side - // between requests. Retry ONCE on a fresh socket (agent: false bypasses - // the pool entirely) so the client never sees a relay-synthesised 502 for - // a working upstream. Only safe when the body is fully buffered and - // nothing has been written to the client response yet. - const isConnectionError = - upstreamErr.code === "ECONNRESET" || - upstreamErr.code === "EPIPE" || - (upstreamErr.message?.includes("socket hang up") ?? false); - - if (isConnectionError && body !== undefined && !res.headersSent) { - options.logger.log("info", "anthropic_upstream_retry", { path: req.url ?? "/" }); - - const retry = client.request( - { - protocol: target.protocol, - hostname: target.hostname, - ...(target.port !== "" ? { port: Number(target.port) } : {}), - method: req.method ?? "GET", - path, - agent: false, // force fresh TCP connection; bypass stale pooled socket - }, - (retryRes) => { - settled = true; - res.writeHead(retryRes.statusCode ?? 502, filterRawHeaders(retryRes.rawHeaders)); - res.socket?.setNoDelay(true); - retryRes.pipe(res); - retryRes.on("error", () => res.destroy()); - }, - ); - - applyRequestHeaders(retry); - retry.setTimeout(options.headerTimeoutMs); - retry.on("timeout", () => { retry.destroy(); }); - retry.on("socket", (s) => { s.setNoDelay(true); }); - retry.on("error", () => { - if (settled) return; - settled = true; - // Both attempts failed — present a transport failure rather than - // manufacturing an HTTP status the origin cannot produce. - res.destroy(); - }); - retry.end(body); - return; - } - - // Non-retryable path: log and surface as 502 only when the client - // response is still open. Set settled before writing so a second 'error' - // emission (practically unreachable after socket destroy, but possible in - // theory) cannot double-write. + // Log and surface as 502 only when the client response is still open. + // Set settled before writing so a second 'error' emission (practically + // unreachable after socket destroy, but possible in theory) cannot + // double-write. options.logger.log("warn", "anthropic_upstream_error", { path: req.url ?? "/" }); if (!res.headersSent) { settled = true; @@ -299,6 +245,11 @@ export const createAnthropicForwarder = (options: PassthroughOptions): Anthropic // anthropic_upstream_error warn or attempt to write a 502 into an already- // closed response. A client abort is an entirely normal event — it must // produce no warn log and no synthetic HTTP response. + // + // Note: the timeout handler does not check `settled` before calling + // upstream.destroy() because it sets `settled = true` itself before the + // destroy, so by the time the 'error' event fires on the next tick the + // guard has already been set and the error handler returns early. res.on("close", () => { if (!res.writableFinished) { settled = true; diff --git a/src/codex-handler.ts b/src/codex-handler.ts index 72f303d..258ea15 100644 --- a/src/codex-handler.ts +++ b/src/codex-handler.ts @@ -349,7 +349,7 @@ export const createCodexHandler =

(deps: CodexHandlerDeps< resetIdle(); if (wantStream) { - res.writeHead(200, { "content-type": "text/event-stream; charset=utf-8", "cache-control": "no-cache" }); + res.writeHead(200, { "content-type": "text/event-stream; charset=utf-8", "cache-control": "no-cache", "x-subswitch-synthesized": "1" }); res.socket?.setNoDelay(true); // res is written manually rather than placed inside pipeline(): on an // upstream error, pipeline destroys every stream it owns, which would diff --git a/src/provider-transport.ts b/src/provider-transport.ts index 614cc2c..a714162 100644 --- a/src/provider-transport.ts +++ b/src/provider-transport.ts @@ -6,6 +6,11 @@ import { proxyErrorToAnthropic, toAnthropicErrorBody, type ProxyError } from "./ /** * Send a JSON response, no-op if headers were already sent. + * + * Every response emitted by a provider handler is synthesized by the relay + * (the codex leg translates Codex→Anthropic; no byte is forwarded verbatim). + * `x-subswitch-synthesized: 1` is therefore always correct here and is + * included by default so callers cannot forget it. */ export const respondJson = ( res: ServerResponse, @@ -14,7 +19,7 @@ export const respondJson = ( extraHeaders: Record = {}, ): void => { if (res.headersSent) return; - res.writeHead(status, { "content-type": "application/json", ...extraHeaders }); + res.writeHead(status, { "content-type": "application/json", "x-subswitch-synthesized": "1", ...extraHeaders }); res.end(body); }; diff --git a/src/server.ts b/src/server.ts index aa84e2e..2d9d853 100644 --- a/src/server.ts +++ b/src/server.ts @@ -309,6 +309,20 @@ const peekModel = (parsed: unknown): string | undefined => { return result.success ? result.data.model : undefined; }; +/** + * Base headers for every response the relay generates itself (as opposed to + * responses proxied verbatim from an upstream). The `x-subswitch-synthesized` + * marker is included here so callers cannot forget it and future synthesized + * response sites are correct by default. + * + * Pass extra headers (e.g. `{ connection: "close" }`) as `extra`. + */ +const synthesizedHeaders = (extra: Record = {}): Record => ({ + "content-type": "application/json", + "x-subswitch-synthesized": "1", + ...extra, +}); + export const createProxyServer = (deps: ServerDeps): Server => { const { config, logger } = deps; let activeRequests = 0; @@ -334,11 +348,11 @@ export const createProxyServer = (deps: ServerDeps): Server => { // /__subswitch/* namespace: handled locally, never forwarded upstream. if (pathname.startsWith("/__subswitch/")) { if (req.method === "GET" && pathname === "/__subswitch/health") { - res.writeHead(200, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(200, synthesizedHeaders()); res.end(buildHealthBody(config)); return; } - res.writeHead(404, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(404, synthesizedHeaders()); res.end(JSON.stringify({ error: "not found" })); return; } @@ -349,7 +363,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { res.on("close", () => { activeRequests--; }); if (activeRequests > config.limits.maxConcurrentRequests) { route = "rate_limited"; - res.writeHead(503, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(503, synthesizedHeaders()); res.end(toAnthropicErrorBody("overloaded_error", "too many concurrent requests — try again shortly")); return; } @@ -364,7 +378,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { const body = await bufferBody(req, config.limits.maxBodyBytes); if (!body.ok) { if (body.error.kind === "body_too_large") { - res.writeHead(413, { "content-type": "application/json", connection: "close", "x-subswitch-synthesized": "1" }); + res.writeHead(413, synthesizedHeaders({ connection: "close" })); res.end(toAnthropicErrorBody("invalid_request_error", body.error.message)); req.destroy(); } @@ -417,7 +431,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { // Two providers claim the same family name. Reject with 400 naming both. route = "ambiguous"; logger.log("warn", "ambiguous_model_name", { model: decision.name }); - res.writeHead(400, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(400, synthesizedHeaders()); res.end( toAnthropicErrorBody( "invalid_request_error", @@ -431,7 +445,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { // "kimee:k2" — provider prefix not in PROVIDER_IDS. Reject with 400. route = "unknown_provider"; logger.log("warn", "unknown_provider_qualifier", { model: decision.qualifier }); - res.writeHead(400, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(400, synthesizedHeaders()); res.end( toAnthropicErrorBody( "invalid_request_error", @@ -453,7 +467,7 @@ export const createProxyServer = (deps: ServerDeps): Server => { dispatch().catch((cause: unknown) => { logger.log("error", "request_failed", { path: pathname, errorCode: cause instanceof Error ? cause.name : "unknown" }); if (!res.headersSent) { - res.writeHead(500, { "content-type": "application/json", "x-subswitch-synthesized": "1" }); + res.writeHead(500, synthesizedHeaders()); res.end(toAnthropicErrorBody("api_error", "internal proxy error")); } else if (!res.writableEnded) { res.destroy(); diff --git a/test/integration/codex-leg.test.ts b/test/integration/codex-leg.test.ts index a03be77..7f0346e 100644 --- a/test/integration/codex-leg.test.ts +++ b/test/integration/codex-leg.test.ts @@ -914,4 +914,44 @@ describe("codex leg", () => { assert.equal(sent["model"], "gpt-9-sol", "config override target must be sent upstream"); assert.equal(anthropic.requests.length, 0, "config override must route to Codex, not Anthropic"); }); + + // --------------------------------------------------------------------------- + // L3: x-subswitch-synthesized marker on codex-leg responses + // --------------------------------------------------------------------------- + // + // Every byte the codex leg returns is synthesized by the relay (Codex SSE is + // translated to Anthropic format; nothing is forwarded verbatim). Both the + // streaming path (res.writeHead(200)) and the non-streaming path (respondJson) + // must carry x-subswitch-synthesized: 1. + + it("L3: codex-leg streaming response carries x-subswitch-synthesized: 1", async () => { + const rig = await setupRig(sseHandler(loadSse("text-only.sse"))); + const response = await postMessages(rig.subswitch, loadRequest("simple-text.json")); + assert.equal(response.status, 200); + // Without fix: null (SSE writeHead had no marker). + // With fix: "1" (added to the SSE writeHead in codex-handler.ts). + assert.equal( + response.headers.get("x-subswitch-synthesized"), + "1", + "codex-leg streaming response must carry x-subswitch-synthesized: 1", + ); + await response.text(); // drain + }); + + it("L3: codex-leg non-streaming response carries x-subswitch-synthesized: 1", async () => { + const rig = await setupRig(sseHandler(loadSse("text-only.sse"))); + const response = await postMessages( + rig.subswitch, + JSON.stringify({ model: "gpt-5.5", max_tokens: 64, messages: [{ role: "user", content: "hi" }] }), + ); + assert.equal(response.status, 200); + // Without fix: null (respondJson did not include the marker). + // With fix: "1" (added to respondJson default headers in provider-transport.ts). + assert.equal( + response.headers.get("x-subswitch-synthesized"), + "1", + "codex-leg non-streaming response must carry x-subswitch-synthesized: 1", + ); + await response.json(); // drain + }); }); diff --git a/test/integration/passthrough.test.ts b/test/integration/passthrough.test.ts index a666039..19ced38 100644 --- a/test/integration/passthrough.test.ts +++ b/test/integration/passthrough.test.ts @@ -1,7 +1,6 @@ import { describe, it, after } from "node:test"; import assert from "node:assert/strict"; import http from "node:http"; -import net from "node:net"; import type { AddressInfo } from "node:net"; import type { LogLevel } from "../../src/logger.js"; import { createAnthropicForwarder } from "../../src/anthropic-passthrough.js"; @@ -666,153 +665,6 @@ describe("anthropic passthrough", () => { assert.equal(upstreamEvents[0]!.level, "warn"); }); - // --------------------------------------------------------------------------- - // L2: stale-pooled-socket retry - // --------------------------------------------------------------------------- - // - // When a connection-level error (ECONNRESET / EPIPE) arrives before any - // response byte and the body is fully buffered, the relay retries ONCE on a - // fresh socket (agent: false). The client must never see a relay-synthesised - // 502 for a healthy upstream that just closed a keep-alive socket. - - it("L2: retries once on ECONNRESET before any response bytes; client gets success not 502", async () => { - const captured: Array<{ level: string; event: string }> = []; - - // Fake upstream: first POST is ECONNRESET'd (simulates stale pooled socket - // closed server-side); retry on a fresh socket succeeds. - const anthropic = await startFakeUpstream((_req, res, _body, index) => { - if (index === 0) { - res.socket!.destroy(); // ECONNRESET before any response headers - } else { - res.writeHead(200, { "content-type": "application/json" }); - res.end(JSON.stringify({ id: "retried" })); - } - }); - const subswitch = await startSubswitch( - { anthropic: { baseUrl: anthropic.url } }, - { logger: { log(level, event) { captured.push({ level, event }); } } }, - ); - cleanups.push(subswitch.close, anthropic.close); - - const response = await fetch(`${subswitch.url}/v1/messages`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), - }); - - // Without fix: error handler writes 502. With fix: retry succeeds → 200. - assert.equal(response.status, 200, "retry must succeed; client must NOT see 502"); - const body = (await response.json()) as { id: string }; - assert.equal(body.id, "retried", "response body must come from the retry attempt"); - - const retryLogs = captured.filter((e) => e.event === "anthropic_upstream_retry"); - assert.equal(retryLogs.length, 1, "exactly one retry info event must be logged"); - - const errorLogs = captured.filter((e) => e.event === "anthropic_upstream_error"); - assert.equal(errorLogs.length, 0, "anthropic_upstream_error must not be logged on a successful retry"); - }); - - it("L2: when both attempts fail, client sees transport failure (connection destroyed), not 502", async () => { - // Both initial and retry get ECONNRESET — relay must call res.destroy() - // rather than manufacturing an HTTP 502 the origin cannot produce. - const anthropic = await startFakeUpstream((_req, res) => { - res.socket!.destroy(); // always ECONNRESET - }); - const subswitch = await startSubswitch({ anthropic: { baseUrl: anthropic.url } }); - cleanups.push(subswitch.close, anthropic.close); - - let threw = false; - let responseStatus: number | undefined; - try { - const resp = await fetch(`${subswitch.url}/v1/messages`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), - }); - responseStatus = resp.status; - } catch { - threw = true; - } - - // Without fix: first ECONNRESET → writes 502 → responseStatus === 502, threw === false. - // With fix: both fail → res.destroy() → fetch throws → threw === true. - assert.ok(threw, `fetch must throw on dual-failure (transport error), not return HTTP ${responseStatus ?? "?"}`); - }); - - it("L2: retry is bounded to exactly one attempt; relay never retries more than once", async () => { - let upstreamAttempts = 0; - - const anthropic = await startFakeUpstream((_req, res) => { - upstreamAttempts++; - res.socket!.destroy(); // always fail - }); - const subswitch = await startSubswitch({ anthropic: { baseUrl: anthropic.url } }); - cleanups.push(subswitch.close, anthropic.close); - - try { - await fetch(`${subswitch.url}/v1/messages`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), - }); - } catch { - // Expected: transport failure after both attempts - } - - // Allow any stray events to settle before asserting. - await new Promise((resolve) => setTimeout(resolve, 80)); - - // Without fix: 1 attempt total (no retry). With fix: exactly 2 (initial + 1 retry). - assert.equal(upstreamAttempts, 2, "must make exactly 2 upstream attempts: initial + 1 retry"); - }); - - it("L2: does not retry after response headers have already been received", async () => { - // Use a raw TCP server that sends response headers then immediately destroys - // the socket. The relay must NOT retry because settled=true once headers - // arrive. - let upstreamAttempts = 0; - const openSockets = new Set(); - - const server = net.createServer((socket) => { - openSockets.add(socket); - socket.once("close", () => openSockets.delete(socket)); - upstreamAttempts++; - let buf = ""; - socket.on("data", (chunk: Buffer) => { - buf += chunk.toString(); - if (buf.includes("\r\n\r\n")) { - // Send HTTP 200 response headers only — no body — then destroy. - socket.write("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nTransfer-Encoding: chunked\r\n\r\n"); - socket.destroy(); - } - }); - }); - await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); - const port = (server.address() as AddressInfo).port; - cleanups.push(() => new Promise((resolve) => { - for (const s of openSockets) s.destroy(); - server.close(() => resolve()); - })); - - const subswitch = await startSubswitch({ anthropic: { baseUrl: `http://127.0.0.1:${port}` } }); - cleanups.push(subswitch.close); - - try { - await fetch(`${subswitch.url}/v1/messages`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), - }); - } catch { - // Expected: partial response, broken body - } - - await new Promise((resolve) => setTimeout(resolve, 80)); - - // Retry would produce 2 upstream attempts. settled=true must prevent retry. - assert.equal(upstreamAttempts, 1, "relay must NOT retry after response headers were received"); - }); - // --------------------------------------------------------------------------- // L3: x-subswitch-synthesized marker header // --------------------------------------------------------------------------- @@ -822,6 +674,8 @@ describe("anthropic passthrough", () => { // (concurrency gate) — carries x-subswitch-synthesized: 1 so operators can // distinguish relay faults from upstream faults. Responses proxied from the // origin (including upstream errors such as 429) must NOT carry this header. + // Additionally, x-subswitch-synthesized is stripped from proxied responses + // so that an origin setting it cannot impersonate the relay's marker. it("L3: x-subswitch-synthesized: 1 is present on a relay-synthesised 504 timeout response", async () => { const anthropic = await startFakeUpstream((_req, _res) => { @@ -891,6 +745,31 @@ describe("anthropic passthrough", () => { ); }); + it("L3: x-subswitch-synthesized set by the upstream is STRIPPED from the proxied response", async () => { + // An upstream that sets x-subswitch-synthesized must not have it forwarded + // to the client — the relay strips it so the marker is authoritative (only + // the relay can assert it). + const { subswitch } = await setup((_req, res) => { + res.writeHead(200, { "content-type": "application/json", "x-subswitch-synthesized": "upstream-injected" }); + res.end(JSON.stringify({ id: "ok" })); + }); + + const response = await fetch(`${subswitch.url}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "claude-sonnet-4-6", messages: [] }), + }); + + assert.equal(response.status, 200); + // Without fix: "upstream-injected" (forwarded verbatim). + // With fix: null (stripped by filterRawHeaders adding it to HOP_BY_HOP). + assert.equal( + response.headers.get("x-subswitch-synthesized"), + null, + "upstream-set x-subswitch-synthesized must be stripped from the proxied response", + ); + }); + // --------------------------------------------------------------------------- // Change 3: client abort must not produce anthropic_upstream_error warn // ---------------------------------------------------------------------------