Skip to content

node:http: enforce server headersTimeout, requestTimeout, setTimeout() and keepAliveTimeout - #32942

Closed
robobun wants to merge 2 commits into
mainfrom
farm/e716cf1a/node-http-server-timeouts
Closed

robobun wants to merge 2 commits into
mainfrom
farm/e716cf1a/node-http-server-timeouts

Conversation

@robobun

@robobun robobun commented Jun 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

All four node:http server timeout knobs are accepted, stored, and silently unenforced: headersTimeout, requestTimeout, server.setTimeout() (and the 'timeout' event), and keepAliveTimeout. A slow or idle client holds its connection open indefinitely regardless of what the server configured, which is the exposure these knobs exist to prevent (slowloris, dead proxies, fd exhaustion). maxRequestsPerSocket, closeAllConnections() and closeIdleConnections() already worked.

Reproduction: each knob set to 400 ms, probed for 1.8 s
import http from 'node:http';
import net from 'node:net';
function probe(name, setup, firstWrite) {
  return new Promise(res => {
    const srv = http.createServer({ connectionsCheckingInterval: 100 }, (q, r) => { q.resume(); q.on('end', () => r.end('ok')); });
    let closedAt = null; const t0 = Date.now();
    setup(srv);
    srv.on('timeout', s => { s.destroy(); });
    srv.on('clientError', (e, s) => s.destroy());
    srv.listen(0, '127.0.0.1', () => {
      const s = net.connect(srv.address().port); s.setNoDelay(true); s.on('error', () => {}); s.resume();
      s.on('connect', () => { if (firstWrite) s.write(firstWrite); });
      s.on('close', () => { closedAt = Date.now() - t0; });
      setTimeout(() => { try{s.destroy();}catch(e){} srv.close();
        res({ name, serverClosedConnAfterMs: closedAt, enforced: closedAt !== null && closedAt < 1500 }); }, 1800);
    });
  });
}
const out = [];
out.push(await probe('headersTimeout=400', s => { s.headersTimeout = 400; s.requestTimeout = 600; }, 'GET / HTTP/1.1\r\nHost: a\r\n'));
out.push(await probe('server.setTimeout(400)', s => s.setTimeout(400), ''));
out.push(await probe('requestTimeout=400', s => { s.requestTimeout = 400; s.headersTimeout = 300; }, 'POST / HTTP/1.1\r\nHost:a\r\nContent-Length: 50\r\n\r\nab'));
out.push(await new Promise(res => {
  const srv = http.createServer((q, r) => { q.resume(); r.end('ok'); });
  srv.keepAliveTimeout = 400; let closedAt = null; let t0 = 0;
  srv.listen(0, '127.0.0.1', () => {
    const s = net.connect(srv.address().port); s.setNoDelay(true); s.on('error', () => {}); s.resume();
    s.on('connect', () => s.write('GET / HTTP/1.1\r\nHost:a\r\n\r\n'));
    s.once('data', () => { t0 = Date.now(); s.on('close', () => closedAt = Date.now() - t0); });
    setTimeout(() => { try{s.destroy();}catch(e){} srv.close();
      res({ name: 'keepAliveTimeout=400', serverClosedConnAfterMs: closedAt, enforced: closedAt !== null && closedAt < 1500 }); }, 1800);
  });
}));
console.log(JSON.stringify(out));

On bun 1.4.0 and main, every probe reports {"serverClosedConnAfterMs":null,"enforced":false}. On Node v26 they close at 529 / 404 / 504 / 1394 ms. With this change: 1039 / 1471 / 475 / 401 ms (the extra second on the first two is the listen socket's 1 s TCP_DEFER_ACCEPT holding back the accept of a client that sent an incomplete head, which is unrelated to the knobs).

Cause

Two halves, both needed:

  1. The JS layer only learned a connection existed once its first request was fully parsed (onNodeHTTPRequest). A socket that never sends a complete request head was invisible to JS, so nothing could time it out. There was no accept hook.
  2. The parts JS did have were stubs. setupConnectionsTracking ran an interval whose callback was a literal no-op, NodeHTTPServerSocket.prototype.setTimeout() returned this without arming anything, and Server.prototype.setTimeout forwarded to the uWS per-app idle timeout, which has 4 second granularity (us_socket_timeout rounds seconds with (s + 3) >> 2) and closes the socket natively without reaching the JS 'timeout' event.

Fix

uWS / native: add an onSocketOpen callback to HttpContext, fired on accept (after the TLS handshake for https), next to the existing onSocketClosed / onSocketData Node compat callbacks. Plumbed through the C API (uws_app_set_on_socket_open), uws_sys, and the server crate as Server__setOnNodeSocketOpen, and exposed to node:_http_server as a new optional sixth argument of the existing setServerCustomOptions internal binding. It is only registered when that argument is a function, so Bun.serve is unaffected.

node:_http_server:

  • onServerSocketOpen wraps every accepted socket in a NodeHTTPServerSocket and emits 'connection' at accept like Node.js (previously it fired on the first parsed request, and never for CONNECT).
  • The connectionsCheckingInterval sweep is now Node's checkConnections: a connection past headersTimeout with an incomplete head, or past requestTimeout with an incomplete request, raises ERR_HTTP_REQUEST_TIMEOUT through 'clientError'. With no listener the server answers 408 Request Timeout and destroys the socket, matching Node's default handler. The request clock stops when the incoming request is complete (at the head for body-less requests, at the body's 'end' otherwise), not when the response finishes, so requestTimeout never kills a slow handler or a long-lived response stream.
  • socket.setTimeout() / socket._unrefTimer() are a real per-socket millisecond inactivity timer. server.setTimeout(ms) stores server.timeout and each accepted socket arms it; keepAliveTimeout re-arms it when a response finishes and the connection goes idle. The timer is unref'd and cleared on close.
  • ERR_HTTP_REQUEST_TIMEOUT is appended to ErrorCode.ts, so existing error code indices are unchanged.

One knowing deviation from Node: the headersTimeout / requestTimeout clock is anchored at accept, where Node anchors it at the first parsed byte of a message (so Node never times out a connected-but-fully-silent socket). Accept is the event we can observe, is never later than the first byte, and is the stricter of the two for shedding idle connections.

Every accepted connection now allocates its Duplex wrapper at accept rather than lazily at the first parsed request. That is required for any of these timeouts to cover a socket that never completes a request, and it is what Node.js does.

Tests

test/js/node/http/node-http-server-timeouts.test.ts: one test per knob, plus the default 408 answer when there is no 'clientError' listener, plus a regression guard that requestTimeout does not fire while a slow handler is still streaming its response. Five of the six fail on the unmodified build (the guard intentionally passes both ways). Every probe awaits the server closing the connection rather than sleeping, so an unenforced knob fails by timing out.

test/js/node/http/ (221 tests) and the test/js/node/test/parallel/test-http-{timeout,keep-alive,server-}* suite (20 files) pass with no new failures.

Fixes #17287

…and keepAliveTimeout

The four node:http server timeout knobs were stored but never enforced:
headersTimeout, requestTimeout, server.setTimeout()/'timeout', and
keepAliveTimeout all accepted assignment and did nothing, so slow or idle
connections were never shed.

The JS layer only learned about a connection once its first request was
fully parsed, so none of these could be tracked for a socket that never
sends a complete request. Add an onSocketOpen hook to uWS's HttpContext
(fired on accept, after the TLS handshake for https), plumb it through
the C API, uws_sys and the server crate as Server__setOnNodeSocketOpen,
and register it from node:_http_server via setServerCustomOptions.

With every accepted socket visible to JS:
- the connectionsCheckingInterval sweep (previously a no-op) now closes
  connections past headersTimeout / requestTimeout with
  ERR_HTTP_REQUEST_TIMEOUT, answering 408 when no clientError listener
  handles it, like Node.js
- server.setTimeout()/keepAliveTimeout arm a per-socket millisecond
  inactivity timer (the uws native idle timeout has 4 second granularity
  and never reached the 'timeout' event)
- 'connection' is emitted on accept instead of on the first parsed
  request, matching Node.js
@coderabbitai

coderabbitai Bot commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@autofix-ci[bot], we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 18 minutes and 16 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 03151f00-4570-4c7e-bd83-a40d2677d643

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9331d and 1ab6ff9.

📒 Files selected for processing (12)
  • packages/bun-uws/src/App.h
  • packages/bun-uws/src/HttpContext.h
  • packages/bun-uws/src/HttpContextData.h
  • src/js/internal/http.ts
  • src/js/node/_http_server.ts
  • src/jsc/bindings/ErrorCode.ts
  • src/jsc/bindings/NodeHTTP.cpp
  • src/runtime/server/mod.rs
  • src/runtime/server/server_body.rs
  • src/uws_sys/App.rs
  • src/uws_sys/libuwsockets.cpp
  • test/js/node/http/node-http-server-timeouts.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jun 28, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 5:14 PM PT - Jun 27th, 2026

❌ @autofix-ci[bot], your commit 1ab6ff9 has 1 failures in Build #65851 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 32942

That installs a local version of the PR into your bun-32942 executable, so you can run:

bun-32942 --bun

@github-actions

Copy link
Copy Markdown
Contributor

Found 3 issues this PR may fix:

  1. Fastify timeout settings not respected when using Bun runtime with NestJS and Fastify adapter #17287 - Fastify timeout settings (keepAliveTimeout, connectionTimeout) not respected on node:http servers — this PR enforces those previously no-op knobs
  2. Node http.createServer can 100% CPU after day uptime in Bun 1.3.13 and 1.4.0 #32600 - http.createServer 100% CPU after long uptime due to stale/idle keep-alive connections never being reaped — fixed by keepAliveTimeout enforcement
  3. Express: req/res/socket 'close' events never fire on client abort for POST requests with a body #28976 - Express req/res/socket close events never fire on client abort for POST requests — this PR's onSocketOpen and request lifecycle tracking fixes the socket lifecycle path these events depend on

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #17287
Fixes #32600
Fixes #28976

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488 - Also implements headersTimeout/requestTimeout enforcement with connectionsCheckingInterval sweep, keepAliveTimeout idle connection closing, and server.setTimeout()/req.setTimeout()/res.setTimeout() real per-socket timers as part of a broader Node v26.3.0 compat PR

🤖 Generated with Claude Code

@robobun

robobun commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author

Checked the three suggestions before adding closing keywords, since those auto-close issues on merge:

@robobun

robobun commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #32488, which predates this PR and already implements everything here as part of a much larger node:http compat pass: headersTimeout / requestTimeout with the connectionsCheckingInterval sweep and the raw 408 reply, keepAliveTimeout, real per-socket timers for server.setTimeout / res.setTimeout / req.setTimeout including TLS, and 'connection' at accept time, validated against Node v26.3.0's own http suites. A competing subset of that is not useful.

One piece that may still be worth taking from this branch: test/js/node/http/node-http-server-timeouts.test.ts is a small standalone test of the four knobs against a raw socket, including a regression guard that requestTimeout stops when the incoming request is complete rather than when the response finishes, so a slow handler or long-lived response stream never gets a spurious 408.

Comment on lines +599 to +605
socket[kHeadersCompleted] = true;
if (socket[kRequestStart] === 0) socket[kRequestStart] = DateNow();
if (socket.timeout !== server.timeout) {
socket.setTimeout(server.timeout || 0);
} else {
socket._unrefTimer();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The per-socket inactivity timer is armed once at the request head (line 604) but never reset as request body chunks arrive, so server.setTimeout(ms) behaves as a wall-clock timer rather than an inactivity timer during uploads. With server.setTimeout(5000), a client uploading a 30-second body in continuous chunks will have 'timeout' fire after 5s and the socket destroyed mid-upload, even though bytes are actively flowing — Node.js resets the timer on every incoming read. Fix: call socket._unrefTimer() in onDataIncomingMessage (or wherever body chunks are pushed to the IncomingMessage).

Extended reasoning...

What the bug is

server.setTimeout(msecs) is documented (and behaves in Node.js) as an inactivity timer: it fires only when no bytes have moved on the socket for msecs milliseconds. In Node.js, every incoming chunk goes through onStreamRead, which calls stream[kUpdateTimer]() → socket._unrefTimer(), refreshing the timer. So an actively-uploading client never trips server.timeout no matter how long the total upload takes.

In this PR the timer is armed exactly once per request, at the head:

// _http_server.ts onNodeHTTPRequest, lines 599-605
socket[kHeadersCompleted] = true;
if (socket[kRequestStart] === 0) socket[kRequestStart] = DateNow();
if (socket.timeout !== server.timeout) {
  socket.setTimeout(server.timeout || 0);
} else {
  socket._unrefTimer();
}

After that, request body chunks flow through handle.ondata → onDataIncomingMessage (_http_incoming.ts:370-386), which only does this.push(chunk) and never touches socket._unrefTimer(). There are no _unrefTimer calls anywhere in _http_incoming.ts, and for normal (non-CONNECT/upgrade) requests socket[kEnableStreaming](false) clears the socket's own handle.ondata, so the body bypasses NodeHTTPServerSocket.#onData entirely.

Why existing code doesn't prevent it

_onTimeout()'s only suppression check is for outgoing backpressure:

if (response && response.writableLength > 0) {
  this._unrefTimer();
  return;
}

It does not check whether incoming reads are in progress, so an upload with an empty response buffer falls straight through to this.emit('timeout') → onNodeHTTPServerSocketTimeout, which destroys the socket if no listener swallows it (or hands it to the user's server.setTimeout callback, which conventionally also destroys it).

Bun's own net.ts Socket does call _unrefTimer() on every data chunk, matching Node — it's only the NodeHTTPServerSocket body path that misses it.

Step-by-step proof

  1. const server = http.createServer((req, res) => { req.pipe(res); }); server.setTimeout(5000, s => s.destroy()); server.listen(0);
  2. Client connects → onServerSocketOpen → socket.setTimeout(5000) arms kTimeoutHandle = setTimeout(socketOnTimeoutFire, 5000, socket).
  3. Client sends POST / HTTP/1.1\r\nHost: a\r\nContent-Length: 30000\r\n\r\n → onNodeHTTPRequest runs, calls socket._unrefTimer() once (re-arms 5000ms).
  4. Client sends 1KB of body every 1s for 30s. Each chunk reaches onDataIncomingMessage → this.push(chunk). Nothing calls socket._unrefTimer().
  5. At t≈5s after the head, socketOnTimeoutFire(socket) → socket._onTimeout() → response.writableLength is likely 0 (small chunks flush immediately) → this.emit('timeout') → onNodeHTTPServerSocketTimeout → server's 'timeout' listener destroys the socket.
  6. The upload is aborted at byte ~5000 of 30000 even though bytes were arriving every second. Node.js would never fire 'timeout' here because each chunk would have reset the 5s timer.

Impact

Any node:http server that calls server.setTimeout(N) (or sets server.timeout = N) and accepts uploads longer than N ms will spuriously destroy active uploads. This is opt-in (default server.timeout is 0), but it's the very feature this PR exists to implement, and server.setTimeout + file uploads is a common combination. It is not a regression (socket.setTimeout was a no-op before), but it makes the newly-implemented knob incorrect for the upload case.

Fix

Call this.socket?._unrefTimer() inside onDataIncomingMessage whenever a chunk arrives (mirroring Node's onStreamRead → kUpdateTimer), e.g.:

function onDataIncomingMessage(chunk, isLast, aborted) {
  if (aborted === NodeHTTPResponseAbortEvent.abort) { this.destroy(); return; }
  this.socket?._unrefTimer();
  if (chunk && !this._dumped) this.push(chunk);
  if (isLast) emitEOFIncomingMessage(this);
}

(or equivalently in the NodeHTTPResponse data path). Outgoing writes should similarly reset the timer, but that's a separate concern from this finding.

Comment on lines +1760 to 1771
if (!socket || socket.destroyed) return;
// Keep-alive idle between requests: the headers/request timeouts only
// apply while a request is in flight; the keep-alive inactivity timer
// takes over until the next request head arrives (Node.js's resOnFinish).
socket[kRequestStart] = 0;
socket[kHeadersCompleted] = false;
const keepAliveTimeout = server.keepAliveTimeout;
if (keepAliveTimeout) {
socket.setTimeout(keepAliveTimeout);
} else {
socket._unrefTimer();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 When two HTTP/1.1 requests are pipelined in one packet and request #1's handler calls res.end() synchronously, request #1's deferred 'finish' → resOnFinish runs after onNodeHTTPRequest has set up request #2's timeout state on the same socket, and unconditionally clobbers it: kRequestStart is zeroed (so request #2 escapes requestTimeout) and socket.setTimeout(keepAliveTimeout) overwrites server.timeout (so a slow async handler on request #2 gets the default 5s keep-alive timer and onNodeHTTPServerSocketTimeout may destroy the socket mid-request). Node.js's resOnFinish guards this via state.outgoing.length === 0; the equivalent here is if (socket._httpMessage) return; before the keep-alive bookkeeping (since res1.end() already detachSocket'd, a non-null _httpMessage means res2 is attached).

Extended reasoning...

What the bug is

resOnFinish (src/js/node/_http_server.ts:1755-1771) unconditionally resets per-socket timeout state to "keep-alive idle":

socket[kRequestStart] = 0;
socket[kHeadersCompleted] = false;
const keepAliveTimeout = server.keepAliveTimeout;
if (keepAliveTimeout) socket.setTimeout(keepAliveTimeout);
else socket._unrefTimer();

But res.end() emits 'finish' via process.nextTick, so for sync-pipelined requests this runs after the next request's onNodeHTTPRequest has already armed the same fields on the same socket, clobbering them.

Code path

uWS explicitly supports sync pipelining (HttpContext.h onData → consumePostPadded: "We still support sync pipelining"). With two requests in one TCP segment:

  1. onNodeHTTPRequest(req1) runs. The user handler calls res1.end() synchronously.
  2. res1.end() (ServerResponse.prototype.end): the native handle.end() clears HTTP_RESPONSE_PENDING; detachSocket(socket) clears socket._httpMessage = null; 'finish' is scheduled via process.nextTick.
  3. onNodeHTTPRequest(req1) returns with handle.finished true → no promise → the parser continues.
  4. onNodeHTTPRequest(req2) runs on the same socket: sets socket[kHeadersCompleted] = true, socket[kRequestStart] = DateNow(), calls socket.setTimeout(server.timeout || 0), and assignSocketInternal(http_res2, socket) sets socket._httpMessage = res2.
  5. onNodeHTTPRequest(req2) then calls drainMicrotasks() (line ~691), which drains the nextTick queue (Bun's Bun__drainMicrotasksFromJS → GlobalObject::drainMicrotasks() drains nextTickQueue).
  6. res1's 'finish' fires now → resOnFinish(server, socket, res1): res1[kMustCloseConnection] is false and socket is alive, so it writes socket[kRequestStart] = 0, socket[kHeadersCompleted] = false, and socket.setTimeout(server.keepAliveTimeout) (default 5000 ms).

Why nothing prevents it

Node.js's resOnFinish guards keep-alive arming with else if (state.outgoing.length === 0) — i.e. only when no further response is queued on the socket. This implementation has no such guard; it assumes 'finish' for response N always precedes the bookkeeping for request N+1, which sync pipelining violates.

Impact

For request #2 on the pipelined socket:

  • requestTimeout is bypassed. checkConnections does const start = socket[kRequestStart]; if (!start) continue; — with kRequestStart zeroed, request Fix calling #private() functions in classes #2 is never checked even if its body stalls forever.
  • keepAliveTimeout (default 5s) replaces server.timeout. If request Fix calling #private() functions in classes #2's handler is async and takes > 5s with no 'timeout' listener on req/res/server, socketOnTimeoutFire → _onTimeout → onNodeHTTPServerSocketTimeout calls this.destroy(), dropping a live in-flight request.

(Note: kHeadersCompleted=false does not spuriously trip headersTimeout, because kRequestStart=0 makes the sweep skip the socket entirely.)

The trigger — HTTP/1.1 pipelining with a synchronously-handled first request — is uncommon in browsers but reachable from raw TCP clients, proxies, and load-test tools, and the maxRequestsPerSocket code path's own comment acknowledges "already-pipelined requests, which still need to be dispatched".

Step-by-step proof

  1. Client sends in one packet: GET /a HTTP/1.1\r\nHost:x\r\n\r\nPOST /b HTTP/1.1\r\nHost:x\r\nContent-Length:50\r\n\r\n (no body bytes for /b yet).
  2. Handler for /a does res.end('ok') synchronously.
  3. Parser dispatches /b: onNodeHTTPRequest sets socket[kRequestStart]=now, socket[kHeadersCompleted]=true, socket.setTimeout(server.timeout||0), attaches res2, then drainMicrotasks().
  4. Inside drainMicrotasks(), res1 'finish' fires → resOnFinish zeroes kRequestStart, sets kHeadersCompleted=false, calls socket.setTimeout(5000).
  5. Client never sends /b's body. checkConnections sees start=0 → continue. requestTimeout never fires. Meanwhile, after 5s, socketOnTimeoutFire → onNodeHTTPServerSocketTimeout: req2.complete is false so it emits 'timeout' on req2; if nothing handles it, the socket is destroyed.

Fix

Skip the keep-alive idle bookkeeping when another response is already attached to the socket:

function resOnFinish(server, socket, res) {
  if (res[kMustCloseConnection]) { socket?.end(); return; }
  if (!socket || socket.destroyed) return;
  // The next pipelined request may already be attached; do not clobber its state.
  if (socket._httpMessage) return;
  socket[kRequestStart] = 0;
  socket[kHeadersCompleted] = false;
  ...
}

res1.end() calls detachSocket synchronously, so by the time res1's 'finish' runs, socket._httpMessage is non-null only if assignSocketInternal has attached res2 — exactly the case to guard.

Comment on lines 27 to 30
kRealListen,
tlsSymbol,
optionsSymbol,
kDeferredTimeouts,
kDeprecatedReplySymbol,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This PR removes the only consumers of kDeferredTimeouts (the import here and the deferred-replay loop in kRealListen/setTimeout), but the symbol is still declared at src/js/internal/http.ts:92 and exported at :547 with no remaining importers. The setServerIdleTimeout re-export in internal/http.ts (:14/:33/:590) is similarly orphaned now that _http_server.ts no longer imports it. Per the root CLAUDE.md ("Delete dead code in the same PR that makes it dead"), both should be removed in this PR.

Extended reasoning...

What

Before this PR, src/js/node/_http_server.ts imported kDeferredTimeouts from internal/http and used it in two places: Server.prototype[kRealListen] replayed any {msecs, callback} entries queued in this[kDeferredTimeouts] once the native server existed, and Server.prototype.setTimeout pushed onto that array when called before listen(). This PR rewrites setTimeout to simply store this.timeout = msecs (which is read per-socket at accept time), so the deferred queue is no longer needed and both the import and the replay loop are deleted.

That deletion was the only consumer of kDeferredTimeouts. After this PR, a repo-wide grep shows exactly two remaining hits, both in src/js/internal/http.ts:

src/js/internal/http.ts:92:const kDeferredTimeouts = Symbol("deferredTimeouts");
src/js/internal/http.ts:547:  kDeferredTimeouts,

No file imports it. The symbol is now an unused module-level const plus an unused named export.

The same applies to setServerIdleTimeout: this PR removes its only JS-side import (_http_server.ts previously called it from Server.prototype.setTimeout). After the PR, the only remaining JS references are the destructure from the native binding at internal/http.ts:14, the type annotation at :33, and the re-export at :590 — none of which has a downstream importer. (The native binding entry in NodeHTTP.cpp and the underlying Server__setIdleTimeout may have reasons to stay for Bun.serve, but the internal/http.ts re-export is dead.)

Step-by-step proof

  1. Pre-PR: rg kDeferredTimeouts → 4 hits: declaration + export in internal/http.ts, import + two uses in _http_server.ts.
  2. This diff removes the _http_server.ts import (line 30 of the old file) and both use sites (the if (this[kDeferredTimeouts]) replay block in kRealListen and the (this[kDeferredTimeouts] ??= []).push(...) branch in setTimeout).
  3. Post-PR: rg kDeferredTimeouts → only internal/http.ts:92 and :547. Nothing reads either.
  4. Likewise rg setServerIdleTimeout post-PR → internal/http.ts:14/33/590 and the native NodeHTTP.cpp binding registration; no JS importer of the internal/http export remains.

Why existing code doesn't prevent it

internal/http.ts is bundled into the binary as an internal module, so an unused export is not flagged by any tooling — it just ships dead. Public/exported items escape dead-code lints, which is exactly why the project guideline at CLAUDE.md:237 calls this out:

Delete dead code in the same PR that makes it dead (required scope — name the deletions in the description): … Public items escape dead-code lints — grep for callers manually.

Impact

None at runtime — an unused Symbol() const and an unused export entry have negligible cost. This is purely a code-cleanliness / project-convention issue, hence nit severity. The cost of not cleaning it up is that the next reader of internal/http.ts has to grep to discover the symbol is dead, and a future PR may accidentally resurrect the old (broken) deferral pattern by finding the symbol still exported.

Fix

In src/js/internal/http.ts:

  • Delete line 92 (const kDeferredTimeouts = Symbol("deferredTimeouts");).
  • Delete the kDeferredTimeouts, entry from the export list at line 547.
  • Delete setServerIdleTimeout from the destructure at line 14, the type at line 33, and the export list at line 590.

No test changes needed.

Comment on lines +968 to +973
function onServerSocketOpen(this: Server, ssl: boolean, handle: unknown) {
const self = this;
// The native side returns the existing wrapper for a handle it has seen
// before; a second Duplex would strand the first in kTrackedConnections.
const existingDuplex = (handle as any).duplex;
const socket = existingDuplex ?? new NodeHTTPServerSocket(self, handle, ssl);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The comment on Server.prototype.getConnections ("Connections are tracked from the first parsed request on each socket (the native server does not surface raw accepts to JS yet)") is now stale: this PR adds onServerSocketOpen, which fires on accept and constructs a NodeHTTPServerSocket whose constructor adds it to kTrackedConnections. Both halves of the comment are now false — please update or drop it.

Extended reasoning...

What the issue is

Server.prototype.getConnections (src/js/node/_http_server.ts:361-362) carries this comment:

// Connections are tracked from the first parsed request on each socket
// (the native server does not surface raw accepts to JS yet).

This PR directly invalidates both factual claims in that sentence, but leaves the comment untouched.

The code path that invalidates it

  1. This PR adds an onSocketOpen hook to uWS (HttpContext.h onOpen/onHandshake), plumbed through uws_app_set_on_socket_open → App::on_socket_open → Server__setOnNodeSocketOpen → the new sixth argument of setServerCustomOptions.
  2. Server.prototype[kRealListen] now passes onServerSocketOpen.bind(this) as that sixth argument.
  3. onServerSocketOpen runs on every accepted connection (after the TLS handshake for https), before any bytes are parsed, and does new NodeHTTPServerSocket(self, handle, ssl).
  4. The NodeHTTPServerSocket constructor calls server[kTrackedConnections]?.add(this).

So after this PR: connections are tracked from accept (not from the first parsed request), and the native server does surface raw accepts to JS. Both halves of the comment are now factually wrong.

Step-by-step proof

  • Client connects but sends nothing.
  • uWS onOpen fires → httpContextData->onSocketOpen(...) → Rust thunk → on_node_socket_open_callback → JS onServerSocketOpen.
  • onServerSocketOpen constructs NodeHTTPServerSocket, whose constructor runs server[kTrackedConnections]?.add(this).
  • server.getConnections(cb) now reports 1 for this socket even though no request head has been parsed — directly contradicting the comment's first clause.
  • The very existence of onServerSocketOpen being called from native on accept contradicts the parenthetical second clause.

Why nothing prevents it

The PR did not touch getConnections itself, so the comment was simply missed. There is no compile-time or test-time guard that ties comments to behavior.

Impact

Documentation accuracy only — no runtime effect. But it is a specific factual claim (not a vague description) that this PR's central change directly contradicts, and per repo convention comments invalidated by a PR should be updated in the same PR. A future reader debugging getConnections semantics would be actively misled about when tracking begins.

How to fix

Update the comment to reflect the new behavior, e.g.:

// Connections are tracked from accept (onServerSocketOpen adds the
// NodeHTTPServerSocket wrapper to kTrackedConnections before any bytes
// are parsed).

Or simply drop the comment if it no longer adds value.

Comment on lines +1392 to +1395
setTimeout(msecs, callback) {
this.timeout = msecs | 0;
if ($isCallable(callback)) this.once("timeout", callback);
this._unrefTimer();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: msecs | 0 coerces to int32, so socket.setTimeout(2_500_000_000) (or any msecs >= 2**31, including server.keepAliveTimeout/server.timeout flowing through here) wraps to a negative number and _unrefTimer's if (msecs > 0) check then leaves the timer disarmed. Node.js's net.Socket.setTimeout (and Bun's own net.ts) instead runs getTimerDuration(msecs, 'msecs'), which clamps to TIMEOUT_MAX = 2**31 - 1 with a TimeoutOverflowWarning. The trigger is a >24.8-day inactivity timeout so the practical impact is negligible, but for parity consider this.timeout = getTimerDuration(msecs, 'msecs') (or at least >>> 0).

Extended reasoning...

What the bug is. NodeHTTPServerSocket.prototype.setTimeout stores the timeout as this.timeout = msecs | 0. The bitwise-OR coerces its operand to a signed 32-bit integer, so any msecs value at or above 2**31 (2147483648) wraps to a negative number — e.g. 2_500_000_000 | 0 === -1794967296. _unrefTimer() then reads this.timeout, sees msecs > 0 is false, and takes the else branch that sets this[kTimeoutHandle] = null instead of arming a setTimeout. The net effect is that asking for a very large inactivity timeout silently disables the timer entirely.

The specific code path. There are three entry points that reach this coercion: (1) a user calling req.socket.setTimeout(N) or res.setTimeout(N) directly on the server socket; (2) server.timeout = N / server.setTimeout(N), which onServerSocketOpen and onNodeHTTPRequest forward to socket.setTimeout(server.timeout); and (3) server.keepAliveTimeout = N, which resOnFinish forwards to socket.setTimeout(keepAliveTimeout) after each response. storeHTTPOptions validates keepAliveTimeout and requestTimeout with validateInteger(..., 0), whose default upper bound is Number.MAX_SAFE_INTEGER, so values ≥ 2^31 pass validation and reach the | 0 truncation unchanged.

Why nothing prevents it. Node.js's net.Socket.prototype.setTimeout (and Bun's own implementation in src/js/node/net.ts:2273) calls getTimerDuration(msecs, 'msecs') from internal/timers, which (a) validates the value is a non-negative number and (b) clamps anything above TIMEOUT_MAX = 2**31 - 1 down to TIMEOUT_MAX while emitting a TimeoutOverflowWarning. The new NodeHTTPServerSocket is a separate Duplex subclass that does not go through net.Socket.prototype.setTimeout, and its | 0 shortcut does not replicate that clamping. Before this PR setTimeout on this class was a no-op, so there was no prior behavior to regress; this is a new divergence introduced alongside the new functionality.

Step-by-step proof.

  1. server.keepAliveTimeout = 3_000_000_000 (≈ 34.7 days). storeHTTPOptions calls validateInteger(3_000_000_000, 'keepAliveTimeout', 0) → passes (≤ MAX_SAFE_INTEGER).
  2. A request is served; on response 'finish', resOnFinish(server, socket, res) runs with keepAliveTimeout = 3_000_000_000, which is truthy, so it calls socket.setTimeout(3_000_000_000).
  3. setTimeout does this.timeout = 3_000_000_000 | 0 → -1294967296, then calls this._unrefTimer().
  4. _unrefTimer reads msecs = -1294967296, clears any existing handle, evaluates msecs > 0 → false, sets this[kTimeoutHandle] = null. No timer is armed.
  5. In Node.js the same call would store socket.timeout = 2147483647, emit a TimeoutOverflowWarning, and arm a ~24.8-day unref'd timer.

Impact. The behavioral divergence is real but the practical impact is minimal. 2^31 ms ≈ 24.8 days; no realistic socket inactivity, keep-alive, or server timeout approaches that (defaults are 0 / 5 s; generous deployments use minutes). The functional difference between "no timer" and "timer fires after 24.8 days" is essentially nil — the peer, OS keepalive, or process restart will close the connection long before either fires. Node itself treats this range as out-of-bounds and only warns. This is why the refuting verifier deems it non-actionable; we agree it does not block the PR, but it is still a correctness/parity gap worth noting.

Fix. Match net.Socket.setTimeout: import getTimerDuration from internal/timers and write this.timeout = msecs === 0 ? 0 : getTimerDuration(msecs, 'msecs') (Node treats 0 as "disable" without going through getTimerDuration, and server.timeout || 0 already passes 0 for the disabled case). A lighter alternative that avoids the warning machinery is this.timeout = Math.min(msecs >>> 0, 2 ** 31 - 1), but getTimerDuration is the established pattern in this codebase.

cirospaciari added a commit that referenced this pull request Jun 29, 2026
Raw-socket coverage of headersTimeout, requestTimeout, server.setTimeout,
and keepAliveTimeout, including that requestTimeout stops once the incoming
request completes and never fires while a slow handler is still streaming
its response. Five of the six cases fail on a Bun without the timeout
enforcement in this branch.

Contributed in #32942, which was closed
in favor of this PR.
cirospaciari added a commit that referenced this pull request Jun 29, 2026
Raw-socket coverage of headersTimeout, requestTimeout, server.setTimeout,
and keepAliveTimeout, including that requestTimeout stops once the incoming
request completes and never fires while a slow handler is still streaming
its response. Five of the six cases fail on a Bun without the timeout
enforcement in this branch.

Contributed in #32942, which was closed
in favor of this PR.
cirospaciari added a commit that referenced this pull request Jun 30, 2026
Raw-socket coverage of headersTimeout, requestTimeout, server.setTimeout,
and keepAliveTimeout, including that requestTimeout stops once the incoming
request completes and never fires while a slow handler is still streaming
its response. Five of the six cases fail on a Bun without the timeout
enforcement in this branch.

Contributed in #32942, which was closed
in favor of this PR.
cirospaciari added a commit that referenced this pull request Jun 30, 2026
Raw-socket coverage of headersTimeout, requestTimeout, server.setTimeout,
and keepAliveTimeout, including that requestTimeout stops once the incoming
request completes and never fires while a slow handler is still streaming
its response. Five of the six cases fail on a Bun without the timeout
enforcement in this branch.

Contributed in #32942, which was closed
in favor of this PR.
cirospaciari added a commit that referenced this pull request Jul 14, 2026
Raw-socket coverage of headersTimeout, requestTimeout, server.setTimeout,
and keepAliveTimeout, including that requestTimeout stops once the incoming
request completes and never fires while a slow handler is still streaming
its response. Five of the six cases fail on a Bun without the timeout
enforcement in this branch.

Contributed in #32942, which was closed
in favor of this PR.
Jarred-Sumner added a commit that referenced this pull request Jul 16, 2026
…stream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed (#32488)

### What this does

Raises `node:http`, `node:https`, and `node:http2` compatibility —
measured by running Node v26.3.0's own `test/parallel` +
`test/sequential` http/https/http2 suites unmodified under Bun — from
~77% to ~94%, and syncs the vendored copies of those suites to v26.3.0.

| module | before | after |
|---|---|---|
| node:http | 319/405 (78.8%) | 382/405 (94.3%) |
| node:https | 45/65 (69.2%) | 61/65 (93.8%) |
| node:http2 | 215/278 (77.3%) | 259/278 (93.2%) |
| **total** | **579/748 (77.4%)** | **702/748 (93.9%)** |

(Counts are upstream Node v26.3.0 test files run with the CI runner
config, exit 0 = pass. Every vendored http/https/http2 file passes;
**this PR adds no `test/expectations.txt` entries** — it only removes
them. The remaining gap is upstream files that are not vendored, listed
under *Not vendored* below.)

Fixes #28656 — `http2.createSecureServer({ allowHTTP1: true })` answered
an HTTPS/1.1 request with an empty reply (curl exit 52): the handshake
completed with no ALPN protocol negotiated and the connection closed
without a response. On this branch the same repro gets `HTTP/1.1 200` +
the body (bun 1.3.14 still reproduces the bug). Covered by the synced
upstream `test-http2-https-fallback*.js` suites and the `allowHTTP1`
cases in `node-http2.test.js`.

### Server (node:http / node:https)

* `headersTimeout` / `requestTimeout` enforcement with Node's
`connectionsCheckingInterval` sweep and raw `408` reply;
`keepAliveTimeout` closes idle keep-alive connections (`0` = no
timeout); `server.setTimeout` / `res.setTimeout` / `req.setTimeout` arm
real per-socket timers, on TLS too. The per-connection idle timer is
**refreshed in place** across the keep-alive cycle (no
`clearTimeout`+`setTimeout` per request).
* Connection lifecycle parity: `'connection'`/`'secureConnection'` at
accept/handshake time, `'close'` emitted whenever the TCP connection
closes, `closeIdleConnections`/`closeAllConnections` counting,
socket-error abort, and connection sockets are now real `net.Socket`
instances with Node's `socket.parser` surface (`incoming`, `free()`,
kOnTimeout slot, freed before `'upgrade'`/`'connect'`).
* Parse errors follow Node's `clientError` contract (error reaches the
client, `bytesParsed`/`rawPacket`, specific `HPE_*` codes incl.
`HPE_PAUSED_H2_UPGRADE` for the full 24-byte HTTP/2 preface,
premature-EOF, chunked+Content-Length, bare CR, oversized chunk
extensions with `413`/`431` replies).
* HTTP/1.1 pipelining (queued responses with `res.socket === null`,
ordered flush including buffered `1xx`, bounded by read backpressure,
never advances after a must-close response); `maxRequestsPerSocket`
(`503` + `'dropRequest'`); half-close handling. Pipelining lives in the
node-http template instantiation, so `Bun.serve` keeps its existing
async-pipeline-denied behaviour and fast paths unchanged. Queued
responses are bounded by Node's *two* gates: transport backpressure
**and** `state.outgoingData >= writableHighWaterMark` (the bytes
buffered across every queued response), so a client that pipelines
without reading cannot grow the queue without bound.
* Upgrade-with-body, CONNECT tunnels (raw data + `'end'` after handoff),
incoming/outgoing trailers (trailer-section size cap honours per-server
`maxHeaderSize`); the captured trailer section is given the parser's
standard post-padding before the 8-byte field-value scanner runs over
it, fixing an out-of-bounds read of up to 3 bytes past the section's
allocation (ASAN `heap-buffer-overflow` in `tryConsumeFieldValue`,
caught in review).
* Strict request parsing only for the node compat layer (llhttp-style
method validation and friends are gated on the node-http flags); leading
bare LF/CR before the request-line is tolerated like llhttp's `s_start`
state.
* `calculateLenientFlags` ported from Node so client `httpValidation`
reaches the response parser; per-server
`insecureHTTPParser`/`maxHeaderSize` (smaller-than-default values are
honoured for buffered partial headers too); `createServer({
highWaterMark })` plumbed to req/res; https
`pfx`/`minVersion`/`maxVersion`/`ciphers`/ALPN properties (with
`validateObject(options)` and the falsy-ALPN default like Node);
`req.socket.servername`/`authorized`/`authorizationError`; client/agent
fixes (Node-exact messages, domain binding via `domain.run` forwarding
args/return, parser reentrancy, abort/half-open lifecycle);
`NODE_TLS_REJECT_UNAUTHORIZED` is now `=== "0"` only.
* A `Connection: close` response whose body was still in the native send
buffer when `res.end()` returned was silently truncated:
`socket.end()`'s native path shut the transport down once the socket's
**own** stream buffer was empty, ignoring the response bytes uWS still
held under backpressure, so the FIN overtook them (~1.4 MB of an 8 MiB
body reached the client on macOS, where the small loopback send buffer
leaves most of the body in userspace; Node v26.3.0 delivers every byte).
When the in-flight response still has buffered data the close is now
handed to uWS — `HTTP_CONNECTION_CLOSE` makes its drain path shut the
socket down right after the last byte flushes, the same sequencing Node
gets from `destroySoon()`. Three regression tests in
`node-http-backpressure.test.ts` (client-requested close, server-set
`Connection: close`, one-shot `res.end(body)`), each failing without the
fix.

### node:http2

* nghttp2-style session errors and teardown for protocol violations (bad
preface, oversized frames, flow-control violations, HEADERS on closed
streams, unsolicited PING ACK, GOAWAY parity), flood/resource limits
(`maxSessionMemory` byte-exact, `maxSessionInvalidFrames`,
PING/SETTINGS-ACK floods).
* Flow control: stream windows are only replenished while the JS
readable is consuming (a paused reader backpressures the peer), and a
response that ends before the request body finished no longer stalls the
connection — fixes a real-world server bug covered by a new wire-level
conformance test.
* Settings parity (empty initial SETTINGS,
`customSettings`/`remoteCustomSettings`, `enableConnectProtocol`,
`strictSingleValueFields`; settings parsing is transactional and the
wire SETTINGS frame carries only the current call's keys while
`localSettings.customSettings` stays cumulative like Node); the
`'localSettings'` event reports the values the ACK actually
acknowledged.
* Writable parity (`_write` callback completes asynchronously so
`write()` returns `false` past the highWaterMark and `'drain'` fires;
over a native socket the deferral is `process.nextTick`, over a JS-side
socket / `duplexPair` it is `setImmediate` so JS-side delivery has run
first); `client.close()` waits for outstanding SETTINGS ACKs like Node's
`kMaybeDestroy` `hasPendingData()` check.
* Session idle timer under `kTimeout` with write-progress suppression (a
buffer that drained to zero between fires counts as progress),
`respondWithFile`/`respondWithFD` fd ownership and delivery,
AsyncLocalStorage context on client streams, request queueing before
connect, per-request `AbortSignal`, `performServerHandshake`, allowHTTP1
fallback headers, Node-exact error wording.
* `session.ping()` matches Node: throws `ERR_HTTP2_PING_LENGTH` for a
non-8-byte payload, `validateFunction(callback)`, returns `false`
(callback gets `ERR_HTTP2_PING_CANCEL`) at `maxOutstandingPings`, and
each call is an `AsyncResource('HTTP2PING')`.
* `client.request()` validates option types before checking session
state (Node order), and `stream.close(NaN)` rejects like Node's
default-parameter form. The server rejects an inbound `:protocol`
pseudo-header when its local `enableConnectProtocol` is `0` (RFC 8441
§4, like nghttp2 — the request never reaches `'stream'`).
* Server-initiated push streams close cleanly on END_STREAM (the
diagnostics `http2.server.stream.close` channel now publishes with
`closed=true && destroyed=false && rstCode=0` for the non-error path and
`destroyed=true` for the error path, matching node's
`onStreamClose`/`_destroy` split).
* Client stream events run in the async context captured when
`request()` was called even when that context is empty: the "never
captured" default is a sentinel now, so a stream requested outside any
`AsyncLocalStorage` scope observes an empty store in
`'response'`/`'data'`/`'end'` instead of the session's connect-time
store (caught in review; new regression test).


### node:net — fatal write errors and connect semantics

* macOS (kqueue) could truncate a hung-up socket's stream: `EV_EOF` is
reported on the same readable event as a connection's final data, and
the event handler honored it even when its read loop had stopped early
(short read, repeat budget, or the data callback pausing the socket),
ending and closing the socket with bytes still queued in the kernel. A
`tls.connect()` client whose peer did `end(big); destroySoon()` observed
`'end'` short of the payload — the darwin-aarch64-only CI failure an
earlier draft had quarantined. Now a hung-up event is drained to `recv()
== 0`/`EAGAIN` first (matching Linux, where the EOF is only ever
discovered by `recv() == 0`); a hard read error after that drain has
delivered data is reported as the end-of-stream the FIN already
announced (an RST behind a FIN — Node's reader also stops at the FIN)
while a pure RST still reports its error; and the EOF is deferred for a
socket the data callback paused mid-burst. Deterministic regression
test; `test-tls-client-destroy-soon.js` is un-quarantined and the two
recv-short TLS decode tests that failed on macOS now pass.

* A `send(2)` that fails after the peer reset the connection now reaches
JS. The errno travels usockets → `write_check_error` → `$write`
(negative errno; `-1` stays the legacy closed/shutdown sentinel) →
node:net fails the pending write callback exactly like Node's
`onWriteComplete` (`lib/internal/stream_base_commons.js#L81-L92`). A
fatal flush of natively-buffered data (write already acknowledged to JS)
is surfaced from the writable dispatch through the socket's `'error'`
handler with the errno-derived code. `ENOBUFS` stays transient and macOS
`EPROTOTYPE` is reported as `ECONNRESET`, matching libuv's
`uv__try_write`.
* This replaces an earlier branch of this PR that synthesized `read
ECONNRESET` from a duplicate EOF dispatch. Node treats a repeated
`UV_EOF` as a no-op, and that synthesized path destroyed healthy sockets
mid-read (the `test-net-write-slow` CI timeout). With the real mechanism
in place, `test-http2-max-invalid-frames` and `test-http2-reset-flood`
pass deterministically (verified run-for-run against the Node v26.3.0
binary), `test-net-write-slow` is clean under 8× parallel load, and
`double-connect.test.ts` no longer needs `test.failing`.
* New regression test: `test/js/node/net/node-net.test.ts` "a write
after the peer reset the connection fails with a write error".
* `socket.connect()` no longer force-`resume()`s the stream. Like Node's
`afterConnect` it only does `read(0)`, so data that arrives before a
`'data'` listener is attached stays buffered instead of being emitted to
nobody and lost, and a socket the user never reads follows Node's
lifecycle: no `'end'`/auto-destroy while data sits unread, while the
handle stops holding the event loop once the peer's FIN arrives (so the
process can still exit). The pre-existing `tls-syscall-fault` "FIN
before close_notify drained" test asserted the old auto-flow side effect
(await `'close'` on a never-read client) and was the deterministic
debian-x64-asan timeout; it now consumes the client like Node requires.
New regression test: "a connected socket is not flowing until the user
reads from it".

### Fixed from review

* **`Bun.serve` was silently made lenient.** The new "skip empty lines
before the request-line" (llhttp's `s_start`) went into the *shared*
templated parser with no `IsNodeHttp` gate, so `Bun.serve` answered
**200** to a request prefixed with `\r\n`, a bare `\n`, or a bare `\r` —
main returns **400**. Gated to node-http; `serve.test.ts` now asserts
the 400 for all three prefixes.
* **A missing `Host` header fired a bogus `clientError`** with
`HPE_INTERNAL`. Node never fires `'clientError'` for it —
`parserOnIncoming` answers the 400 itself — so Bun now replies
byte-for-byte with Node.
* **Unbounded pipelined response buffering** (see Server section):
Node's `outgoingData >= writableHighWaterMark` gate had no equivalent.
With 50k pipelined requests and the head response held open, Node
plateaus at 2148 dispatched; Bun climbed past 6385 with RSS following.
* **`H2FrameParser` leaked on a throwing handler.** `process.exit()`
never unwinds — the VM is destructed from inside the `exit()` call — so
a native frame still on the stack (`on_native_read`) never returns to
drop its `+1`, and neither the cork slot nor the queued auto-flush task
is released. The parser stranded at refcount 4 and `deinit()` never ran.
The self-keepalive refs now use a counted RAII guard, and `finalize`
releases the stranded ones while the VM is shutting down. (Reproduces on
main 10/10 under ASAN — pre-existing, not a regression.)
* **A terminated `Worker` leaked its loop state**: `EventLoop::deinit()`
never reclaimed `deferred_tasks`, finalizer-closed listen sockets were
only *queued* for close before the loop was freed, and the HPACK scratch
buffer's `thread_local` destructor is not guaranteed to run. (Also
pre-existing on main — a worker running a plain `net.createServer()`
reproduces the orphaned poll 5/5 under ASAN.)
* **h2 sessions re-reported a post-teardown RST.** Node's
`socketOnError` starts from `const session = this[kSession]` and does
nothing when the session is gone; Bun had no destroyed-guard, so a peer
RST racing our own teardown re-entered `destroy(error)`. It also now
ignores `ECONNRESET` once a GOAWAY has been *received*, like Node —
verified against v26.3.0, where GOAWAY-then-RST yields
`['goaway','goaway','close']` and no error.
* **The Windows h2 `read ECONNRESET` family had a single root cause.**
On graceful session close Bun stopped reading the socket, so the peer's
late GOAWAY sat unread in the kernel buffer and the close sent **RST
instead of FIN**. Node's `finishSessionClose` explicitly
`socket.resume()`s on graceful close for exactly this reason ([core.js
v26.3.0](https://github.com/nodejs/node/blob/v26.3.0/lib/internal/http2/core.js#L1188)).
Reproduced on Linux with the socket fault-injection layer (`recv →
ECONNRESET` after the GOAWAY exchange — the identical uncaught-error
signature) and fixed by matching Node: `resume()` before `end()` in both
session classes.
* **node:https never saw a premature EOF.** Both TLS EOF paths (peer
close_notify and the raw FIN behind it) force-closed the socket without
dispatching the end event, so the `clientError`
(`HPE_INVALID_EOF_STATE`) that fires over plain http was silently
swallowed over https — along with CONNECT/Upgrade half-open and
pipeline-drain-after-FIN. The dispatch is scoped to uWS HTTP server
sockets; every other TLS kind synthesizes its JS `'end'` from the close
event and keeps the historical force-close. Covered by a new test that
fails on the unfixed build.
* **The h2 session idle timer misjudged an active transfer as idle.** It
compared instantaneous buffer levels between expiries, so a response
that filled and fully drained within one period sampled as idle and
`'timeout'` fired mid-transfer (Windows CI's slower loopback hit the gap
reliably). Now judged by a monotonic written counter like Node's
`callTimeout`.
* **A fatally-failed native h2 write hung the session forever.** The
parser treated a kernel-rejected send as "wrote 0" and re-buffered,
waiting for a drain that can never come; on Windows the peer's RST
routinely completes the send before the read path observes anything, so
nothing ever closed the socket — the flood tests timed out only there. A
fatal errno now tears the transport down from the deferred tick
(reproduced and verified on Linux via socket fault injection), and
Windows reports real WSA codes instead of a generic fatal sentinel.
* **Queued pipelined responses under-counted their output, deadlocking
the read gate.** Node's `_storeHeader` counts the serialized header
block in `outputData`; Bun counted only body chunks, so power-of-two
chunks could land `outgoingData` exactly on `writableHighWaterMark` and
pause reads one request earlier than Node — hanging a client that
pipelines the unblocking request behind the crossing one
(`test-http-pipeline-socket-parser-typeerror`). Headers are now
accounted once per queued response.
* **Process exit stranded FileSink refs** (piped stdout with a pending
write leaked at every exit — flagged by LSan once the worker-teardown
fix stopped masking it). Released from finalize while the VM shuts down,
same pattern as the `H2FrameParser` fix.
* **More quarantine entries removed, none added**: the grpc-js pair
(passes 45/0 debug, 4× clean under ASAN), the http/fetch ASAN trio, and
the Windows half-close entry are gone from `test/expectations.txt`;
remaining entries are down to platform-environmental causes
(FinalizationRegistry timing on musl, darwin CI routing).
* **Session write-progress was read from a frozen mirror.** The
idle-timer fix initially compared `socket.bytesWritten`, but the JS
getter only mirrors the native counter on drain events - which the
parser's direct native writes never raise. Now reads the native handle's
live counter (with the JS getter as the fallback for duplexPair-backed
sessions).
* **Error-path teardown now matches Node's `finishSessionClose`
literally**: `end()` first, hard destroy one `setImmediate` later -
Node's own documented Windows-ECONNRESET avoidance ([core.js
v26.3.0](https://github.com/nodejs/node/blob/v26.3.0/lib/internal/http2/core.js#L1188)).
The immediate `destroySoon()` destroyed before the peer drained our
final GOAWAY, turning its close abortive.
* **A refused connect stopped failing grpc-style clients.** The
fatal-write teardown closed via `close_and_detach`, severing the JS
wrapper before the close could dispatch - `http2.connect` to a
nonexistent server emitted neither `'error'` nor `'close'` and grpc-js
calls waited out their full deadline on all platforms. The teardown now
closes without detaching, and leaves not-yet-established sockets to the
connect-error path.
* **macOS's racy `EPROTOTYPE` no longer kills healthy connections.**
libuv retries it
([RETRY_ON_WRITE_ERROR](https://github.com/libuv/libuv/blob/v1.x/src/unix/stream.c));
Bun renamed it to `ECONNRESET`, which the new fatal-write handling then
acted on - spuriously tearing down darwin h2 sessions mid-transfer. Now
classified with the transient errors and retried.
* **Write retries no longer unpause a paused socket.** All nine
backpressured-write re-arm sites set the poll to `READABLE|WRITABLE`
absolutely, silently resuming reads the application had paused; they now
go through one pause-preserving helper.
* **`failWrite` applies the same listener policy as `SocketEmitEndNT`**:
a failed flush on an orphaned socket (no callback, no `'error'` listener
- an h2 teardown racing the peer's reset) closes quietly instead of
surfacing an unhandled `write ECONNRESET`.
* **Session `destroy()` is now idempotent, like Node's** (`if
(this.destroyed) return;` opens Node's, v26.3.0). A second destroy
re-ran the teardown and re-emitted `'error'`; on the Windows agents a
surfaced socket reset had consumed grpc-js's `once('error')` absorber,
and the received-GOAWAY handler's second destroy then threw an unhandled
*"Session closed with error code 8"* on every Windows lane. Guarded with
a one-shot latch (not the `destroyed` getter, which reads "socket
detached" and is set by `#onError` pre-destroy) that releases when a
validation-failure destroy throws — pinned by the conformance suite.
* **Windows peer-FIN delivery on the libuv path**: `UV_DISCONNECT` —
AFD's only signal for a peer FIN with no read outstanding — was never
requested, so a client FIN against a half-closed server socket never
fired and `server.close()` waited forever. Now armed unconditionally,
delivered one-shot (AFD re-reports it forever once signaled; a
level-triggered re-fire on a paused socket starved the write side), and
mapped to the EOF hint **only for sockets whose write side is already
shut down** — unlike kqueue's `EV_EOF`, AFD can signal DISCONNECT while
data is still in flight, and an unconditional EOF mapping truncated
in-flight transfers; for a shut-down socket no data-bearing flow
remains, and that is exactly the state that hung.
* **Fatal-write errno whitelist in the h2 transport latch**: the
teardown latch treated every errno below `-1` as connection death. It
now enumerates the peer-gone class
(`EPIPE`/`ECONNRESET`/`ECONNABORTED`/`ENOTCONN`/`ETIMEDOUT`/`ENET*`/`EHOSTUNREACH`
+ the matching raw WSA codes); anything unclassified keeps the
historical re-buffer behavior instead of killing a live session (macOS
returns racy errnos from `send()` on healthy sockets).
* **Received error-GOAWAY after our own graceful close is mutual
teardown**: grpc-js `forceShutdown` destroys server sessions with
`NGHTTP2_CANCEL`, racing a GOAWAY(8) against a client that already
`close()`d. Node never observes that frame (its socket is torn down
first); on the Windows agents it deterministically arrived and destroyed
the closing session with an unhandled error. A session that is closed
with zero live streams now destroys cleanly on an error GOAWAY; a
session that did **not** initiate close keeps Node's exact throw
(verified side-by-side with v26.3.0).
* **Deferred stream-error sweep respects destroyed streams —
precisely**: the native per-stream error sweep defers each emission with
`process.nextTick`, and session destroy's synchronous pass can tear the
same stream down first. Re-destroying a listener-less destroyed stream
re-emitted the error as an uncaught exception (Windows); skipping *all*
destroyed streams swallowed grpc's terminal status codes on darwin (`1
CANCELLED` across ten suites). The sweep now skips only listener-less
destroyed streams — listened ones keep the delivery, de-duped by Node's
`errorEmitted` semantics.
* **Windows event-loop wedge resolved at the accounting layer — the root
of the "server.close() waits forever" class** (mechanism and A/B proof
in #34158): `uv_run`'s alive-guard skips timer processing *and* I/O
polling when the loop has no ref'd handles, and every uSockets handle is
`uv_unref`'d by design, so a graceful `server.close()` awaiting a
half-closed connection's teardown wedged forever once the server's
KeepAlive dropped. Three pieces land here, in dependency order:
* **Truthful request accounting on tunnel handoff**: a raw
`'upgrade'`/CONNECT exchange never released `pending_requests` (the
tunneled socket's close had no release path), so the server's all-closed
promise silently never resolved for such connections. The handoff now
marks the response TUNNELED and rides the existing single-fire
`mark_request_as_done` path.
* **Truthful websocket close accounting**: server-initiated
`ws.close()`/`terminate()` pre-set the closed flag and `on_close` then
skipped the `active_connections` decrement — every server-initiated
close leaked the count forever (`server.stop()`'s promise never
resolved; reproducible on current releases) — and nothing re-evaluated
server teardown when websockets drained. `on_close` now owns the single
decrement and notifies the server.
* **Graceful-stop keeps the server's ref until the drain completes**
(Node parity: `server.close()` keeps the process alive until connections
end), so the Windows loop keeps polling through teardown. Direct
loop-layer fixes (always-run tick with a bounded deadline) were tried,
CI-validated as unwedging, and deliberately reverted — they change
Windows timer punctuality globally; that avenue is documented in #34158.
* **Silent write-path jam on unclassified send errnos fixed** (the
darwin h2 silent-death class): an errno that was neither
would-block/transient nor in the peer-gone set fell between two layers —
the socket layer reported it fatally *without re-arming the writable
poll*, and the h2 transport's whitelist re-buffered the bytes *waiting
for a drain that could never come* — leaving a half-alive session that
reads frames but never writes again and dies silently later
(wire-tape-proven on the darwin agents: the client never even sends its
SETTINGS ACK; macOS's racy `EPROTOTYPE` on healthy sockets is the
natural trigger, previously "handled" by an infinite silent retry).
Deterministically reproduced on Linux via fault injection (×8 recovers /
×9 jammed forever). Unclassified errnos now get a bounded retry window
(32 consecutive failures, libuv-style) and then surface like peer-gone
errors; the h2 fatal check returns to the blanket form; and a second
silent-loss bug found in the same audit — node:net's drain-path flush
discarding fatal errnos, acking silently truncated streams as flushed —
now fails the pending write and emits `'error'`. Fault-injection
regression tests cover burst-recovery and sustained-failure surfacing at
both the h2 and net layers.
* **Stale fatal-write latch fixed** (the darwin h2 silent-death
residual): a failing write latched the deferred fatal teardown, but when
the same-cycle retry succeeded and drained the buffer, the deferred
flush still destroyed the now-healthy session — silently, with no error
on any channel. Reproduced on Linux by fault injection at the exact
SETTINGS-ACK window the darwin wire tapes identified. The teardown now
verifies the buffer is still undrained before closing.
* **Peer resets now reach write-only and paused polls on Windows** (the
aarch64 PING-flood exit hang): libuv never subscribed `AFD_POLL_ABORT`
unless readable polling was armed, and masked its report — so an RST
against a socket armed for writable+disconnect was structurally
invisible (vendored patch fixes request + report). A paused socket then
discriminates without violating the pause: `MSG_PEEK` separates graceful
FIN (deferred until resume, as before) from a reset — including a reset
*behind* buffered data, recovered via `SO_ERROR` — which errors
immediately like Node's paused sockets (an abandoned, never-resumed
socket otherwise never learns its peer died and pins the process
forever).
* **Windows net-reset class un-quarantined and green**: the three
main-quarantined `test-net-*reset*` tests ("reset not surfaced as
ECONNRESET") pass on the Windows lanes with the `UV_DISCONNECT` delivery
fixes — the entries are removed rather than carried.
* **Staged/event-taped twins added** for the platform-intermittent
scenarios (half-close-mid-upload teardown; PING-flood teardown over both
loopback families and the dual-stack default path; late-RST tolerance;
push-refusal conformance) plus a default-localhost loopback contract
test: same contracts as the originals, but every stage runs under its
own deadline and failures print the full event/frame tape instead of a
silent timeout. These located every root cause fixed above, and they
stay as regression coverage.
* **`test-http2-ping-flood.js` is dropped rather than shipped red**: it
was added by this branch's suite sync (never on main — the old
expectations machinery removed http2 files from runs entirely), and on
windows-11-aarch64 it cannot pass for *any* runtime: its never-reading
flood client cannot observe the teardown reset on Windows (`SO_ERROR`
doesn't latch a received RST, a zero-byte `send()` still succeeds,
`MSG_PEEK` sees only the buffered data, and the one-shot AFD DISCONNECT
report is consumed by the FIN that node's own `finishSessionClose` shape
— ported verbatim here — sends first). The flood-detection contract is
covered on every lane by the staged twin instead.
* **uWS cleanups** (Jarred): the runtime node-http-compat flag is gone
(the mode is a template instantiation — `setUsingNodeHttpCompat(bool)` →
`enableNodeHttpCompat()`), three `[[maybe_unused]]` markers removed, the
per-response booleans folded into the flags word, and the repeated byte
loads in the header/trailer value loops hoisted into a local.

### Follow-ups (recorded, not in this PR)

* Replace the `-errno` / `-1` sentinel protocol on the write path with
an explicit `WriteResult::Fatal(errno)` variant, and plumb Windows
WSA→errno translation so Windows surfaces fatal writes too.
* Surface the fatal-flush errno on the remaining `internal_flush` call
sites (`on_open` deferred flush, `end_buffered`, `flush()`), and don't
re-buffer a chunk whose send already failed fatally.
* `h2_frame_parser`'s direct native writes treat a negative
`write_maybe_corked` result as "wrote 0, re-buffer"; the session should
fail the stream instead.
* A trailer section whose bytes are malformed (NUL, bare CR/LF in a
field line) currently parses to "no trailers" while Node's llhttp
rejects the whole message; strict per-byte trailer validation (and the
`clientError` it implies) is recorded here rather than widening this PR.
* A Node-v26.3.0 source audit of net/tls/http/https/http2 (every finding
double-verified with a Node citation) produced 34 further divergences
for this area and 9 for `node:tls` (handed to the tls PR); they will be
addressed in follow-up batches.

### Tests

* 78 upstream tests added (all passing on at least one platform), 84
resynced to v26.3.0 content.
* Adopted the standalone server-timeout suite contributed via #32942
(closed in favor of this PR) as
`test/js/node/http/node-http-server-timeouts.test.ts`: raw-socket probes
of `headersTimeout`, `requestTimeout`, `server.setTimeout`, and
`keepAliveTimeout`, including the guard that `requestTimeout` stops at
request completion and never fires while a slow handler is still
streaming its response. All six pass on this branch; five of six fail on
Bun without this PR (the knobs never fire).
* The huge-ArrayBuffer `structuredClone` tests (added here for the 2GiB
serialization-buffer growth crash) now use the smallest buffer that
still exercises that growth path and treat a host OOM kill of the child
(`SIGKILL`, no output) as an environment skip; any other signal, nonzero
exit, or wrong round-trip still fails. Their previous ~5GB peak was
OOM-killed on the Linux x64 CI runners.
* `test/expectations.txt`: **−135 entries, +0**. This PR does not
quarantine a single test. The eight named-pipe tests that cannot run on
Windows are gated in-file with `common.skip()` and a reason, which is
how the upstream suite gates a platform gap — and an improvement on
main, where they carry `if (common.isWindows) return;`, a bare top-level
return that reports a false **PASS** without running an assertion.
* The `--expose-internals` harness shim gained `internal/http`,
`internal/options`, and `internal/streams/state` entries so more
upstream tests run unmodified.
* Bun-authored tests updated where they asserted the old divergent
behaviour (http2 wire error codes, clientError handling, abort-signal
error code, TLS option message wording, server-timeout auto-destroy).

### Not vendored (the 46-file gap to 100%)

Upstream files that are **deleted, not quarantined** — they exercise
Node-internal machinery Bun has no equivalent for, and they never passed
on main either (main only appeared green because `expectations.txt`
skipped them):

* `internalBinding('http2')` shape / monkey-patching and
`require('internal/http2/util')` — `binding`, `client-onconnect-errors`,
`info-headers-errors`, `respond-errors`, `respond-nghttperrors`,
`respond-with-fd-errors`, `server-push-stream-errors`,
`util-headers-list`, `util-update-options-buffer`,
`options-max-headers-exceeds-nghttp2`, `socket-proxy`,
`allow-http1-upgrade-ws`
* `NODE_DEBUG=http2` output, `perf_hooks` `'http2'` entries,
nghttp2-exact frame padding, `async_hooks` resource lifecycle events,
and adopting a raw `net.Socket` into an h2 server via
`emit('connection')`

`test-http2-ping-flood.js` is deleted for a different reason — a
platform gap rather than missing internals. Its client floods PINGs and
never reads; flood detection, session teardown and `server.close()` all
complete, but on **windows-11-aarch64** the process then cannot exit: a
paused socket with the FIN deferred behind buffered data cannot observe
the peer's later RST without consuming the stream (AFD's one-shot
DISCONNECT report is consumed by the FIN, `SO_ERROR` does not latch a
received RST, and a zero-byte `send()` still succeeds). Node's own
client would sit in the identical state — the test simply never ran on
this lane before this PR un-skipped the h2 suite (Node's CI has no
windows-11-aarch64). Removed rather than quarantined, per this PR's
no-new-`expectations.txt` rule. The reset-observability work it prompted
(AFD `ABORT` subscription, paused-socket terminal probes, sweep-based
escalation) is kept and is what un-quarantines the `test-net-*reset*`
trio.

### Known platform gap (unchanged by this PR)

`Bun.serve({ unix: ... })` does not bind a Windows named pipe, so eight
upstream unix-socket http/https tests `common.skip()` on Windows. The
client side already works via `net.connect`. Not a regression — main
hides the same gap behind a bare `return`.

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 2 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/http/serve.test.ts
test/js/bun/net/socket-syscall-fault.test.ts
test/js/bun/websocket/websocket-server.test.ts
test/js/node/net/net-syscall-fault.test.ts
test/js/node/net/node-net.test.ts
test/js/node/tls/tls-syscall-fault.test.ts
test/js/web/fetch/fetch-leak.test.ts

<!-- robobun:evidence:end -->

---------

Co-authored-by: Alistair Smith <hi@alistair.sh>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fastify timeout settings not respected when using Bun runtime with NestJS and Fastify adapter

1 participant