diff --git a/changelog.d/fixes/7003-jetbrains-ai-loopback-connect.md b/changelog.d/fixes/7003-jetbrains-ai-loopback-connect.md new file mode 100644 index 0000000000..bc3b0a89c4 --- /dev/null +++ b/changelog.d/fixes/7003-jetbrains-ai-loopback-connect.md @@ -0,0 +1 @@ +- fix(api): raise the main server's `keepAliveTimeout`/`headersTimeout` well above Node's 5s default so pooled keep-alive clients (e.g. JetBrains AI Assistant's JVM `HttpClient`) stop getting 0 bytes back on a reused connection (#7003) diff --git a/scripts/dev/run-next.mjs b/scripts/dev/run-next.mjs index 68eb9d2198..fffdb06cfb 100644 --- a/scripts/dev/run-next.mjs +++ b/scripts/dev/run-next.mjs @@ -14,6 +14,7 @@ import headResponseGuard from "./head-response-guard.cjs"; import { ensureNativeSqlite } from "./ensure-native-sqlite.mjs"; import { isTurbopackCacheCorruption, purgeAllTurbopackCaches } from "./turbopackCacheHeal.mjs"; import { randomUUID } from "node:crypto"; +import { getMainServerTimeoutConfig } from "../../src/shared/utils/runtimeTimeouts.ts"; const { maybeHandleDisallowedMethod } = methodGuard; const { wrapRequestListenerWithHeadResponseGuard } = headResponseGuard; @@ -153,6 +154,15 @@ async function start() { return requestHandler(req, res); }) ); + // Node's http.Server default keepAliveTimeout (5_000ms) races pooled + // keep-alive HTTP clients that idle longer than that between requests (e.g. + // the JVM java.net.http.HttpClient used by JetBrains AI Assistant), which + // reuse a socket the server already tore down and get 0 response bytes back + // (#7003). Raise both timeouts well above any realistic client idle-pool + // window, mirroring src/lib/apiBridgeServer.ts's pattern. + const mainServerTimeouts = getMainServerTimeoutConfig(); + server.keepAliveTimeout = mainServerTimeouts.keepAliveTimeoutMs; + server.headersTimeout = mainServerTimeouts.headersTimeoutMs; server.on("upgrade", async (req, socket, head) => { try { const responsesWsHandled = await responsesWsProxy.handleUpgrade(req, socket, head); diff --git a/scripts/dev/standalone-server-ws.mjs b/scripts/dev/standalone-server-ws.mjs index 3d7bd1f0ca..ebbca99936 100644 --- a/scripts/dev/standalone-server-ws.mjs +++ b/scripts/dev/standalone-server-ws.mjs @@ -7,6 +7,7 @@ import { maybeHandleWebdav } from "./webdav-handler.mjs"; import methodGuard from "./http-method-guard.cjs"; import headResponseGuard from "./head-response-guard.cjs"; import { resolveTlsOptions, createServerListener } from "./tls-options.mjs"; +import { getMainServerTimeoutConfig } from "../../src/shared/utils/runtimeTimeouts.ts"; const originalCreateServer = http.createServer.bind(http); const proxiesByPort = new Map(); @@ -151,6 +152,19 @@ http.createServer = function createServerWithResponsesWs(...args) { // listener); otherwise the original http.Server. The downstream .on/.addListener // patches below apply identically to both (https.Server extends http.Server). const server = createServerListener(args, tlsOptions, { createHttp: originalCreateServer }); + // Node's http.Server default keepAliveTimeout (5_000ms) races pooled + // keep-alive HTTP clients that idle longer than that between requests (e.g. + // the JVM java.net.http.HttpClient used by JetBrains AI Assistant), which + // reuse a socket the server already tore down and get 0 response bytes back + // (#7003). This wrapper is what `omniroute serve` / Docker / Electron actually + // spawn in production (run-standalone.mjs prefers server-ws.mjs over the bare + // Next server.js), so it needs the same fix already wired into run-next.mjs + // (the dev-only entry point) — otherwise real installs never got it. Raise + // both timeouts well above any realistic client idle-pool window, mirroring + // src/lib/apiBridgeServer.ts's pattern. + const mainServerTimeouts = getMainServerTimeoutConfig(); + server.keepAliveTimeout = mainServerTimeouts.keepAliveTimeoutMs; + server.headersTimeout = mainServerTimeouts.headersTimeoutMs; const originalOn = server.on.bind(server); const originalAddListener = server.addListener.bind(server); diff --git a/src/shared/utils/runtimeTimeouts.ts b/src/shared/utils/runtimeTimeouts.ts index 294bb53b8a..cd148d9177 100644 --- a/src/shared/utils/runtimeTimeouts.ts +++ b/src/shared/utils/runtimeTimeouts.ts @@ -19,6 +19,14 @@ export const DEFAULT_API_BRIDGE_SERVER_REQUEST_TIMEOUT_MS = 300_000; export const DEFAULT_API_BRIDGE_SERVER_HEADERS_TIMEOUT_MS = 60_000; export const DEFAULT_API_BRIDGE_SERVER_KEEPALIVE_TIMEOUT_MS = 5_000; export const DEFAULT_API_BRIDGE_SERVER_SOCKET_TIMEOUT_MS = 0; +// Node's http.Server default keepAliveTimeout is 5_000ms with no Keep-Alive +// response header hint. Pooled keep-alive clients that don't race that exact +// window (e.g. the JVM java.net.http.HttpClient used by JetBrains AI +// Assistant) can reuse a socket the server has already torn down, getting 0 +// response bytes back (#7003). Raise both well above any realistic client +// idle-pool window, mirroring the API bridge server's pattern. +export const DEFAULT_MAIN_SERVER_KEEPALIVE_TIMEOUT_MS = 65_000; +export const DEFAULT_MAIN_SERVER_HEADERS_TIMEOUT_MS = 66_000; function hasEnvValue(env: EnvSource, name: string): boolean { const raw = env[name]; @@ -49,6 +57,11 @@ export type ApiBridgeTimeoutConfig = { serverSocketTimeoutMs: number; }; +export type MainServerTimeoutConfig = { + keepAliveTimeoutMs: number; + headersTimeoutMs: number; +}; + function readTimeoutMs( env: EnvSource, name: string, @@ -255,3 +268,37 @@ export function getApiBridgeTimeoutConfig( ), }; } + +export function getMainServerTimeoutConfig( + env: EnvSource = process.env, + logger?: TimeoutLogger +): MainServerTimeoutConfig { + const keepAliveTimeoutMs = readTimeoutMs( + env, + "MAIN_SERVER_KEEPALIVE_TIMEOUT_MS", + DEFAULT_MAIN_SERVER_KEEPALIVE_TIMEOUT_MS, + { + allowZero: true, + logger, + } + ); + const headersTimeoutMs = readTimeoutMs( + env, + "MAIN_SERVER_HEADERS_TIMEOUT_MS", + DEFAULT_MAIN_SERVER_HEADERS_TIMEOUT_MS, + { + allowZero: true, + logger, + } + ); + + return { + keepAliveTimeoutMs, + // Node requires headersTimeout > keepAliveTimeout to avoid its internal + // race-condition warning; keep both configurable but always coherent. + headersTimeoutMs: + headersTimeoutMs > 0 && keepAliveTimeoutMs > 0 + ? Math.max(headersTimeoutMs, keepAliveTimeoutMs + 1_000) + : headersTimeoutMs, + }; +} diff --git a/tests/unit/main-server-keepalive-timeout-7003.test.ts b/tests/unit/main-server-keepalive-timeout-7003.test.ts new file mode 100644 index 0000000000..fd20f19774 --- /dev/null +++ b/tests/unit/main-server-keepalive-timeout-7003.test.ts @@ -0,0 +1,202 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import net from "node:net"; +import { getMainServerTimeoutConfig } from "../../src/shared/utils/runtimeTimeouts.ts"; + +// #7003 — JetBrains AI Assistant ("Test Connection" / completions) reported +// "HTTP/1.1 header parser received no bytes". The main OmniRoute server +// (scripts/dev/run-next.mjs) boots a bare `http.createServer(...)` and never +// configures `keepAliveTimeout`/`headersTimeout`, leaving Node's http.Server +// default of keepAliveTimeout=5_000ms with no `Keep-Alive: timeout=N` response +// hint. JetBrains AI Assistant's JVM `java.net.http.HttpClient` connection pool +// can reuse a socket idle for longer than that window; the server has already +// torn the socket down, so the client gets 0 response bytes back instead of a +// fresh HTTP response. +// +// This spec proves both halves: +// 1. `getMainServerTimeoutConfig()` raises the defaults well above Node's +// unconfigured 5_000ms window (the actual fix wired into run-next.mjs). +// 2. A bare http.Server left at Node's defaults drops a socket reused after +// an idle gap past 5s, while the same server configured via +// `getMainServerTimeoutConfig()` keeps serving the reused connection. + +describe("#7003 getMainServerTimeoutConfig", () => { + it("defaults keepAliveTimeout/headersTimeout well above Node's 5_000ms default", () => { + const config = getMainServerTimeoutConfig({}); + assert.equal(config.keepAliveTimeoutMs, 65_000); + assert.equal(config.headersTimeoutMs, 66_000); + assert.ok(config.keepAliveTimeoutMs > 5_000, "must exceed Node's unconfigured default"); + assert.ok( + config.headersTimeoutMs > config.keepAliveTimeoutMs, + "headersTimeout must stay above keepAliveTimeout per Node's own requirement" + ); + }); + + it("honors env overrides and keeps headersTimeout coherent with a raised keepAliveTimeout", () => { + const config = getMainServerTimeoutConfig({ + MAIN_SERVER_KEEPALIVE_TIMEOUT_MS: "120000", + MAIN_SERVER_HEADERS_TIMEOUT_MS: "121000", + }); + assert.equal(config.keepAliveTimeoutMs, 120_000); + assert.equal(config.headersTimeoutMs, 121_000); + }); + + it("bumps an inconsistent explicit headersTimeout override above keepAliveTimeout", () => { + const config = getMainServerTimeoutConfig({ + MAIN_SERVER_KEEPALIVE_TIMEOUT_MS: "120000", + MAIN_SERVER_HEADERS_TIMEOUT_MS: "1000", + }); + assert.equal(config.keepAliveTimeoutMs, 120_000); + assert.equal(config.headersTimeoutMs, 121_000); + }); + + it("falls back to defaults on invalid env values", () => { + const config = getMainServerTimeoutConfig({ + MAIN_SERVER_KEEPALIVE_TIMEOUT_MS: "not-a-number", + }); + assert.equal(config.keepAliveTimeoutMs, 65_000); + }); +}); + +/** + * Sends a raw HTTP/1.1 GET over an already-connected keep-alive socket and + * resolves with whatever bytes arrive within a short settle window (empty + * string if nothing comes back — the exact "0 bytes back" failure mode + * JetBrains AI Assistant surfaces as "header parser received no bytes"). + * + * The socket is opened with `allowHalfOpen: true` so it faithfully mimics a + * JVM/OkHttp-style client: Node's default `allowHalfOpen: false` proactively + * ends the writable side the instant it processes an incoming FIN, turning + * the reused write into a synchronous "socket has been ended" error instead + * of the real-world race — a write that is accepted locally (the server + * already destroyed the connection, so it never arrives) whose response + * settles as 0 bytes. + */ +function sendKeepAliveRequest(socket: net.Socket, port: number): Promise { + return new Promise((resolve) => { + let received = ""; + let settleTimer: NodeJS.Timeout; + const finish = () => { + socket.off("data", onData); + clearTimeout(settleTimer); + resolve(received); + }; + // A short settle window once the full chunked response has arrived (fast + // path); a generous cap in case nothing ever comes back — the torn-down + // connection case this test proves, and a safety margin against first-run + // JIT/module-load jitter under the test runner. + const onData = (chunk: Buffer) => { + received += chunk.toString("utf8"); + if (received.endsWith("0\r\n\r\n")) { + clearTimeout(settleTimer); + settleTimer = setTimeout(finish, 50); + } + }; + socket.on("data", onData); + socket.write(`GET / HTTP/1.1\r\nHost: 127.0.0.1:${port}\r\nConnection: keep-alive\r\n\r\n`); + settleTimer = setTimeout(finish, 3_000); + }); +} + +function startEchoServer(configure: (server: http.Server) => void): Promise { + return new Promise((resolve) => { + const server = http.createServer((_req, res) => { + res.writeHead(200, { "content-type": "text/plain" }); + res.end("ok"); + }); + configure(server); + server.listen(0, "127.0.0.1", () => resolve(server)); + }); +} + +async function withServer( + configure: (server: http.Server) => void, + run: (port: number) => Promise +): Promise { + const server = await startEchoServer(configure); + try { + const address = server.address(); + if (typeof address !== "object" || address === null) { + throw new Error("expected server to bind a TCP address"); + } + await run(address.port); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } +} + +// Node's default keepAliveTimeout is 5_000ms, but the server only starts that +// timer once the response has fully flushed and there is a small amount of +// internal scheduling overhead before the socket is actually torn down — +// empirically ~5.8-6s end-to-end on loopback. 6.5s reliably clears that +// window without relying on a hair-trigger race. +const IDLE_GAP_MS = 6_500; + +describe("#7003 keep-alive socket reuse across an idle gap", () => { + it( + "current Node defaults (keepAliveTimeout=5000ms): a pooled socket reused after 6.5s idle gets 0 bytes back", + { timeout: 30_000 }, + async () => { + await withServer( + () => { + /* leave Node's http.Server defaults untouched (keepAliveTimeout=5000ms) */ + }, + async (port) => { + const socket = net.connect({ port, host: "127.0.0.1", allowHalfOpen: true }); + await new Promise((resolve, reject) => { + socket.once("connect", () => resolve()); + socket.once("error", reject); + }); + + const first = await sendKeepAliveRequest(socket, port); + assert.match(first, /200/, "first request on a fresh socket must succeed"); + + await new Promise((resolve) => setTimeout(resolve, IDLE_GAP_MS)); + + const second = await sendKeepAliveRequest(socket, port); + assert.equal( + second, + "", + "reusing the idle-torn-down socket must get exactly 0 bytes back (the reported bug)" + ); + socket.destroy(); + } + ); + } + ); + + it( + "fixed config (getMainServerTimeoutConfig): the same reused connection stays alive past 6.5s idle", + { timeout: 30_000 }, + async () => { + const fixedTimeouts = getMainServerTimeoutConfig({}); + await withServer( + (server) => { + server.keepAliveTimeout = fixedTimeouts.keepAliveTimeoutMs; + server.headersTimeout = fixedTimeouts.headersTimeoutMs; + }, + async (port) => { + const socket = net.connect({ port, host: "127.0.0.1", allowHalfOpen: true }); + await new Promise((resolve, reject) => { + socket.once("connect", () => resolve()); + socket.once("error", reject); + }); + + const first = await sendKeepAliveRequest(socket, port); + assert.match(first, /200/, "first request on a fresh socket must succeed"); + + await new Promise((resolve) => setTimeout(resolve, IDLE_GAP_MS)); + + const second = await sendKeepAliveRequest(socket, port); + assert.match( + second, + /200/, + "the reused connection must still get a valid response after the fix" + ); + socket.destroy(); + } + ); + } + ); +}); diff --git a/tests/unit/standalone-server-ws-keepalive-timeout-7003.test.ts b/tests/unit/standalone-server-ws-keepalive-timeout-7003.test.ts new file mode 100644 index 0000000000..076e84b1e4 --- /dev/null +++ b/tests/unit/standalone-server-ws-keepalive-timeout-7003.test.ts @@ -0,0 +1,73 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +// #7003 — the RED/GREEN spec in main-server-keepalive-timeout-7003.test.ts proves +// getMainServerTimeoutConfig() raises keepAliveTimeout/headersTimeout above Node's +// unconfigured 5_000ms default, and that the original fix wired it into +// scripts/dev/run-next.mjs. But run-next.mjs only runs `npm run dev`/`npm start` +// from a source checkout. The server real end users run — `omniroute serve` +// (npm-installed CLI), Docker, and Electron — spawns the standalone Next build's +// server.js via scripts/dev/run-standalone.mjs, which prefers server-ws.mjs +// (built from scripts/dev/standalone-server-ws.mjs, copied byte-for-byte into +// dist/server-ws.mjs by scripts/build/assembleStandalone.mjs) over the bare +// server.js specifically because it wraps `http.createServer` with production +// behavior the bare server lacks (peer-IP stamping, method/HEAD guards, WS +// proxying, TLS). Before this fix, that wrapper left Node's http.Server +// keepAliveTimeout/headersTimeout at their unconfigured defaults, so the +// JetBrains AI Assistant reconnect bug reproduced by main-server-keepalive-timeout +// -7003.test.ts still hit the production entry point every real user runs. +// +// standalone-server-ws.mjs has top-level side effects (monkeypatches +// http.createServer, generates a random UUID, and unconditionally +// `await import("./server.js")` — a file that only exists in the assembled +// standalone output, not in the source tree) so it cannot be imported +// in-process. Guard the fix by inspecting the source, mirroring the pattern +// used for run-next.mjs in run-next-node-env.test.ts. +const here = path.dirname(fileURLToPath(import.meta.url)); +const source = fs.readFileSync( + path.resolve(here, "../../scripts/dev/standalone-server-ws.mjs"), + "utf8" +); + +test("standalone-server-ws.mjs imports getMainServerTimeoutConfig", () => { + assert.match( + source, + /import\s*\{\s*getMainServerTimeoutConfig\s*\}\s*from\s*["'][^"']*runtimeTimeouts(?:\.ts)?["']/, + "expected the production server wrapper to import getMainServerTimeoutConfig, " + + "the same helper run-next.mjs uses" + ); +}); + +test("standalone-server-ws.mjs applies keepAliveTimeout/headersTimeout to the wrapped server", () => { + assert.match( + source, + /server\.keepAliveTimeout\s*=\s*\w*[Tt]imeouts?\.keepAliveTimeoutMs/, + "expected the wrapped server object to have keepAliveTimeout set from getMainServerTimeoutConfig()" + ); + assert.match( + source, + /server\.headersTimeout\s*=\s*\w*[Tt]imeouts?\.headersTimeoutMs/, + "expected the wrapped server object to have headersTimeout set from getMainServerTimeoutConfig()" + ); +}); + +test("keepAliveTimeout/headersTimeout are applied inside createServerWithResponsesWs, before the server is returned", () => { + const factoryIdx = source.search(/function createServerWithResponsesWs/); + const keepAliveIdx = source.search(/server\.keepAliveTimeout\s*=/); + const returnIdx = source.search(/return server;/); + + assert.ok(factoryIdx !== -1, "expected createServerWithResponsesWs to exist"); + assert.ok(keepAliveIdx !== -1, "expected a server.keepAliveTimeout assignment to exist"); + assert.ok(returnIdx !== -1, "expected the wrapped server to be returned"); + assert.ok( + keepAliveIdx > factoryIdx, + "timeout wiring must happen inside createServerWithResponsesWs" + ); + assert.ok( + keepAliveIdx < returnIdx, + "timeout wiring must happen before the server object is returned to the caller" + ); +});