node:http: remove the dispatch promise so that a 'close' emitted by user code acts as in Node.js

RO
20d ago
+1131-417

Problem

  • res.emit('close') on an open node:http response makes the server answer with a bare \r\n. The next request throws ERR_HTTP_SOCKET_ASSIGNED. Node.js sends the full response. Found in #44217, not by a user.
  • onNodeHTTPRequest (src/js/node/_http_server.ts) returned a promise and resolved it at any 'close' of the response. node_http_request_on_resolve (src/runtime/server/NodeHTTPResponse.rs) then ended the open response.

Fix

  • onNodeHTTPRequest returns nothing. An open response completes through its 'finish' listener and native handle. Deleted: the close slot, the own ServerResponse.prototype.emit, the hand-off promises, both native promise reactions.
  • A hand-emitted socket 'close' closes the open response and aborts an incomplete request, like Node's socketOnClose. A 'connect' or 'upgrade' hand-off removes that listener.
  • Verified: test/js/node/http/node-http.test.ts (4 new tests, main fails 3, values from Node.js v26.3.0). Suites: Notes.
  • Self-reviewed: 9 concerns raised, 7 addressed (Notes).

Background

  • uWS calls the dispatcher onNodeHTTPRequest once per request. Native code keeps a response pending until JS ends it or the connection closes.
  • The promise also made native code run the listener's ticks and promise jobs, then report unhandled rejections. Response flags do that now.
  • Considered: delete only the own emit (21 lines). That fixes res.emit('close') but not a socket 'close', and the promise stays.

Downsides

  • A hand-emitted socket 'close' closes the open response, as in Node.js. A later end() sends no body. main sent it.
  • Closed this way before its header, a response still answers with an empty 200. Node.js sends nothing (#39366, Notes).
  • Other callers: nothing added. 1 promise and 1 closure fewer per open response, .text 5,632 bytes smaller.
Notes

Repro. #44217 lists this case under "Found, not fixed here". No user report exists. Node.js v26.3.0 prints the full HTTP/1.1 200 OK ... Content-Length: 4\r\n\r\nbody. Bun 1.4.3 and main print "\r\n".

import http from "node:http";
import net from "node:net";
const server = http.createServer((req, res) => {
  setImmediate(() => {
    res.emit("close");
    setImmediate(() => res.end("body"));
  });
});
server.listen(0, "127.0.0.1", () => {
  const c = net.connect(server.address().port, "127.0.0.1", () => c.write("GET / HTTP/1.1\r\nHost: t\r\n\r\n"));
  let got = "";
  c.on("data", d => { got += d.toString("latin1"); });
  setTimeout(() => { console.log(JSON.stringify(got.replace(/Date: [^\r]+\r\n/, ""))); c.destroy(); server.close(); }, 500);
});

Effects of the same signal on main. Each is a scenario of node-http-emitted-close-fixture.js.

User codemainNode.js v26.3.0 and this PR
res.emit('close') after the listener returned, then res.end('body')\r\n, then ERR_HTTP_SOCKET_ASSIGNED for the next requestthe full response
the same after res.write('a')no terminating chunk1\r\na\r\n4\r\nbody\r\n0\r\n\r\n
res.emit('close') in the listener, then a second requestresponse 2 leaves first, res.end() of response 1 throws ERR_HTTP_HEADERS_SENTresponse 1, then response 2
res.emit('close') while an ended response drainsemit() returns false, 'finish' fires before the client readsemit() returns true, 'finish' after the client reads
socket.emit('close') in a 'connect' or 'upgrade' tunnel\r\n written into the tunnelnothing written
socket.emit('close') on the socket of an open responsethe parser stops, and the response still goes outrequest aborted, connection closed
the same after the request body was read (Bun 1.4.3)the response stays open and sends the chunk of a later end()response closed, end() sends no chunk, the socket stays
socket.emit('close'), response class with its own assignSocket()\r\n, connection stays openrequest aborted, connection closed

The JS-only cut. A preload that deletes the own ServerResponse.prototype.emit (21 lines in the source) on the merge-base binary makes 21 of the 25 TCP rows of node-http-emitted-close-fixture.js equal Node.js. The merge base has 3, this PR 25 (fixture as of 09df76ac2b). The 4 rows left have a hand-emitted socket 'close': two with an own assignSocket() (\r\n, and the connection stays open) and the CONNECT and upgrade tunnels (\r\n written into the tunnel). They need the hand-off promises gone. The fixture has six more rows now: a socket 'close' with the default classes (2), a tunnel on a connection that served a request before, and a socket 'close' after the request body was read (3). I did not run the cut on them. When JS returns nothing, the native promise arm has no caller, so the same change deletes it.

What else the promise did. Native code read a pending promise as "this response is not complete". It then ran the ticks and promise jobs of the listener before its last steps, and reported unhandled rejections after them. The dispatch now reads the same from the flags of the response: CURRENT and not REQUEST_HAS_COMPLETED. Nothing else depended on the promise. node-http-completion-paths-fixture.js runs 13 ways an exchange ends: end in the listener, after an await, in a later turn, a 32 MB body against a client that does not read, a client that leaves, res.destroy(), a body that nothing reads, half of a body (then the end of the response, or the client leaves), two pipelined requests, a 'connect' and an 'upgrade' hand-off, and a ws upgrade. main and this PR print the same lines: the same order of 'finish' / 'close' / 'aborted', and 0 event loop refs left after server.close(). The test for it passes on main too.

Native dispatch. A breakpoint count (gdb, 200 dispatches per mode, 20 for the body that drains) gives the same numbers for the merge base 95690fc530 and this PR.

Modeset_on_aborted_handler callsdrain before the last stepsdrain after them
end in the listener00200
await null, then end02000
end from setImmediate2002000
two pipelined requests per write200100100
CONNECT hand-off2002000
'upgrade' hand-off2002000
res.destroy() in the listener2002000
end in the listener, 16 MB body that drains0200

'unhandledRejection'. A listener queues a tick, a 0 ms timer and an immediate, and leaves a rejection. The order is the same on the merge base and on this PR in 11 cases: response left open, after write(), an async listener that throws, end after an await, res.destroy(), end in the listener, end with a body that drains, a 'connect' listener, an 'upgrade' listener, two pipelined requests, two connections in one poll. In 9 of them the rejection comes right after the tick, as in Node.js. After an end in the listener it comes after the timer. With two pipelined requests it comes before the second listener. The commits of this PR before 09df76ac2b reported every rejection after the timer: the dispatch had lost its handle_rejected_promises() call. The fixture scenario "an unhandled rejection in a listener that leaves the response open" fails on them (tick, timer, rejection).

Measurements. Taken with the first commit of this PR on bf42a525d5. I did not repeat them after the later commits and the merges of main. A later commit moves the last block of the dispatch into NodeHTTPResponse::on_dispatch_returned. main has since renamed the dispatch function to on_node_http_request (#45223). The sizes of the binary and of the dispatch function are from before that move.

WhatmainPRHow
onNodeHTTPRequest bytecode: create_lexical_environment / new_func / new_promise1 / 1 / 30 / 0 / 0BUN_JSC_dumpGeneratedBytecodes=1, debug builds
onNodeHTTPRequest bytecode size749 instructions, 4,079 bytes697 instructions, 3,831 bytesthe same dump
cells per open response: Promise / JSLexicalEnvironment / Function1.00 / 1.00 / 5.000.00 / 0.00 / 4.00heapStats(), N=500, 3 runs each, debug builds
calls of the own ServerResponse.prototype.emit per response40counter, 1000 requests
Object.hasOwn(ServerResponse.prototype, "emit")truefalse (as Node.js)
close-slot reads per response, end in the listener / from setImmediate2 / 20 / 0counter, 1000 requests
event loop refs / native responses left over 13 completion paths0 / 00 / 0activeTasks, heapStats() after Bun.gc(true), Linux x64
the same over 6 paths with a hand-emitted 'close'hangs0 / 0
release binary, stripped80,844,360 bytes80,836,168 (-8,192)ls -l, release builds, same toolchain
.text58,159,221 bytes58,153,589 (-5,632)llvm-size -A
Bun__NodeHTTPRequest__onResolve + onReject957 + 1,010 bytesabsentllvm-nm -S
on_node_http_request_with_upgrade_ctx, each of 4 instances3,852 bytes, 869 instructions, 58 calls3,134 bytes, 715 instructions, 47 callsllvm-nm -S, llvm-objdump -d
PromiseFunctions entries5654ZigGlobalObject.h
size_of::<NodeHTTPResponse>()544 bytes536debug info (gdb)
bundled builtins: node/_http_server.js / internal/http.js96,531 / 13,075 bytes94,890 / 12,841wc -c, release builds
user CPU per request, end in the listener / after await / from setImmediatemedian 10.25 / 10.45 / 10.45 us (9.6 to 11.4)median 9.88 / 9.80 / 9.69 us (9.0 to 11.3)process.cpuUsage(), 100,000 keep-alive requests, 8 interleaved runs. The ranges overlap: the host was loaded
instructions per requestnot measurednot measuredperf and valgrind cannot be installed in the container (perf_event_paranoid is 4)

Suites. Debug+ASAN builds, one file at a time, on Linux x64. CI runs the tests on macOS and Windows.

  • This head (f4753eb40a, with main at 5eab34dacb merged): node-http.test.ts 296 pass, 1 skip, 0 fail with --timeout 120000. The other 37 files of test/js/node/http and the 21 files of test/js/first_party/ws pass. node-http-emitted-close-fixture.js prints the lines of Node.js v26.3.0 on all 31 TCP rows and 25 TLS rows.
  • The same head: 115 of Node's test-http-* and test-https-* files about upgrade, CONNECT, close, abort, destroy, pipelining and keep-alive, all pass.
  • fe005a21c4 (main at 47ba1c8d85): the same runs, on a slower debug build. At the default 5 s, 13 tests of node-http.test.ts that exist on main timed out: each needed up to 9 s. "should not crash on abort (node-http-uaf-fixture.ts)" needed 63 s and allows 60 s. test-http-agent-keepalive.js failed: the 'close' of the client socket came later than the 1 ms timer of the test, and it passed with a 100 ms timer. CI passed on that head (build 125546, 181 of 181 jobs).
  • 09df76ac2b against the merge base 95690fc530: 64 bun:test files (test/js/node/http and the express, body-parser, hono, st and socket.io tests), 1,424 tests pass. 9 tests in 4 third-party files fail, and they fail on the merge base too in this container (express-memory-leak 4, socket.io-handshake 3, express 1, socket.io-close 1).
  • First commit: 463 of Node's test-http-* and test-https-* files, 456 pass. 6 of the other 7 pass when they run alone, on both builds. test-https-timeout.js times out on both. The new tests also pass with BUN_JSC_validateExceptionChecks=1.

Windows. The scenario with a draining response does not run there. Winsock takes the 32 MB body in one send() on loopback, so no response is left that drains. node-http-pinned-write.test.ts skips its Windows cases for the same reason.

The count of native responses is gone. Earlier commits also counted the NodeHTTPResponse cells that heapStats() reports after a full GC. For a cell that stayed counted, the fixture looked for a path from a root in the debugging heap snapshot, and it reported 0 when it found none. That check could report 0 for a cell that was alive (review finding), so fe005a21c4 removes the count. The fixture reports the order of the events and the event loop refs, like the other node:http leak tests (node-http-ondata-reregister-leak, node-http-with-ws).

  • What the plain count showed (one exchange per path, then 64 turns with a full GC each): 0 after every path on the Linux and macOS lanes (build 122201) and on Windows 2019 x64 (build 122167).
  • On Windows 11 aarch64 (build 122167), one cell stayed counted after the path "the client leaves" and through the next four paths. The count was 0 again from "two pipelined requests" on (3 of 4 attempts). With a hand-emitted 'close', one cell stayed counted after "res.emit('close'), then the client leaves" only (4 of 4 attempts).
  • I did not find what holds that cell. I cannot run Windows. Nothing holds it for good: later exchanges in the same process free it, and the event loop refs are 0 in every row.

The kPendingFinish branch of the deleted emit. It finished a draining response when its 'close' came first. The close path of the socket (flushPendingFinish in #onClose) and the HTTP/1 fallback already do that before they emit 'close'. A counter in that branch on main had 3 hits over the 64 test files. All 3 came from the new fixture, which emits 'close' by hand. An earlier run of the 463 Node files had none.

Tests that used the old behaviour.

  • node-http-displaced-response: the trigger emit-close and the scenario completedButPending are gone. A 'close' emitted by user code no longer gives the connection to the next response. The state "request complete, not ended, still pending" has no producer left: on_request_complete() now only runs for a response that ended, whose socket closed, or that upgraded.
  • node-http-ondata-reregister-leak.test.ts: the scenario with res.emit('close') stays and still expects 0 refs. Its comments no longer describe a handler promise.
  • The completed-response branch of handle_abort_or_timeout stays. A connection that closes after its response completed enters it, and it releases the pending request of an ended response whose body reader is still armed (gdb backtrace in the path "half of a body, then the end of the response").

Still different from Node.js, not changed here.

  • A dispatch that finds socket._httpMessage set throws ERR_HTTP_SOCKET_ASSIGNED (assignSocketInternal). Node.js queues that response (parserOnIncoming). This PR removes the producer that a hand-emitted 'close' was.
  • 'unhandledRejection' after an end of the response in the listener comes after a due timer. With two pipelined requests in one read it comes before the second listener. Node.js reports it before the timer, and runs both listeners first (#43736 is about that order). Both are as on main.
  • res.emit('finish') from user code: Node.js runs its resOnFinish. Bun detaches the socket through other code.
  • res.destroy() and req.destroy() before the headers go out answer with HTTP/1.1 200 OK and Connection: close (#39366).
  • A hand-emitted socket 'close' after the request body was read, with no header sent, then res.end('body'): Node.js sends nothing, because its write_() returns before _implicitHeader(). Bun sends a 200 with Content-Length: 0, as both do for res.end() without a chunk. The socket stays open in both. To send nothing, the response must finish in JS and stay open in native code until the connection closes. This PR removes that state.
  • uws_res_end_without_body writes no status line when close_connection is false. It is shared with Bun.serve. The node:http caller that reached it with an open header section is deleted.

Other effects.

  • Found, not changed here: after a ws connection of an https server closed on both sides, the process can exit before the callback of server.close(callback) when nothing else holds the event loop. The merge base does the same. Bun 1.4.3 calls back.
  • The steps that the dispatch runs after the handler returned are NodeHTTPResponse::on_dispatch_returned now. Without the promise, that block reads only the response. As a method it compiles once, not once per server type (SSL x DEBUG). The generic_body_not_generic lint of mordant reported the block.
  • The internal Bun.serve({ onNodeHTTPRequest }) (typed in src/js/private.d.ts only) no longer waits for a returned promise.
  • #39366 and #43736 edit the same native dispatch function. #39366's note that node_http_request_on_resolve stays unchanged no longer applies.

Self-review. A review of this diff ran for about 13 hours on 09df76ac2b. The machines that run it restarted four times, and its final report did not complete. Its answer to "should this change exist" was: wanted, keep the PR open. The 9 concerns that I could read from it, and what I did:

  1. 'unhandledRejection' came after due timers and after other connections: fixed in 09df76ac2b (section above).
  2. The dispatch drained after its last steps for a response that ends while its body drains: fixed in the same commit.
  3. The repro was described as "from the report": corrected. It is the case from #44217.
  4. The smallest fix was not named: "The JS-only cut" above.
  5. ERR_HTTP_SOCKET_ASSIGNED as a class is not fixed: stated under "Still different from Node.js".
  6. The abort for a hand-emitted socket 'close' covered only a response with its own assignSocket(): fixed in e72a47cf24. The listener that every connection has for the 'close' of its socket does it now, as Node's socketOnClose.
  7. The count of native responses decided by the heap snapshot: the count is removed (section above).
  8. Not done: two PRs (JS, then native). The native promise arm would be dead code between them.
  9. Not done: the sizes of the release binary are from the first commit.

Review of e72a47cf24. Three findings.

  1. The hand-off to 'connect' and 'upgrade' left the socketOnClose listener on the socket: 2525782846 removes it, as Node's onParserExecuteCommon does. On Bun 1.4.3 the 'connect' listener of a connection that served a request before sees 2 'close' listeners. Node.js and this PR have 0.
  2. The count of native responses could report 0 for a cell that was alive: removed (section above).
  3. A socket 'close' that is emitted after the request body was read: Node.js closes that response too, and three new fixture rows equal Node.js. One case differs and is not changed ("Still different from Node.js").

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http/node-http.test.ts