From 332d38bb215db06f80e7824b4248ba9325e9e034 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:07:31 +0000 Subject: [PATCH 1/2] fix(server): Keep the idle limit of a tunnel for each of its requests A `CONNECT` tunnel gets a 10-minute idle limit on its raw socket. In a tunnel without TLS, the tunnel server of Node uses that socket. After a response, Node sets the limit to its keep-alive time (5 s and 1 s more), and at the next request it sets it to the `timeout` of the server, which was 0. So the limit went 600000 -> 6000 -> 0, and a second request that hung, such as one whose body or upstream never came, had no idle limit. Set `timeout` of the tunnel server to the idle limit, so Node sets the socket back to 10 minutes at each new request. A TLS tunnel keeps the limit of its raw socket as before, and its TLS socket gets the same limit. The new test reads the limit that the service sets on its end of a plain tunnel at the second request. It gets 0 on main and 600000 now. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com> --- README.md | 6 ++--- packages/service/src/server.ts | 5 ++++ packages/service/tests/service.test.ts | 32 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ad4afc3..3ec502f 100644 --- a/README.md +++ b/README.md @@ -763,9 +763,9 @@ To see how well the cache works: - A `CONNECT` tunnel that sends no byte for 10 s after the `200` closes. A tunnel that sends and gets no byte for 10 minutes while a request waits for its response closes. Between two requests, a tunnel closes after - about 5 s without a new request (the `keepAliveTimeout` of Node), and - the client opens a new one. A gap: in a tunnel without TLS, Node removes - the 10-minute limit after the first request. + 6 s without a new request (the `keepAliveTimeout` of Node, 5 s, and 1 s + that Node adds), and the client opens a new one. The 10-minute limit + holds for each request of a tunnel, with or without TLS. - The credentials that a run learns and the values that the sessions of a run without the token remember have limits. See [Config](#config) and [Changing values](#changing-values). diff --git a/packages/service/src/server.ts b/packages/service/src/server.ts index 0dd2210..3573af6 100644 --- a/packages/service/src/server.ts +++ b/packages/service/src/server.ts @@ -520,6 +520,11 @@ export async function listenProxy( } serveProxied(tunnel, incoming, outgoing, tunnel.origin); }); + // Between two requests, Node sets the time limit of a socket to its + // `keepAliveTimeout`, and at the next request back to `timeout`. With + // the default `timeout` of 0, a later request in a tunnel without TLS + // had no idle limit. A TLS tunnel also keeps the limit of its raw socket. + tunnelServer.timeout = TUNNEL_IDLE_MS; const server = http.createServer(HTTP_OPTIONS, (incoming, outgoing) => { // A request to the proxy itself has a path. A proxied plain HTTP // request has an absolute URL. diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index c86f8dc..d9e6728 100644 --- a/packages/service/tests/service.test.ts +++ b/packages/service/tests/service.test.ts @@ -1450,6 +1450,38 @@ describe("limits of the service", () => { ]), ).resolves.toBe("closed"); }); + it("keeps the idle limit of a tunnel without TLS after its first request", async () => { + // Between two requests, Node sets the time limit of a socket to its + // keep-alive time, and at the next request to the `timeout` of the + // server. Before, that was 0, so a later request had no idle limit. + const run = await startRun(undefined, "replay"); + const tunnel = await openTunnel(service.url, run.url); + // The service has its end of the tunnel in a socket of the same class. + const socketClass = Object.getPrototypeOf(tunnel) as typeof tunnel; + const setTimeoutSpy = vi.spyOn(socketClass, "setTimeout"); + onTestFinished(() => setTimeoutSpy.mockRestore()); + const port = Number(new URL(service.url).port); + /** The time limits that the service set on its end of the tunnel. */ + const limits = () => + setTimeoutSpy.mock.calls.flatMap(([ms], n) => { + const socket = setTimeoutSpy.mock.contexts[n] as typeof tunnel; + const ours = + socket.localPort === port && socket.remotePort === tunnel.localPort; + return ours ? [ms] : []; + }); + const post = `POST /v1/messages HTTP/1.1\r\nhost: ${new URL(origin).host}\r\ncontent-length: 2\r\n\r\n`; + tunnel.write(`${post}{}`); + const [first] = (await once(tunnel, "data")) as [Buffer]; + expect(first.toString()).toMatch(/^HTTP\/1.1 412 /); + // Node waits 5 s and 1 s more for the next request. + await until(() => limits().at(-1) === 6000); + const before = limits().length; + // A second request whose body does not come. + tunnel.write(`${post}{`); + await until(() => limits().length > before); + expect(limits().at(-1)).toBe(10 * 60 * 1000); + }); + it("times out slow headers, also with proxyProtocol and in a tunnel", async () => { // With `proxyProtocol`, the HTTP servers get their connections from a // TCP server, and Node did not check their time limits. From 4c69f814b067c1d79d54d6ce02fc15516b1dca07 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:06:22 +0000 Subject: [PATCH 2/2] fix(server): Keep the idle limit on proxied and control connections Node's requestTimeout stops when a request is read, and the main server had no timeout. A request that waited on a hung upstream, or a client that did not read its response, held the connection with no limit. Set the same 10-minute idle limit as the tunnel server. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com> --- README.md | 12 ++-- packages/service/src/server.ts | 17 ++++-- packages/service/tests/service.test.ts | 83 ++++++++++++++++---------- 3 files changed, 69 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 3ec502f..629d338 100644 --- a/README.md +++ b/README.md @@ -760,12 +760,12 @@ To see how well the cache works: - Request headers that do not come in 10 s get HTTP 408, and a request whose body does not come in 300 s too. This is also true in a tunnel and with `proxyProtocol`. -- A `CONNECT` tunnel that sends no byte for 10 s after the `200` closes. A - tunnel that sends and gets no byte for 10 minutes while a request waits - for its response closes. Between two requests, a tunnel closes after - 6 s without a new request (the `keepAliveTimeout` of Node, 5 s, and 1 s - that Node adds), and the client opens a new one. The 10-minute limit - holds for each request of a tunnel, with or without TLS. +- A `CONNECT` tunnel that sends no byte for 10 s after the `200` closes. +- A connection or tunnel that sends and gets no byte for 10 minutes + closes, also while a request waits for its response. This is true for + each request, with or without TLS. Between two requests, a connection + closes after 6 s without a new request (the `keepAliveTimeout` of Node, + 5 s, and 1 s that Node adds), and the client opens a new one. - The credentials that a run learns and the values that the sessions of a run without the token remember have limits. See [Config](#config) and [Changing values](#changing-values). diff --git a/packages/service/src/server.ts b/packages/service/src/server.ts index 3573af6..4c489f4 100644 --- a/packages/service/src/server.ts +++ b/packages/service/src/server.ts @@ -70,11 +70,12 @@ const MAX_BODY_BYTES = 64 * 1024 * 1024; /** A tunnel that sends no byte this long after its `200` closes. */ const TUNNEL_FIRST_BYTE_MS = 10_000; /** - * A tunnel that sends and gets no byte this long closes. A live model - * call can take minutes before its first byte, so this is long. Between - * two requests, Node closes the tunnel sooner (`keepAliveTimeout`, 5 s). + * A connection or tunnel that sends and gets no byte this long closes. A + * live model call can take minutes before its first byte, so this is long. + * Between two requests, Node closes the connection sooner + * (`keepAliveTimeout`, 5 s, and 1 s that Node adds). */ -const TUNNEL_IDLE_MS = 10 * 60 * 1000; +const IDLE_MS = 10 * 60 * 1000; /** * The HTTP servers of the proxy. Headers that do not come in 10 s get * HTTP 408 from Node. A whole request, with its body, has Node's default @@ -524,7 +525,7 @@ export async function listenProxy( // `keepAliveTimeout`, and at the next request back to `timeout`. With // the default `timeout` of 0, a later request in a tunnel without TLS // had no idle limit. A TLS tunnel also keeps the limit of its raw socket. - tunnelServer.timeout = TUNNEL_IDLE_MS; + tunnelServer.timeout = IDLE_MS; const server = http.createServer(HTTP_OPTIONS, (incoming, outgoing) => { // A request to the proxy itself has a path. A proxied plain HTTP // request has an absolute URL. @@ -555,6 +556,10 @@ export async function listenProxy( const listener = options.proxyProtocol ? net.createServer((socket) => acceptProxied(socket, server)) : server; + // Node's `requestTimeout` stops when a request is read. Without this, a + // request that waits on a hung upstream, or a client that does not read + // its response, holds the connection with no limit. + server.timeout = IDLE_MS; listener.on("connection", (socket: Socket) => { sockets.add(socket); socket.on("close", () => sockets.delete(socket)); @@ -602,7 +607,7 @@ export async function listenProxy( } socket.write("HTTP/1.1 200 Connection Established\r\n\r\n"); // A tunnel that a client leaves open must not stay open forever. - socket.setTimeout(TUNNEL_IDLE_MS, () => socket.destroy()); + socket.setTimeout(IDLE_MS, () => socket.destroy()); const open = async (first: Buffer) => { socket.unshift(first); // A TLS handshake starts with byte 0x16. Anything else is plain HTTP. diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index d9e6728..a3fde9b 100644 --- a/packages/service/tests/service.test.ts +++ b/packages/service/tests/service.test.ts @@ -1450,37 +1450,58 @@ describe("limits of the service", () => { ]), ).resolves.toBe("closed"); }); - it("keeps the idle limit of a tunnel without TLS after its first request", async () => { - // Between two requests, Node sets the time limit of a socket to its - // keep-alive time, and at the next request to the `timeout` of the - // server. Before, that was 0, so a later request had no idle limit. - const run = await startRun(undefined, "replay"); - const tunnel = await openTunnel(service.url, run.url); - // The service has its end of the tunnel in a socket of the same class. - const socketClass = Object.getPrototypeOf(tunnel) as typeof tunnel; - const setTimeoutSpy = vi.spyOn(socketClass, "setTimeout"); - onTestFinished(() => setTimeoutSpy.mockRestore()); - const port = Number(new URL(service.url).port); - /** The time limits that the service set on its end of the tunnel. */ - const limits = () => - setTimeoutSpy.mock.calls.flatMap(([ms], n) => { - const socket = setTimeoutSpy.mock.contexts[n] as typeof tunnel; - const ours = - socket.localPort === port && socket.remotePort === tunnel.localPort; - return ours ? [ms] : []; - }); - const post = `POST /v1/messages HTTP/1.1\r\nhost: ${new URL(origin).host}\r\ncontent-length: 2\r\n\r\n`; - tunnel.write(`${post}{}`); - const [first] = (await once(tunnel, "data")) as [Buffer]; - expect(first.toString()).toMatch(/^HTTP\/1.1 412 /); - // Node waits 5 s and 1 s more for the next request. - await until(() => limits().at(-1) === 6000); - const before = limits().length; - // A second request whose body does not come. - tunnel.write(`${post}{`); - await until(() => limits().length > before); - expect(limits().at(-1)).toBe(10 * 60 * 1000); - }); + it.each(["a tunnel without TLS", "an absolute-form connection"])( + "keeps the idle limit of %s after its first request", + async (kind) => { + // Between two requests, Node sets the time limit of a socket to its + // keep-alive time, and at the next request to the `timeout` of the + // server. Before, that was 0, so a later request had no idle limit, + // also after Node read all of it (`requestTimeout` stops there). + const run = await startRun(undefined, "replay"); + const host = new URL(origin).host; + let socket: ReturnType; + let post: string; + if (kind === "a tunnel without TLS") { + socket = await openTunnel(service.url, run.url); + post = `POST /v1/messages HTTP/1.1\r\nhost: ${host}\r\ncontent-length: 2\r\n\r\n`; + } else { + const { username, password } = new URL(run.url); + const credentials = Buffer.from( + `${decodeURIComponent(username)}:${decodeURIComponent(password)}`, + ).toString("base64"); + socket = connect(Number(new URL(service.url).port), "127.0.0.1"); + onTestFinished(() => { + socket.destroy(); + }); + await once(socket, "connect"); + post = `POST ${origin}/v1/messages HTTP/1.1\r\nhost: ${host}\r\nproxy-authorization: Basic ${credentials}\r\ncontent-length: 2\r\n\r\n`; + } + // The service has its end of the connection in a socket of the same + // class. + const socketClass = Object.getPrototypeOf(socket) as typeof socket; + const setTimeoutSpy = vi.spyOn(socketClass, "setTimeout"); + onTestFinished(() => setTimeoutSpy.mockRestore()); + const port = Number(new URL(service.url).port); + /** The time limits that the service set on its end of the connection. */ + const limits = () => + setTimeoutSpy.mock.calls.flatMap(([ms], n) => { + const peer = setTimeoutSpy.mock.contexts[n] as typeof socket; + const ours = + peer.localPort === port && peer.remotePort === socket.localPort; + return ours ? [ms] : []; + }); + socket.write(`${post}{}`); + const [first] = (await once(socket, "data")) as [Buffer]; + expect(first.toString()).toMatch(/^HTTP\/1.1 412 /); + // Node waits 5 s and 1 s more for the next request. + await until(() => limits().at(-1) === 6000); + const before = limits().length; + // A second request whose body does not come. + socket.write(`${post}{`); + await until(() => limits().length > before); + expect(limits().at(-1)).toBe(10 * 60 * 1000); + }, + ); it("times out slow headers, also with proxyProtocol and in a tunnel", async () => { // With `proxyProtocol`, the HTTP servers get their connections from a