diff --git a/.claude/docs/launch-and-wrapper.md b/.claude/docs/launch-and-wrapper.md index 7be78b4a..4b9f246e 100644 --- a/.claude/docs/launch-and-wrapper.md +++ b/.claude/docs/launch-and-wrapper.md @@ -120,11 +120,12 @@ out from under a running child. ## Outbound proxy -`src/outbound-proxy.ts`. When `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` are set in clodex's environment, -`installOutboundProxyDispatcher()` (called at the top of `main()`) installs undici's -`EnvHttpProxyAgent` as the global fetch dispatcher, so every fetch-based call (OAuth device -flow/refresh, model-list and models.dev refresh, AI-SDK upstream calls) honors them. Without proxy -env vars it is a no-op. +`src/outbound-proxy.ts`. `installOutboundDispatcher()` (called at the top of `main()`) always +installs the package undici dispatcher globally with HTTP/2 disabled. It uses `EnvHttpProxyAgent` +when `HTTP_PROXY`/`HTTPS_PROXY` are configured, so every fetch-based call (OAuth device flow/refresh, +model-list and models.dev refresh, AI-SDK upstream calls) honors those variables and `NO_PROXY`; +otherwise it uses a direct `Agent`. Pinning fetch to HTTP/1.1 prevents Node 26's bundled undici 8 +from retaining a destroyed pooled HTTP/2 session and failing every later request to that origin. Transports that do not use the undici dispatcher share the same resolver: the `ws`-based OAuth Responses WebSocket gets an `https-proxy-agent` CONNECT tunnel via `outboundWsProxyAgent()`, and the diff --git a/CLAUDE.md b/CLAUDE.md index bd47c500..c7690526 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,6 +180,12 @@ These bite from outside the subsystem that owns them, so they live here rather t - **`node-gyp-build` is a deliberate direct dependency that no clodex source imports.** Routine "remove the unused dependency" cleanup breaks fresh installs. Reason in `.claude/docs/patcher.md`. +- **clodex always installs package undici's global fetch dispatcher with HTTP/2 disabled** + (`installOutboundDispatcher()` at the top of `main()`), proxy env or not. Node 26's bundled + undici 8 negotiates HTTP/2 and keeps a dead pooled session forever after a fatal TLS alert, so + every request to that origin fails until restart (#233); Node 24 CI cannot see that. Do not gate + the install on proxy env again and do not drop the explicit `allowH2: false` because "undici 7 + already defaults to it" — the option is what survives an undici 8 bump. - **Every AI SDK generation entry point must resolve its timeout and retry budget through `src/upstream-retry.ts`.** Anthropic- and OpenAI-format `streamText` consumers abort at idle and total deadlines; `generateText` consumers abort at total only. Cancellation remains cooperative diff --git a/src/cli.ts b/src/cli.ts index 429f6a2a..a696ba9a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -79,7 +79,7 @@ import { startConfiguredHttpProxy, } from './http-proxy/index.js'; import { runPatchCommand, runLaunchPatchCheck } from './patcher.js'; -import { installOutboundProxyDispatcher } from './outbound-proxy.js'; +import { installOutboundDispatcher } from './outbound-proxy.js'; const STARTER_CLAUDE_FLAGS = new Set(['--dry-run', '--trace', '--fast', '--endpoint', '--proxy', '--save-mode', '--help', '-h', '--version', '-v']); const CLODEX_LAUNCH_FLAGS = new Set(['--provider', '--model', '--context']); @@ -1643,9 +1643,10 @@ export async function runClaudeCommand(parsed: ParsedArgs): Promise { } export async function main(args: string[] = process.argv.slice(2)): Promise { - // Honor HTTP_PROXY/HTTPS_PROXY/NO_PROXY for clodex's own outbound calls - // (no-op when no proxy env var is set; never throws). - await installOutboundProxyDispatcher(); + // Pin clodex's fetch calls to HTTP/1.1 so Node 26 cannot retain a destroyed + // HTTP/2 session, while still honoring HTTP_PROXY/HTTPS_PROXY/NO_PROXY. + // Installation is idempotent and warns rather than throwing on failure. + await installOutboundDispatcher(); const parsed = parseArgs(args); diff --git a/src/http-proxy/server.ts b/src/http-proxy/server.ts index 2c243458..f2f653d4 100644 --- a/src/http-proxy/server.ts +++ b/src/http-proxy/server.ts @@ -1172,6 +1172,10 @@ export async function startHttpProxy(options: HttpProxyOptions): Promise clientSocket.destroy()); const target = authorityParts(req.url ?? ''); if (!target) { clientSocket.end('HTTP/1.1 400 Bad Request\r\n\r\n'); diff --git a/src/outbound-proxy.ts b/src/outbound-proxy.ts index 8c5a055e..8ed32e27 100644 --- a/src/outbound-proxy.ts +++ b/src/outbound-proxy.ts @@ -1,12 +1,17 @@ -// src/outbound-proxy.ts — make clodex's OWN outbound network calls honor -// HTTP_PROXY / HTTPS_PROXY / NO_PROXY. +// src/outbound-proxy.ts — control clodex's OWN outbound fetch transport and +// honor HTTP_PROXY / HTTPS_PROXY / NO_PROXY. // // Node's fetch (undici) ignores proxy env vars by default, so OAuth device // flow/token refresh, model-list refresh, models.dev fetches, and upstream // OpenAI calls made through the AI SDK would all bypass a corporate proxy. -// installOutboundProxyDispatcher() installs undici's EnvHttpProxyAgent as the -// global fetch dispatcher — but only when a proxy env var is actually set, so -// proxy-less environments are completely unaffected. +// installOutboundDispatcher() installs the package undici dispatcher +// globally: EnvHttpProxyAgent when proxy env is configured, or Agent otherwise. +// +// Node 26's bundled undici 8 turns on HTTP/2 in fetch(). If a peer tears down a +// pooled h2 session with a fatal TLS alert, Node marks it destroyed but never +// closes it, undici never evicts it, and every later request to that origin +// fails immediately with ERR_HTTP2_INVALID_SESSION until restart (issue #233). +// Both dispatcher variants therefore disable HTTP/2 explicitly. // // The OAuth Responses WebSocket transport and raw first-party passthrough do // not go through the undici dispatcher. outboundHttpProxyAgent() builds an @@ -109,27 +114,29 @@ export function proxyUrlTargetsListener( let dispatcherInstalled = false; /** Reset the install-once latch (tests only). */ -export function resetOutboundProxyDispatcherForTests(): void { +export function resetOutboundDispatcherForTests(): void { dispatcherInstalled = false; } /** - * Install undici's EnvHttpProxyAgent as the global fetch dispatcher when any - * proxy env var is set. Idempotent. A failure warns and falls back to direct - * connections — it must never break the CLI. + * Install package undici's global fetch dispatcher with HTTP/2 disabled, + * honoring proxy env vars when present. Idempotent. A failure warns and keeps + * Node's existing dispatcher — it must never break the CLI. */ -export async function installOutboundProxyDispatcher(): Promise { +export async function installOutboundDispatcher(): Promise { if (dispatcherInstalled) return true; - if (!hasOutboundProxyEnv()) return false; try { - const { EnvHttpProxyAgent, setGlobalDispatcher } = await import('undici'); - setGlobalDispatcher(new EnvHttpProxyAgent()); + const { Agent, EnvHttpProxyAgent, setGlobalDispatcher } = await import('undici'); + const dispatcher = hasOutboundProxyEnv() + ? new EnvHttpProxyAgent({ allowH2: false }) + : new Agent({ allowH2: false }); + setGlobalDispatcher(dispatcher); dispatcherInstalled = true; return true; } catch (err) { console.error( - 'clodex: HTTP(S)_PROXY is set but installing the outbound proxy dispatcher failed; ' - + `using direct connections (${err instanceof Error ? err.message : String(err)})`, + 'clodex: installing the outbound fetch dispatcher failed; ' + + `continuing with Node's existing dispatcher (${err instanceof Error ? err.message : String(err)})`, ); return false; } diff --git a/src/upstream-forward.ts b/src/upstream-forward.ts index 368f4c8e..ed553767 100644 --- a/src/upstream-forward.ts +++ b/src/upstream-forward.ts @@ -45,7 +45,21 @@ export function anthropicUpstreamHeaders( export class UpstreamUnreachableError extends Error { constructor(cause: unknown) { - super(`Upstream unreachable: ${cause instanceof Error ? cause.message : String(cause)}`); + const detail = cause instanceof Error ? cause.message : String(cause); + const directCode = cause !== null && typeof cause === 'object' + ? (cause as { code?: unknown }).code + : undefined; + const nestedCause = cause instanceof Error ? cause.cause : undefined; + const nestedCode = nestedCause !== null && typeof nestedCause === 'object' + ? (nestedCause as { code?: unknown }).code + : undefined; + const code = typeof directCode === 'string' + ? directCode + : typeof nestedCode === 'string' ? nestedCode : undefined; + const detailWithCode = code && !detail.includes(code) + ? (detail ? `${detail} (${code})` : code) + : detail; + super(`Upstream unreachable: ${detailWithCode}`, { cause }); this.name = 'UpstreamUnreachableError'; } } diff --git a/tests/http-proxy-server.test.ts b/tests/http-proxy-server.test.ts index 138cf0e7..0d7137da 100644 --- a/tests/http-proxy-server.test.ts +++ b/tests/http-proxy-server.test.ts @@ -423,6 +423,83 @@ describe('selective HTTP proxy', () => { } }); + it('handles a client reset in a passthrough CONNECT tunnel and tears down upstream', async () => { + let acceptUpstream!: (socket: net.Socket) => void; + const upstreamAccepted = new Promise(resolve => { acceptUpstream = resolve; }); + const upstreamServer = net.createServer(socket => { + socket.on('error', () => {}); + socket.on('data', data => socket.write(data)); + acceptUpstream(socket); + }); + const upstreamPort = await listen(upstreamServer); + const proxy = await startHttpProxy({ routes: [] }); + const client = net.connect(proxy.port, proxy.host); + client.on('error', () => {}); + const uncaught: Error[] = []; + const onUncaught = (error: Error): void => { uncaught.push(error); }; + process.prependListener('uncaughtException', onUncaught); + let upstreamSocket: net.Socket | undefined; + + try { + await once(client, 'connect'); + client.write( + `CONNECT 127.0.0.1:${upstreamPort} HTTP/1.1\r\n` + + `Host: 127.0.0.1:${upstreamPort}\r\n\r\n`, + ); + const [established] = await once(client, 'data') as [Buffer]; + expect(established.toString()).toContain('200 Connection Established'); + upstreamSocket = await upstreamAccepted; + + client.write('ping'); + const [echoed] = await once(client, 'data') as [Buffer]; + expect(echoed.toString()).toBe('ping'); + const upstreamClosed = once(upstreamSocket, 'close'); + client.resetAndDestroy(); + await Promise.race([ + upstreamClosed, + new Promise((_, reject) => setTimeout( + () => reject(new Error('upstream tunnel socket did not close after client reset')), + 1_000, + )), + ]); + await new Promise(resolve => setImmediate(resolve)); + + expect(uncaught).toEqual([]); + expect(upstreamSocket.destroyed).toBe(true); + } finally { + process.off('uncaughtException', onUncaught); + client.destroy(); + upstreamSocket?.destroy(); + await proxy.close(); + await new Promise(resolve => upstreamServer.close(() => resolve())); + } + }); + + it('handles a client reset while answering a malformed CONNECT authority', async () => { + const proxy = await startHttpProxy({ routes: [] }); + const client = net.connect(proxy.port, proxy.host); + client.on('error', () => {}); + const uncaught: Error[] = []; + const onUncaught = (error: Error): void => { uncaught.push(error); }; + process.prependListener('uncaughtException', onUncaught); + + try { + await once(client, 'connect'); + // '[' is not a valid authority, so the handler takes the 400 branch. + client.write('CONNECT [ HTTP/1.1\r\nHost: x\r\n\r\n'); + // Reset before the 400 is written so the write hits a dead socket. + await new Promise(resolve => setImmediate(resolve)); + client.resetAndDestroy(); + await new Promise(resolve => setTimeout(resolve, 200)); + + expect(uncaught).toEqual([]); + } finally { + process.off('uncaughtException', onUncaught); + client.destroy(); + await proxy.close(); + } + }); + it('forwards first-party request bytes and auth unchanged', async () => { const certificates = ensureHttpProxyCertificates(); const inferenceLogPath = join(testHome, 'anthropic-inference.jsonl'); diff --git a/tests/outbound-proxy.test.ts b/tests/outbound-proxy.test.ts index fceed5de..2e829d2f 100644 --- a/tests/outbound-proxy.test.ts +++ b/tests/outbound-proxy.test.ts @@ -1,16 +1,96 @@ // tests/outbound-proxy.test.ts -import { describe, it, expect, vi } from 'vitest'; +import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; +import { once } from 'node:events'; +import * as http from 'node:http'; +import * as http2 from 'node:http2'; +import * as net from 'node:net'; +import type { AddressInfo } from 'node:net'; +import { Agent, getGlobalDispatcher, setGlobalDispatcher, type Dispatcher } from 'undici'; +import { ensureHttpProxyCertificates } from '../src/http-proxy/ca.js'; import { hasOutboundProxyEnv, + installOutboundDispatcher, noProxyBypasses, outboundHttpProxyAgent, outboundProxyUrlForTarget, outboundWsProxyAgent, proxyUrlTargetsListener, + resetOutboundDispatcherForTests, } from '../src/outbound-proxy.js'; const PROXY = 'http://127.0.0.1:8888'; +const dispatcherEnvNames = [ + 'HTTPS_PROXY', + 'https_proxy', + 'HTTP_PROXY', + 'http_proxy', + 'NO_PROXY', + 'no_proxy', + 'NODE_TLS_REJECT_UNAUTHORIZED', +] as const; +let originalDispatcher: Dispatcher; +let originalDispatcherEnv: Record; +const testDispatchers = new Set(); +const testServers = new Set(); +const testSockets = new Set(); +const h2Sessions = new Set(); + +async function listen(server: net.Server): Promise { + testServers.add(server); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + return (server.address() as AddressInfo).port; +} + +beforeEach(() => { + originalDispatcher = getGlobalDispatcher(); + originalDispatcherEnv = Object.fromEntries( + dispatcherEnvNames.map(name => [name, process.env[name]]), + ) as Record; + for (const name of dispatcherEnvNames) delete process.env[name]; + resetOutboundDispatcherForTests(); +}); + +afterEach(async () => { + resetOutboundDispatcherForTests(); + setGlobalDispatcher(originalDispatcher); + for (const dispatcher of testDispatchers) await dispatcher.destroy().catch(() => {}); + testDispatchers.clear(); + for (const socket of testSockets) socket.destroy(); + testSockets.clear(); + for (const session of h2Sessions) session.destroy(); + h2Sessions.clear(); + for (const server of testServers) { + await new Promise(resolve => server.close(() => resolve())); + } + testServers.clear(); + for (const name of dispatcherEnvNames) { + const value = originalDispatcherEnv[name]; + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } +}); + +function startVersionServer(versions: string[]): http2.Http2SecureServer { + const certificates = ensureHttpProxyCertificates(); + const server = http2.createSecureServer({ + key: certificates.serverKey, + cert: certificates.serverCert, + allowHTTP1: true, + }); + server.on('session', session => { + h2Sessions.add(session); + session.once('close', () => h2Sessions.delete(session)); + }); + server.on('request', (req, res) => { + versions.push(req.httpVersion); + res.writeHead(200, { 'content-type': 'text/plain' }); + res.end('ok'); + }); + return server; +} + describe('hasOutboundProxyEnv', () => { it('is false with no proxy vars or blank values', () => { expect(hasOutboundProxyEnv({})).toBe(false); @@ -151,3 +231,73 @@ describe('noProxyBypasses', () => { expect(noProxyBypasses('e.test', {})).toBe(false); }); }); + +describe('installOutboundDispatcher', () => { + it('pins global fetch to HTTP/1.1 even when the previous dispatcher enables HTTP/2', async () => { + const versions: string[] = []; + const upstream = startVersionServer(versions); + const upstreamPort = await listen(upstream); + process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; + + // Control leg: reproduce Node 26's default, where the bundled undici 8 + // negotiates HTTP/2. The built-in fetch reads this package agent directly + // on undici 7; if the pinned undici is bumped to 8.x, Node <=24's fetch + // wraps it in a Dispatcher1Wrapper that forces HTTP/1.1, so this control + // needs Node 26 to observe '2.0'. `allowH2: false` in the installer is + // inert on undici 7 (already the default) and is exactly what keeps the + // fix in place on 8.x -- this test is the tripwire for that bump. + const h2Dispatcher = new Agent({ allowH2: true, connect: { rejectUnauthorized: false } }); + testDispatchers.add(h2Dispatcher); + setGlobalDispatcher(h2Dispatcher); + expect(await (await fetch(`https://127.0.0.1:${upstreamPort}`)).text()).toBe('ok'); + + resetOutboundDispatcherForTests(); + await installOutboundDispatcher(); + testDispatchers.add(getGlobalDispatcher()); + expect(await (await fetch(`https://127.0.0.1:${upstreamPort}`)).text()).toBe('ok'); + + expect(versions).toEqual(['2.0', '1.1']); + }); + + it('still sends HTTPS fetches through the configured CONNECT proxy', async () => { + const versions: string[] = []; + const upstream = startVersionServer(versions); + const upstreamPort = await listen(upstream); + const connectTargets: string[] = []; + const tunnelSockets = new Set(); + const connectProxy = http.createServer(); + connectProxy.on('connect', (req, clientSocket, head) => { + connectTargets.push(req.url ?? ''); + const upstreamSocket = net.connect(upstreamPort, '127.0.0.1'); + testSockets.add(clientSocket); + testSockets.add(upstreamSocket); + tunnelSockets.add(upstreamSocket); + upstreamSocket.once('close', () => { + tunnelSockets.delete(upstreamSocket); + testSockets.delete(upstreamSocket); + }); + clientSocket.once('close', () => testSockets.delete(clientSocket)); + upstreamSocket.once('connect', () => { + clientSocket.write('HTTP/1.1 200 Connection Established\r\n\r\n'); + if (head.length > 0) upstreamSocket.write(head); + clientSocket.pipe(upstreamSocket); + upstreamSocket.pipe(clientSocket); + }); + upstreamSocket.once('error', () => clientSocket.destroy()); + clientSocket.once('error', () => upstreamSocket.destroy()); + clientSocket.once('close', () => upstreamSocket.destroy()); + }); + await listen(connectProxy); + const proxyPort = (connectProxy.address() as AddressInfo).port; + process.env['HTTPS_PROXY'] = `http://127.0.0.1:${proxyPort}`; + process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; + + await installOutboundDispatcher(); + testDispatchers.add(getGlobalDispatcher()); + expect(await (await fetch(`https://127.0.0.1:${upstreamPort}`)).text()).toBe('ok'); + + expect(connectTargets).toEqual([`127.0.0.1:${upstreamPort}`]); + expect(versions).toEqual(['1.1']); + for (const socket of tunnelSockets) socket.destroy(); + }); +}); diff --git a/tests/upstream-forward.test.ts b/tests/upstream-forward.test.ts index 8877c17e..97e42697 100644 --- a/tests/upstream-forward.test.ts +++ b/tests/upstream-forward.test.ts @@ -6,6 +6,7 @@ import { fetchWithOAuthRetry, anthropicSseModelRewrite, relayAnthropicMessages, + UpstreamUnreachableError, } from '../src/upstream-forward.js'; describe('anthropicUpstreamHeaders', () => { @@ -83,6 +84,54 @@ describe('anthropicUpstreamHeaders', () => { }); }); +describe('UpstreamUnreachableError', () => { + it('preserves the fetch error and adds its nested network code to the generic message', () => { + const networkCause = Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:443'), { + code: 'ECONNREFUSED', + }); + const fetchError = new TypeError('fetch failed', { cause: networkCause }); + + const error = new UpstreamUnreachableError(fetchError); + + expect(error.cause).toBe(fetchError); + expect(error.message).toBe('Upstream unreachable: fetch failed (ECONNREFUSED)'); + }); + + it('does not repeat a code already present in the cause message', () => { + const cause = Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:443'), { + code: 'ECONNREFUSED', + }); + + const error = new UpstreamUnreachableError(cause); + + expect(error.cause).toBe(cause); + expect(error.message).toBe('Upstream unreachable: connect ECONNREFUSED 127.0.0.1:443'); + }); + + it('adds a code carried directly on the cause', () => { + const cause = Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }); + + const error = new UpstreamUnreachableError(cause); + + expect(error.message).toBe('Upstream unreachable: socket hang up (ECONNRESET)'); + }); + + it('falls back to the code alone when the cause message is empty', () => { + const cause = Object.assign(new Error(''), { code: 'ETIMEDOUT' }); + + expect(new UpstreamUnreachableError(cause).message).toBe('Upstream unreachable: ETIMEDOUT'); + }); + + it('preserves and describes a non-Error cause', () => { + const cause = 'connection unavailable'; + + const error = new UpstreamUnreachableError(cause); + + expect(error.cause).toBe(cause); + expect(error.message).toBe('Upstream unreachable: connection unavailable'); + }); +}); + describe('fetchWithOAuthRetry', () => { it('refreshes once on 401 and retries with the refreshed token', async () => { const refreshToken = vi.fn(async () => 'new-token');