mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
* fix: raise main server keepAliveTimeout/headersTimeout above Node's 5s default (#7003) JetBrains AI Assistant's pooled java.net.http.HttpClient reuses a keep-alive connection past Node's unconfigured 5_000ms keepAliveTimeout, hitting a socket the server already tore down and getting 0 response bytes back ("HTTP/1.1 header parser received no bytes"). Wire a new getMainServerTimeoutConfig() (mirroring apiBridgeServer's pattern) into run-next.mjs so the main dashboard/API server raises keepAliveTimeout to 65s and headersTimeout to 66s by default, both env-overridable. * fix: wire main-server keepAlive timeouts into standalone/production server path (#7003) getMainServerTimeoutConfig() was only wired into scripts/dev/run-next.mjs, the dev-only entry point for `npm run dev`/`npm start`. The server real end users run — `omniroute serve` (npm-installed CLI), Docker, and Electron — spawns the standalone Next build's server.js via run-standalone.mjs, which prefers server-ws.mjs (built verbatim from scripts/dev/standalone-server-ws.mjs by assembleStandalone.mjs) over the bare server.js precisely because it wraps http.createServer with production behavior the bare server lacks. That wrapper never configured keepAliveTimeout/headersTimeout, so the JetBrains AI Assistant reconnect bug this issue reports still hit the production entry point after the first pass of this fix. Wire the same helper into the wrapped server object there too.
This commit is contained in:
committed by
GitHub
parent
c859931314
commit
af0c72fba5
1
changelog.d/fixes/7003-jetbrains-ai-loopback-connect.md
Normal file
1
changelog.d/fixes/7003-jetbrains-ai-loopback-connect.md
Normal file
@@ -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)
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
202
tests/unit/main-server-keepalive-timeout-7003.test.ts
Normal file
202
tests/unit/main-server-keepalive-timeout-7003.test.ts
Normal file
@@ -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<string> {
|
||||
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<http.Server> {
|
||||
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<void>
|
||||
): Promise<void> {
|
||||
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<void>((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<void>((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<void>((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();
|
||||
}
|
||||
);
|
||||
}
|
||||
);
|
||||
});
|
||||
@@ -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"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user