d496780d43741d1145febcdbbd624eba14e455d3
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
7611076436 |
fix(proxy): reuse upstream connections for OpenCode API requests (#2916)
* fix(proxy): reuse upstream connections for OpenCode API requests `createProxyMiddleware` was constructed without an `agent`, so `http-proxy` fell back to `agent: false`. That disables connection pooling and forces `Connection: close` on every proxied request, consuming one ephemeral port per request. Measured against a real `opencode serve` instance, 200 sequential requests through the proxy created 201 TIME_WAIT entries (1.005 ports/request). With a keep-alive agent the same load creates 0. On macOS the ephemeral range is 16,384 ports and TIME_WAIT lasts 30s, so sustained traffic around 546 req/sec exhausts the pool — after which every process on the host fails to open outbound connections with EADDRNOTAVAIL. `maxSockets: Infinity` preserves the unbounded concurrency of `agent: false`, so this changes connection reuse only, not request throughput. Partially addresses #2915. * fix(proxy): derive proxy agent class from the target scheme Addresses review feedback on #2916. The first commit created an unconditional `http.Agent`, which regresses external OpenCode servers configured over https via `OPENCODE_HOST` (accepted by env-config.js). http-proxy dispatches through `https.request` when the target protocol is `https:` (http-proxy/lib/http-proxy/passes/web-incoming.js:126), and `http.Agent#createConnection` is plain `net.createConnection` — so an http.Agent would open a plaintext socket to a TLS port and fail every proxied request. `agent: false` previously worked for both schemes. `createOpenCodeProxyAgent(target)` now returns an `https.Agent` for https targets and an `http.Agent` otherwise, derived once from `resolveProxyTarget()` at registration so the single shared instance is preserved across `apiProxy` and `interactiveOAuthProxy`. Guarded in both test layers, verified to fail when the selection is reverted to an unconditional http.Agent. `https.Agent` extends `http.Agent`, so the http cases assert `not.toBeInstanceOf(https.Agent)`. * Round 2: fix: resolve the proxy agent lazily so cold starts honor https Addresses the round-2 blocker on #2916. Deriving the agent class at registration is too early: startup-pipeline-runtime.js calls setupProxy() (line 104) before bootstrapOpenCodeAtStartup() (line 141), so on a fresh process state.openCodePort is null, buildOpenCodeUrl() throws (network-runtime.js:86-88), and resolveProxyTarget() returns the http loopback fallback. An external server configured via OPENCODE_HOST=https:// only appears on state.openCodeBaseUrl after bootstrap, so it was still getting a plain http.Agent — the regression the previous commit intended to fix. `agent` is now a getter backed by a per-scheme memoizing resolver. http-proxy-middleware rebuilds per-request options with `Object.assign({}, this.proxyOptions)` in prepareProxyRequest, which invokes getters, so resolution happens at request time while still yielding one shared pool per scheme. Tests now model the production ordering — registration while the port is null and buildOpenCodeUrl throws, then an https base URL appearing after bootstrap — and fail against the eager implementation. A behavioral test pins the http-proxy-middleware option re-read the fix depends on, so a library change that froze options would fail loudly instead of silently regressing https targets. The resolver is module-private; `bun run dead-code` flagged it as an unused export when it was exported. * Round 3: docs(changelog): note upstream connection reuse under [Unreleased] Repo precedent adds [Unreleased] bullets for comparable proxy/stability fixes (1.18.4 Stability, 1.9.3 Reliability/Proxy). Non-blocker raised in review on #2916. * Round 3: docs(changelog): use repo-standard 'behavior' spelling * Round 4: docs(changelog): don't imply a restart is the only recovery The ephemeral port pool drains on its own once the exhausting traffic stops (TIME_WAIT expiry), so a restart is sufficient but not necessary. Optional nit raised in review on #2916. * Round 5: fix: construct the proxy agent through one factory; widen the pool Review found the https branch was mutation-uncovered: the resolver re-implemented agent construction inline instead of calling the exported `createOpenCodeProxyAgent(target)`, so replacing its https branch with `new https.Agent()` — dropping OPENCODE_AGENT_OPTIONS, and with it keep-alive — left the entire suite green. Since `createOpenCodeProxyAgent` also had no production callers, its four tests were pinning dead code. Delegating collapses both: the factory is now the single construction path, and the mutation fails 2 tests including the live resolver path. Also from review: - maxFreeSockets 32 -> 256 (Node's own default). The lower cap evicted pooled sockets under concurrency, reintroducing the churn this agent exists to prevent: at 64 concurrent requests it left 303 sockets in TIME_WAIT versus 0 at 256. - Added `timeout` to OPENCODE_AGENT_OPTIONS. Free-socket eviction is governed by agent.options.timeout, which was unset, so idle sockets persisted until the peer closed them. `keepAliveMsecs` is the TCP probe delay, not the idle lifetime. - resolveProxyTarget() now checks openCodePort before calling buildOpenCodeUrl instead of relying on it throwing. The port is nulled on several runtime paths (health-check failure, failed restart), so a degraded OpenCode made every proxied request pay for a thrown-and-caught exception — and the getter added a second call per request. - Test fixtures use :4096 rather than :443; WHATWG URL elides the default port, so parseInt('') is NaN and env-config rejects that host. The fixtures modeled a state that cannot reach production. - The getter-read assertion is now exact (0 at construction, 1, then 2) rather than >= 2, which would have passed if the getter were read twice at construction and never per-request. - listen() rejects on 'error' and servers start inside try/finally, so a bind failure fails the test instead of hanging to timeout. |