* 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.
253 lines
8.4 KiB
JavaScript
253 lines
8.4 KiB
JavaScript
import http from 'node:http';
|
|
import https from 'node:https';
|
|
|
|
import { createProxyMiddleware } from 'http-proxy-middleware';
|
|
import { describe, expect, it } from 'vitest';
|
|
|
|
import {
|
|
createDirectoryQueryCanonicalizer,
|
|
createOpenCodeProxyAgent,
|
|
normalizeForwardedDirectoryHeaders,
|
|
} from './proxy.js';
|
|
|
|
describe('createDirectoryQueryCanonicalizer', () => {
|
|
it('canonicalizes directory query params and preserves other params', async () => {
|
|
const canonicalize = createDirectoryQueryCanonicalizer({
|
|
realpath: async (value) => value === '/link/project' ? '/real/project' : value,
|
|
});
|
|
|
|
await expect(canonicalize('/session?foo=1&directory=/link/project&bar=2'))
|
|
.resolves.toBe('/session?foo=1&directory=%2Freal%2Fproject&bar=2');
|
|
});
|
|
|
|
it('caches directory realpath lookups', async () => {
|
|
let calls = 0;
|
|
const canonicalize = createDirectoryQueryCanonicalizer({
|
|
realpath: async () => {
|
|
calls += 1;
|
|
return '/real/project';
|
|
},
|
|
});
|
|
|
|
await expect(canonicalize('/session?directory=/link/project')).resolves.toBe('/session?directory=%2Freal%2Fproject');
|
|
await expect(canonicalize('/session?directory=/link/project')).resolves.toBe('/session?directory=%2Freal%2Fproject');
|
|
expect(calls).toBe(1);
|
|
});
|
|
|
|
it('deduplicates concurrent directory realpath lookups', async () => {
|
|
let calls = 0;
|
|
let release = () => undefined;
|
|
const pending = new Promise((resolve) => {
|
|
release = () => resolve('/real/project');
|
|
});
|
|
const canonicalize = createDirectoryQueryCanonicalizer({
|
|
realpath: async () => {
|
|
calls += 1;
|
|
return pending;
|
|
},
|
|
});
|
|
|
|
const first = canonicalize('/session?directory=/link/project');
|
|
const second = canonicalize('/session?directory=/link/project');
|
|
await Promise.resolve();
|
|
|
|
expect(calls).toBe(1);
|
|
release();
|
|
await expect(Promise.all([first, second])).resolves.toEqual([
|
|
'/session?directory=%2Freal%2Fproject',
|
|
'/session?directory=%2Freal%2Fproject',
|
|
]);
|
|
});
|
|
|
|
it('falls back to the original URL when realpath fails', async () => {
|
|
const canonicalize = createDirectoryQueryCanonicalizer({
|
|
realpath: async () => {
|
|
throw new Error('missing');
|
|
},
|
|
});
|
|
|
|
await expect(canonicalize('/session?foo=1&directory=/missing/project'))
|
|
.resolves.toBe('/session?foo=1&directory=/missing/project');
|
|
});
|
|
|
|
it('leaves URLs without directory params unchanged', async () => {
|
|
const canonicalize = createDirectoryQueryCanonicalizer({
|
|
realpath: async () => '/real/project',
|
|
});
|
|
|
|
await expect(canonicalize('/session?foo=1')).resolves.toBe('/session?foo=1');
|
|
});
|
|
});
|
|
|
|
describe('normalizeForwardedDirectoryHeaders', () => {
|
|
it('decodes marked directory headers before forwarding to OpenCode', () => {
|
|
const headers = normalizeForwardedDirectoryHeaders({
|
|
'x-opencode-directory': encodeURIComponent('/Users/example/project'),
|
|
'x-opencode-directory-encoding': 'uri',
|
|
});
|
|
|
|
expect(headers).toEqual({
|
|
'x-opencode-directory': '/Users/example/project',
|
|
});
|
|
});
|
|
|
|
it('preserves unmarked percent sequences from direct clients', () => {
|
|
const headers = normalizeForwardedDirectoryHeaders({
|
|
'x-opencode-directory': '/Users/example/project%20literal',
|
|
});
|
|
|
|
expect(headers).toEqual({
|
|
'x-opencode-directory': '/Users/example/project%20literal',
|
|
});
|
|
});
|
|
});
|
|
|
|
const listen = (server) => new Promise((resolve, reject) => {
|
|
server.once('error', reject);
|
|
server.listen(0, '127.0.0.1', () => {
|
|
server.removeListener('error', reject);
|
|
resolve(server.address().port);
|
|
});
|
|
});
|
|
|
|
const closeServer = (server) => new Promise((resolve) => {
|
|
server.close(resolve);
|
|
});
|
|
|
|
const request = (port, agent) => new Promise((resolve, reject) => {
|
|
const req = http.request({ host: '127.0.0.1', port, path: '/', method: 'GET', agent }, (res) => {
|
|
res.resume();
|
|
res.on('end', resolve);
|
|
res.on('error', reject);
|
|
});
|
|
req.on('error', reject);
|
|
req.end();
|
|
});
|
|
|
|
/**
|
|
* Proxies two sequential requests through `createProxyMiddleware` and reports
|
|
* what the upstream server observed for each one.
|
|
*/
|
|
const proxyTwoRequests = async (proxyAgent) => {
|
|
const seen = [];
|
|
let middleware;
|
|
const upstream = http.createServer((req, res) => {
|
|
seen.push({ connection: req.headers.connection, remotePort: req.socket.remotePort });
|
|
res.end('ok');
|
|
});
|
|
const front = http.createServer((req, res) => {
|
|
middleware(req, res, () => {
|
|
res.statusCode = 502;
|
|
res.end();
|
|
});
|
|
});
|
|
const clientAgent = new http.Agent({ keepAlive: true });
|
|
|
|
try {
|
|
const upstreamPort = await listen(upstream);
|
|
middleware = createProxyMiddleware({
|
|
target: `http://127.0.0.1:${upstreamPort}`,
|
|
...(proxyAgent ? { agent: proxyAgent } : {}),
|
|
});
|
|
|
|
const frontPort = await listen(front);
|
|
await request(frontPort, clientAgent);
|
|
await request(frontPort, clientAgent);
|
|
} finally {
|
|
clientAgent.destroy();
|
|
proxyAgent?.destroy();
|
|
await closeServer(front);
|
|
await closeServer(upstream);
|
|
}
|
|
|
|
return seen;
|
|
};
|
|
|
|
describe('createOpenCodeProxyAgent', () => {
|
|
it('reuses a single upstream socket across sequential proxied requests', async () => {
|
|
const seen = await proxyTwoRequests(createOpenCodeProxyAgent('http://127.0.0.1'));
|
|
|
|
expect(seen).toHaveLength(2);
|
|
expect(seen[0].connection).not.toBe('close');
|
|
expect(seen[1].remotePort).toBe(seen[0].remotePort);
|
|
});
|
|
|
|
it('without an agent, http-proxy forces Connection: close and a new socket per request', async () => {
|
|
const seen = await proxyTwoRequests(null);
|
|
|
|
expect(seen).toHaveLength(2);
|
|
expect(seen[0].connection).toBe('close');
|
|
expect(seen[1].remotePort).not.toBe(seen[0].remotePort);
|
|
});
|
|
|
|
// http-proxy dispatches through `https.request` when the target protocol is
|
|
// `https:`, so an http.Agent would open a plaintext socket to a TLS port.
|
|
// External OpenCode servers can be configured over https via OPENCODE_HOST.
|
|
it('returns an https agent for https targets', () => {
|
|
const agent = createOpenCodeProxyAgent('https://opencode.example.com:4096');
|
|
|
|
expect(agent).toBeInstanceOf(https.Agent);
|
|
expect(agent.options.keepAlive).toBe(true);
|
|
});
|
|
|
|
it('returns a plain http agent for http targets', () => {
|
|
const agent = createOpenCodeProxyAgent('http://127.0.0.1:4096');
|
|
|
|
// https.Agent extends http.Agent, so the negative assertion is the load-bearing one.
|
|
expect(agent).toBeInstanceOf(http.Agent);
|
|
expect(agent).not.toBeInstanceOf(https.Agent);
|
|
expect(agent.options.keepAlive).toBe(true);
|
|
});
|
|
|
|
it('falls back to an http agent for missing or unparseable targets', () => {
|
|
expect(createOpenCodeProxyAgent(undefined)).not.toBeInstanceOf(https.Agent);
|
|
expect(createOpenCodeProxyAgent('not a url')).not.toBeInstanceOf(https.Agent);
|
|
});
|
|
|
|
// The cold-start fix relies on http-proxy-middleware rebuilding its per-request
|
|
// options via `Object.assign({}, this.proxyOptions)` in prepareProxyRequest,
|
|
// which invokes getters. If that ever changes to a cached or shallow-reference
|
|
// copy, the agent would freeze at its registration-time value and https targets
|
|
// would silently regress — so pin the behavior here against the real library.
|
|
it('http-proxy-middleware re-reads the agent option on every proxied request', async () => {
|
|
let reads = 0;
|
|
let middleware;
|
|
const agent = createOpenCodeProxyAgent('http://127.0.0.1');
|
|
const upstream = http.createServer((_req, res) => res.end('ok'));
|
|
const front = http.createServer((req, res) => {
|
|
middleware(req, res, () => {
|
|
res.statusCode = 502;
|
|
res.end();
|
|
});
|
|
});
|
|
const clientAgent = new http.Agent({ keepAlive: true });
|
|
|
|
try {
|
|
const upstreamPort = await listen(upstream);
|
|
middleware = createProxyMiddleware({
|
|
target: `http://127.0.0.1:${upstreamPort}`,
|
|
get agent() {
|
|
reads += 1;
|
|
return agent;
|
|
},
|
|
});
|
|
|
|
// Construction itself must not read the getter — otherwise the assertion
|
|
// below could be satisfied without any per-request resolution happening.
|
|
expect(reads).toBe(0);
|
|
|
|
const frontPort = await listen(front);
|
|
await request(frontPort, clientAgent);
|
|
expect(reads).toBe(1);
|
|
|
|
await request(frontPort, clientAgent);
|
|
expect(reads).toBe(2);
|
|
} finally {
|
|
clientAgent.destroy();
|
|
agent.destroy();
|
|
await closeServer(front);
|
|
await closeServer(upstream);
|
|
}
|
|
});
|
|
});
|