Skip to content

node:http: report backpressure for an empty res.write() - #43496

Closed
robobun wants to merge 3 commits into
mainfrom
robobun/5adaf836/node-http-empty-write-backpressure
Closed

robobun wants to merge 3 commits into
mainfrom
robobun/5adaf836/node-http-empty-write-backpressure

Conversation

@robobun

@robobun robobun commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • res.write("") (or an empty Buffer) on a node:http response under backpressure returns true and runs every parked write callback on the next tick. The 'drain' owed by the earlier write() === false never fires. A handler that waits for it hangs.
  • The cause is write_or_end (src/runtime/server/NodeHTTPResponse.rs:2114). uWS accepts an empty write without a look at its backpressure. The WantMore arm then calls clear_on_writable(), and ServerResponse.prototype.write (src/js/node/_http_server.ts:3392) treats the result as flushed.

Fix

  • write_or_end answers an empty write itself while bytes are pending. It keeps the drain armed and returns -1. JS then parks the callback and returns false.
  • ServerResponse.prototype.write reports the per-turn high water mark count for an empty chunk too, as Node does.
  • A response that cannot have a body (HEAD, 204, 304) still ignores the write and returns true, like Node.
  • Verified: test/js/node/http/node-http-backpressure.test.ts (two tests fail without the fix, all four pass on Node v26.3.0). Also node-http.test.ts and the Node test-http* tests. Self-reviewed: 8 concerns raised, 4 addressed, 4 answered in the Notes.

Background

  • uWS keeps bytes that the kernel did not take in a backpressure buffer per socket. It calls onWritable when the socket drains.
  • For a write above 16 KB, NodeHTTPResponse holds the unsent tail by reference (pending_pinned_write), not in the uWS buffer.
  • The native write returns a negative number for backpressure. JS then parks the write callback until the drain callback runs it and emits 'drain'.
  • JS also counts the bytes written in one event-loop turn. At the high water mark write() returns false.
Notes

Repro (plain bun file.mjs and node file.mjs):

import http from "node:http"; import net from "node:net"; import { once } from "node:events";
const BIG = Buffer.alloc(8 * 1024 * 1024, "a");
const server = http.createServer(async (req, res) => {
  res.setHeader("Content-Length", BIG.length + 2);
  const events = [];
  const first = res.write(BIG, () => events.push("big cb"));
  const second = res.write("", () => events.push("empty cb"));
  const drained = await Promise.race([once(res, "drain").then(() => true), new Promise(r => setTimeout(r, 3000, false))]);
  console.log(JSON.stringify({ first, second, drained, events }));
  res.end("ok");
});
await new Promise(r => server.listen(0, "127.0.0.1", r));
const c = net.connect(server.address().port, "127.0.0.1");
let received = 0; c.on("data", d => (received += d.length));
c.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
await once(c, "close"); console.log("client received", received); process.exit(0);
  • Node v26.3.0: {"first":false,"second":false,"drained":true,"events":["big cb","empty cb"]}
  • Bun 1.4.3 and main: {"first":false,"second":true,"drained":false,"events":["big cb","empty cb"]}. The client still receives all 8388710 bytes.
  • With this PR: the same output as Node.

Node's rule. write_() sends an empty chunk through _send("") to conn.write("", cb) (_http_outgoing.js#L1013). Writable.write queues the callback behind the earlier writes and returns state.length < highWaterMark (writable.js#L576). A message with no body returns true before any of that (_http_outgoing.js#L983-L991).

Details of the change:

  • "Bytes are pending" means pending_pinned_write is set or the uWS buffer is not empty. Both are checked because the held tail is not in the uWS buffer. In both states the last socket write was partial, so the socket polls for writable and the drain callback does run.
  • The check runs before spill_pending_pinned_write. An empty write has no bytes to order behind the held tail, so the tail stays zero-copy. Before, an empty write copied it into the uWS buffer.
  • The return value is -1 because the JS caller only tests result < 0, and -0 is not negative.
  • The body-less guard in JS is needed because of the native change. A HEAD or 204 response that is pipelined behind a response that still flushes sees a socket buffer that is not empty. With the native change alone its write() returned false and parked the callback (checked with a probe). The two but not on ... tests pin this. They pass without the fix by design.
  • One difference from Node stays, and it is the one that writes with bytes already have. Bun reports backpressure for any byte that waits in user space. Node reports it only at the high water mark. So with 1 to 65535 bytes pending, an empty write returns false here and true in Node. A 'drain' always follows the false.
  • Windows: node-http-pinned-write.test.ts notes that Winsock can take a whole 64 MB payload in one send(). Then only the per-turn count reports backpressure. The first test accepts both paths there, so on Windows it covers the JS part only. The JS part is also what keeps the rule the same on all platforms.
  • The first test uses 64 MB because the response must stay backed up while the client does not read. The limit is tcp_wmem max plus tcp_rmem max (4 MB and 6 MB by default, 4 MB and 32 MB on the machine I used).

Self-review (three independent read-only passes: native code, JS and Node compatibility, tests). No defect found in src/. Concerns and answers:

  1. Test 1 did not check when 'drain' fires. Addressed: it asserts that 'drain' has not fired before the client reads (Linux and macOS).
  2. A handler exception could be hidden by a socket or fetch error. Addressed in tests 1 and 2 with Promise.all.
  3. A test comment claimed the response stays backed up on every platform. Addressed.
  4. The HEAD and 204 tests did not prove that bytes were pending. Addressed: they assert it before the writes (not on Windows).
  5. The HEAD and 204 tests pass on the unfixed build. Answered above: they guard the JS part against the native part.
  6. A parked callback never runs if the connection dies before the drain. This is how writes with bytes already behave. node:http: fail the write callbacks a dead connection can no longer drain #39889 fails every entry of kPendingCallbacks, and the callback of an empty write is in that list.
  7. The 1 to 65535 byte difference from Node. Answered above.
  8. Test 2 waits for 'drain' without a race. Left as is: the per-turn count schedules that 'drain', this PR does not change when it fires, and a loss still fails the test by timeout.

How test 1 waits. The handler waits for the write callbacks and then reads the drained flag. Both runtimes run a parked callback after they emit 'drain', so the flag is final at that point. On an unfixed build the callbacks run at once and the flag is still false, so the test fails with a diff and no timeout. An earlier version raced 'drain' against the client's last byte. That is not valid under Node: libuv completes a batch of writes that ends in an empty buffer one writable event late, so the client saw the last byte first in 5 of 12 runs. The current block passes 30 of 30 runs under Node v26.3.0.

Windows in test 1. Three states are legitimate there: the payload is partly taken (as on Linux), fully taken, or fully taken with the one-byte write refused. The test asserts that the callbacks that ran before the client read are an in-order prefix of the writes, which holds in all three.

Comments in src/ are one line each. The Node links live here: write_() body-less branch, empty chunk to _send, writeOrBuffer return value.

Tests run with the debug (ASAN) build:

  • node-http-backpressure.test.ts: all pass, the new block 10 times in a row (and 30 times in a row under Node). On a debug build of main the two new tests fail and the two guard tests pass.
  • node-http.test.ts (162 pass), node-http-pinned-write.test.ts, node-http-nested-cork.test.ts, node-http-server-socket-end-drain.test.ts, node-http-backpressure-max.test.ts.
  • All 451 test-http-* and test-https-* files in test/js/node/test/{parallel,sequential}. Three fail: test-http-agent-keepalive, test-http-client-timeout-option, test-http-outgoing-end-cork. They fail the same way on a debug build of main (timers of 1 to 500 ms against a slow build) and pass on the release build.

Found on the way, not part of this PR:


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

uWS answers an empty write with "flushed" without looking at its
backpressure, so NodeHTTPResponse took the WantMore path. That path
disarmed the drain callback an earlier write had armed, and
ServerResponse.write then ran every parked write callback and returned
true. The 'drain' that the earlier write() === false promised never
fired.

An empty write on a response that still has unflushed bytes (the
zero-copy tail or the uWS backpressure buffer) now keeps the drain
armed and reports backpressure. ServerResponse.write also reports the
per-turn high water mark accounting for an empty chunk, and it still
ignores a write to a response that cannot have a body, like Node.js.
@robobun

robobun commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: closed. Superseded by #43557, which is merged (5d5f03f) and carries these tests.

Verified on a debug build of main at 5d5f03f:

  • The repro from the PR notes prints {"first":false,"second":false,"drained":true,"events":["big cb","empty cb"]}, the same as Node v26.3.0. Bun 1.4.3 prints "second":true,"drained":false.
  • With a client that never reads, res.write("", cb) returns false and no callback runs early, as in Node.
  • The four an empty res.write() ... tests in test/js/node/http/node-http-backpressure.test.ts pass.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: cee1b35c-1610-4642-974b-8c3ef7d504e3

📥 Commits

Reviewing files that changed from the base of the PR and between f3e5b6d and 64dc2f0.

📒 Files selected for processing (2)
  • src/js/node/_http_server.ts
  • src/runtime/server/NodeHTTPResponse.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


Walkthrough

Empty response writes now preserve backpressure when output remains buffered. Bodyless HEAD and 204 responses bypass callbacks and backpressure. New tests cover return values, callback ordering, drain events, high-water marks, and response delivery.

Changes

HTTP backpressure handling

Layer / File(s) Summary
Native unflushed-output detection
src/runtime/server/NodeHTTPResponse.rs
NodeHTTPResponse detects pending zero-copy tails or buffered uWS output. Non-ending empty writes register the writable callback and return -1 when output remains unflushed.
ServerResponse write-result handling
src/js/node/_http_server.ts
ServerResponse.write uses writable callbacks and negative write results only for responses with bodies. High-water-mark checks now include empty writes.
Backpressure and bodyless-response tests
test/js/node/http/node-http-backpressure.test.ts
Tests cover buffered empty writes, high-water-mark transitions, callback and drain ordering, complete body delivery, and HEAD or 204 responses.

Suggested reviewers: cirospaciari

Priority: ➖ Normal

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: reporting backpressure for empty node:http response writes.
Description check ✅ Passed The description explains the problem, fix, affected behavior, compatibility details, and verification results. It does not use the exact template headings, but it provides the required information in …

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

@claude claude Bot left a comment

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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked two things in the new early return in src/runtime/server/NodeHTTPResponse.rs: the raw_response.get().unwrap() cannot panic because write_or_end already unwraps raw_response for the state check at line 1880 before reaching it, and the async-context wrapping of the stored callback matches what the existing Backpressure arm does, so parked callbacks and 'drain' listeners do not run under a different context than they did before.

Extended reasoning...

Findings-present run; this note only records what was additionally examined and ruled out from reading the diff. The early return at NodeHTTPResponse.rs:2019-2030 is reached only after self.raw_response.get().unwrap().state() at line 1880 has already succeeded in the same call, so the second unwrap is safe. The with_async_context_if_needed wrapping mirrors the existing Backpressure arm's treatment of the same callback slot. The inline findings (dropped callback on disconnect before drain, _send storing a raw user callback in the onWritable slot, the win32 precision loss and the guard tests' weak preconditions, and the pre-existing inline-flush WantMore case) are what a human should weigh before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟣 src/runtime/server/NodeHTTPResponse.rs — pre-existing: a handler that awaits 'drain' after a write returned false still hangs when a later non-empty write happens to flush the backlog inline. This PR fixes the empty-chunk case but the same WantMore arm at NodeHTTPResponse.rs:2146 still calls clear_on_writable() and drops the cached callback when raw_response.write(bytes) drains the uWS buffer before the writable event is dispatched, so the owed 'drain' is never emitted. Fix: treat an armed drain as owed in every arm of write_or_end, not only for empty chunks; when WantMore flushes a backlog that had on_writable armed, schedule the cached callback (drain) instead of clearing it. The PR description names this as found-on-the-way; it is the same class as the fix here.

    Extended reasoning...

    Handler does res.write(big, cb1) with the socket full. Native goes to the Backpressure arm at NodeHTTPResponse.rs:2150, arms on_drain_shim, returns negative; JS at _http_server.ts:3398 parks cb1 and returns false, so a 'drain' is now owed per Node's contract. Before the writable event fires the peer reads and the kernel frees space. Handler then calls res.write("y", cb2) in a later turn. raw_response.write(bytes) reaches AsyncSocket::write (packages/bun-uws/src/AsyncSocket.h:320): buffer.length() > 0 so it us_socket_write()s the backlog; it fully drains, the buffer is cleared, "y" is written, WantMore is returned. The arm at NodeHTTPResponse.rs:2146-2147 calls clear_on_writable() and on_writable_set_cached(UNDEFINED). JS at _http_server.ts:3407 runs cb1 and cb2 on nextTick, accounting adds only 5 bytes so flushWriteAccountingNT does not emit 'drain', and write() returns true. No 'drain' is ever emitted for the earlier false; await once(res, 'drain') hangs. Node emits 'drain' whenever needDrain was set and the buffer empties (writable.js afterWrite). The new empty-chunk guard at…

    Verification: pre-existing. Trigger: a handler whose res.write() returned false (native Backpressure arm) issues another non-empty write after the peer has read and the kernel has room but before uWS dispatches the writable event (e.g. the write is driven by a timer, another socket's event, or a nextTick/setImmediate chain in the same loop turn), and then awaits 'drain'. Mechanism verified:… | pre-existing —…

Comment thread src/js/node/_http_server.ts
Comment thread test/js/node/http/node-http-backpressure.test.ts Outdated
Comment thread test/js/node/http/node-http-backpressure.test.ts Outdated
Comment thread src/runtime/server/NodeHTTPResponse.rs
- On Windows, assert that the callbacks that ran before the client read
  are an in-order prefix of the writes, not any object.
- The HEAD and 204 tests assert that the earlier response still has
  unsent bytes, except on Windows, where Winsock can take all of them.
- Wait for the write callbacks and do not race 'drain' against the
  client. Node completes a batch of writes that ends in an empty one a
  writable event late, so the client could see the last byte first.
Comment thread src/js/node/_http_server.ts Outdated
Comment thread src/js/node/_http_server.ts Outdated
Comment thread src/runtime/server/NodeHTTPResponse.rs Outdated
Comment thread src/runtime/server/NodeHTTPResponse.rs Outdated
@robobun

robobun commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:20 PM PT - Sep 19th, 2026

✅ @robobun, your commit 64dc2f061105719f5ab783091308cc16d52ce618 passed in Build #118460! 🎉


🧪   To try this PR locally:

bunx bun-pr 43496

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

bun-43496 --bun

@claude claude Bot left a comment

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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

The second commit addresses both test-side nits from the earlier review (the Windows prefix assertion and the earlierResponsePending precondition on the HEAD/204 tests). Beyond the inline finding, I also checked the new self.raw_response.get().unwrap() in the empty-write branch of write_or_end (src/runtime/server/NodeHTTPResponse.rs): it is covered by the raw_response.get().is_none() early return at the top of the function, and nothing between that guard and the new block re-enters JS, so it matches the existing R-2 pattern at the state() read above it.

Extended reasoning...

Findings are present (one confirmed inline note about rejectNonStandardBodyWrites on an empty chunk), so approval is not on the table. This body only records what changed since the prior review and one concrete concern that was examined and ruled out: the new unwrap() on raw_response in the !IS_END && bytes.is_empty() && has_unflushed_write() branch. write_or_end returns early when SOCKET_CLOSED is set or raw_response is None, and the only calls between that guard and the new block are StringOrBuffer::from_js_with_encoding_into (which rejects anything that is not a string or buffer rather than coercing through user JS) and the strict-content-length arithmetic, so the slot cannot have been cleared by re-entry. The two source-level concerns from the earlier review (parked callbacks on disconnect, direct _send callers) are unchanged in the code and remain as open inline threads.

Still open from earlier reviews (2):

  • Unresolved: 2 minor or pre-existing.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟣 src/js/node/_http_server.ts — pre-existing nit: a server created with rejectNonStandardBodyWrites: true still gets true from res.write("") on a HEAD/204/304 response, where Node throws ERR_HTTP_BODY_NOT_ALLOWED. The guard at http_server.ts:3350 is if (chunk && !this._hasBody), so an empty chunk skips the throw and falls into the new hasBody path at 3369-3410, which returns true and runs the callback. Node's write checks !msg._hasBody before looking at the chunk (the very block the new comment at 3366-3368 cites). Fix: when !this._hasBody, throw $ERR_HTTP_BODY_NOT_ALLOWED if kRejectNonStandardBodyWrites regardless of chunk length, at both write sites (3350 and bufferPipelinedWrite 2726); end("") stays as is since Node's end() only reaches write_ for a truthy chunk.

    Extended reasoning...

    The guard chunk && !this._hasBody at src/js/node/http_server.ts:3350 (and the queued twin at 2726) only rejects non-empty chunks. Node v26 lib/http_outgoing.js#L983-L991, cited by the PR's new comment at 3366-3368, runs if (!msg._hasBody) { if (msg[kRejectNonStandardBodyWrites]) throw new ERR_HTTP_BODY_NOT_ALLOWED(); ... } before any chunk-length branch, and write is reached for "" and Buffer.alloc(0) (only null and non-string/non-Uint8Array chunks are rejected earlier). Trigger: http.createServer({ rejectNonStandardBodyWrites: true }, (req, res) => { res.write("") }) for a HEAD request or after res.statusCode = 204. Bun: 3350 is skipped, hasBody is false at 3369, handle.write returns 0 or -1, 3398 does not park, 3408-3409 nextTick the callback and 3439 returns true. Node: throws synchronously. This is pre-existing on the base branch; the PR extends the same branch with the new hasBody guard without closing it. end("") is not a sibling: Node's end() only calls write when chunk is truthy.

    Verification: pre-existing (nit) — the base already returns true for the same call by the same route; the diff does not change the guard, only the surrounding function and adds the comment citing the Node block. Trigger: a server created with rejectNonStandardBodyWrites: true whose handler calls res.write("") on a HEAD/204/304 response. Mechanism: /home/claude/bun/src/js/node/_http_server.ts:3350 `if…

@robobun

robobun commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

Answers to the two findings that are outside the diff. Both exist on main and on 1.4.3, and this PR leaves both alone.

  • A write with bytes that flushes the backlog inline drops the owed 'drain'. Confirmed with a probe against Node v26.3.0: a client in a worker thread reads while the server thread is blocked, then the handler writes one byte. Bun returns true and 'drain' never fires. Node emits it. The response has to remember that a 'drain' is owed, which is a different fix from the one here. It is tracked as separate work and is named in the PR notes.
  • res.write("") on a HEAD or 204 response with rejectNonStandardBodyWrites: true. Confirmed: Node throws ERR_HTTP_BODY_NOT_ALLOWED for an empty string, Bun returns true (an empty Buffer throws in both). Filed as node:http: res.write("") on a HEAD or 204 response does not throw ERR_HTTP_BODY_NOT_ALLOWED with rejectNonStandardBodyWrites #43535 with the repro.

The inline threads have replies. Two led to test changes in f3e5b6d. On Windows the first test now asserts an in-order prefix of the callbacks, and the HEAD and 204 tests assert that the earlier response still has unsent bytes. That commit also stops racing 'drain' against the client. Node completes a batch of writes that ends in an empty one a writable event late, so under Node the client saw the last byte before 'drain' in 5 of 12 runs. The handler now waits for the write callbacks, which both runtimes run after 'drain'. The block passes 30 of 30 runs under Node and 10 of 10 on the debug build.

@claude claude Bot left a comment

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.

Code review found no issues

No high-confidence issues detected in this change.

Jarred-Sumner added a commit that referenced this pull request Sep 26, 2026
…inish, lifecycle) (#43557)

One pull request for the open `node:http` server pull requests. Each
root cause is fixed once, and each pull request's tests are carried
over. Node is the reference: every scenario was run under Node and under
Bun from one script, and the outputs were compared.

Fixes #4733
Fixes #18613
Fixes #40350
Fixes #43155
Fixes #43297
Fixes #43513
Fixes #43527
Fixes #25632
Fixes #31301
Fixes #43027
Fixes #43163
Fixes #43342
Fixes #43344
Fixes #43370
Fixes #43490
Fixes #43512
Fixes #43519

Each of these has a repro that is wrong on Bun 1.4.3, right on this
branch, and the same as Node.

| Issue | Not closed by this PR, because |
| --- | --- |
| #30501 (msal-node keeps Bun alive at exit) | Probably fixed. The repro
copies the teardown of msal-node. The package itself was not run. |
| #14430 (yarn: "does not support SSL") | Probably fixed.
`response.hasOwnProperty("socket")` is now `true`. yarn itself was not
run. |
| #39681 (`server.setTimeout` callback after destroy) | Probably fixed.
The repro is the deterministic case of #39686. The script in the issue
depends on timing and on Windows. |
| #43455 (`req.complete`, nine flows) | Partially addressed. Flows 1, 2,
3, 5 and 7 are fixed, and flow 6 was already right. Flow 8 (a socket
timeout while the body of an Upgrade request arrives) and flow 9 (a
request that `stream.pipeline()` destroyed never reports `complete`) are
not. Flow 4 differs only in `_readableState.ended`. |

### What changes for users

| Area | Before | After (same as Node) |
| --- | --- | --- |
| `req.pause()` | The socket stops at once. `req.complete` stays `false`
for a small body. | The body is received until the buffer is full. Then
the socket stops. |
| `res.end()` before the body arrives | `req` gets `'end'` and `'close'`
at once, and the body is lost | The request completes when its body
really ends |
| `res.destroy()` in the middle of a body | `'end'` with bytes missing |
`aborted`, then `ECONNRESET` |
| `socket.destroy()` inside the `'request'` listener | The body that
came with the head is dropped | That body is still delivered |
| `'finish'` and the `end()` callback | Fire when `end()` buffers the
bytes | Fire when the last bytes have left the socket |
| A response that closes the connection | The server half-closes and
waits for the peer | The socket closes right behind the FIN |
| `'drain'` after a later write flushed the backlog | Lost. `pipe(res)`
could hang. | Emitted |
| A pipelined request whose body continues after the previous response
ends | Body dropped, no response, `server.close()` hangs | Delivered |
| CONNECT and Upgrade tunnel sockets | Keep reading when paused or full
| Stop reading. `_read()` starts them again. |
| A tunnel write that waits for a drain when the client goes away | Its
callback, the callbacks of the writes behind it and the `end()` callback
never run | They run with an error before `'close'` |
| Upgrade request with a body, paused in its listener | The body flows
away | The request keeps its body |
| `ws` on a reused keep-alive socket | Writes after the Upgrade could
stall | Sent |
| A raw `socket.write()` behind a response that still drains (the 400
for a bad pipelined request, the reply of a `'clientError'` listener) |
Lands in the middle of that response | Sent after it |
| `server.close()` | Could report closed while connections were open |
Waits for every connection. An idle tunnel does not keep the process
alive. |
| `closeAllConnections()` on a listening server | Also stops the
listener and destroys tunnels and WebSockets | Destroys only the HTTP
connections |
| `Proxy-Connection: close` (node:http only) | Ignored. The connection
stays open. | Ends the connection, like `Connection: close` |
| A response larger than 16 KB, also in `Bun.serve` and over TLS | Up to
4 `send()` calls for each chunk. Slower than Node in most cases. | One
write for the writes of one tick. 1.1x to 2.7x the requests per second
of main, and faster than Node. |
| `socket.destroy()` and then `res.end()` in a listener | (this PR,
earlier) `req` ended as if it were complete | `'aborted'`, then
`ECONNRESET` |
| `emit('connection')` or http2 `allowHTTP1`: the response ends while
the listener still reads the body | The rest of the body is dropped |
The body is complete |
| `httpValidation: "relaxed"`, `Content-Length` or `Transfer-Encoding`
in trailers | Accepted | `HPE_INVALID_CONTENT_LENGTH`,
`HPE_INVALID_TRANSFER_ENCODING` |
| `req.complete` inside `'connect'`, and inside `'upgrade'` without a
body | `false` | `true` |
| `optimizeEmptyRequests`: `socket.parser.incoming` after the response |
Keeps the request alive on an idle connection | `null` |
| A HEAD or OPTIONS request with `Content-Length` | The body is dropped,
and `req.complete` is `true` before it comes | The request has its body
|
| `res.end(chunk)` after the client went away | `finished` and
`writableEnded` stay `false`, no `'prefinish'` | The response ends |
| An HTTP/1.0 request with an `Expect` header | `100 Continue`,
`'checkContinue'`, `'checkExpectation'` or a 417 | A plain `'request'` |
| The idle sweep of `close()` and `closeIdleConnections()` | Could
destroy a connection that was still receiving a request, or whose
response was still draining | Closes only idle connections |

### Design

| Piece | What it is |
| --- | --- |
| Request body state | `None / Pending / Complete / Aborted / Upgraded /
Detached`. Only the last chunk sets `Complete`. One function,
`leave_pending`, is the only other way out of `Pending`. |
| Read flow control | One path: `push()` returning false stops the
socket, `_read()` starts it. Both native pause buffers are removed: no
read is copied and replayed. |
| "This read is parsed" signal | `notifyWhenReadParsed()` sets a uws
state bit. uws delivers a `readParsed` event after the read. It replaces
a `setImmediate`. |
| Close during a parse | One uws bit defers a close to the end of the
current message. |
| Response finish | A response is finished when it has ended and the
socket has fully drained. |
| Idle connection | One rule, `HttpResponse::closeIfIdle()`. A
connection is idle when it receives no request (head or body) and no
response is in flight, queued or undrained. The sweep of `close()` and
`closeIdleConnections()` both use it. |
| Idle tunnel | A tunnel at read EOF with nothing left to send. uws
reports it to the server through the connection filter (`-3`, `+3`,
`-4`). It still counts for `'close'`, but it does not hold the event
loop, like a libuv handle in that state. |
| Server `'close'` | One native close promise per `listen()`. `close()`
records whether its sweep left nothing open. Then a `listen()` in the
same tick cannot hold `'close'` back, as in
`net.Server._emitCloseIfDrained`. |
| Raw socket writes | While uws holds response bytes (its buffer, the
zero-copy tail of a `res.write()`, the cork buffer), a raw write goes
through `AsyncSocket::write`, the path a 1xx line takes. So the order on
the wire is the order of the calls. |
| Upgrade verdict | One scanner and one verdict, shared by the parser
and the dispatcher. |
| llhttp | Updated from 9.3.0 to 9.4.2, as Node v26.5.0 vendors it, plus
one local patch (see below). Node v26.5.1 and later vendor 9.4.3. That
update is not in this PR. |

The parser changes also tighten request framing so that it agrees with
llhttp in more cases. There is no new API surface.

### A pause holds from the next read

The copy of the rest of a read (`nodeHttpPausedSpill`), its replay from
a posted task and the nested parse are removed. Like in Node, the rest
of the read that caused a pause is still parsed, and the socket stops at
the next read. usockets reads up to 512 KB in one call. libuv reads 64
KB.

| One paused, unread request (client sends 64 MB) | Bytes held |
| --- | --- |
| Node 25.6 | 131,018 |
| This pull request | 524,234 |

The price is in one case. A client sends 512 KB of small pipelined
requests (19,418 of them) and never reads. Each handler answers with its
own 64 KB body:

| Handler | Runtime | Requests dispatched | RSS |
| --- | --- | --- | --- |
| Answers at once | Node 26.3 | 2,425 | +177 MB |
| Answers at once | main | 41 | +9 MB |
| Answers at once | This PR | 2,425 | +171 MB |
| Answers one tick later | Node 26.3 | 4,850 | +336 MB |
| Answers one tick later | main | 2,426 | +181 MB |
| Answers one tick later | This PR | 4,850 | +330 MB |

Release builds on Linux x64. This PR now does what Node does. main held
fewer responses, mostly for a handler that answers at once. On macOS one
read can return all 512 KB. There, Bun 1.4.3 already reached +951 MB for
the handler that answers one tick later, and Node reached +1,294 MB.
`server.maxRequestsPerSocket` bounds it.

### Performance

#### Responses larger than 16 KB are faster, and now faster than Node

On main, a response that did not fit the 16 KB uWS cork buffer released
the cork. After that, each piece was its own `send()`: the buffered
head, the chunk-size line, the data, the `\r\n` and the last chunk. Over
TLS, each 2-byte piece was also its own record. Two changes fix that,
for `Bun.serve` and for node:http:

| Change | Effect |
| --- | --- |
| A write that does not fit goes out with the cork buffer and its
framing in one vectored write | No copy is added. Over TLS, the records
of all the pieces share the write batch that one `SSL_write` loop
already had. |
| The cork buffer holds 128 KB, up from 16 KB. Only a write of 16 KB or
less is copied into it, as before. | Several writes in one tick go out
in one write, like in Node. A longer write still goes out without a
copy. |

The bytes on the wire are the same. The vectored write uses `sendmsg()`
with the flags that `send()` uses.

Write syscalls for one response:

| Response | Node 26.3 | main | This PR |
| --- | --- | --- | --- |
| 4 x `res.write(16 KB)` | 1 | 16 | 1 |
| 40 x `res.write(2 KB)` | 1 | 16 | 1 |
| `res.end(64 KB)` | 1 | 2 | 1 |
| 256 KB file, `.pipe(res)` | 4 | 16 | 5 |

Throughput (req/s, the mean of 2 rounds). Node v26.3.0, main
`97246d044e`, this PR `fe0ed1fbea`, with the method below:

| Case | Node | main | This PR | main / Node | PR / Node | PR / main |
| --- | --- | --- | --- | --- | --- | --- |
| http, 4 x `res.write(16 KB)` | 17,735 | 6,983 | 19,160 | 0.39x | 1.08x
| 2.74x |
| https, 4 x `res.write(16 KB)` | 11,720 | 6,110 | 15,510 | 0.52x |
1.32x | 2.54x |
| http, 40 x `res.write(2 KB)` | 9,879 | 6,140 | 14,980 | 0.62x | 1.52x
| 2.44x |
| https, 40 x `res.write(2 KB)` | 6,981 | 5,541 | 11,780 | 0.79x | 1.69x
| 2.13x |
| http, `res.end(64 KB)` | 18,535 | 16,528 | 19,889 | 0.89x | 1.07x |
1.20x |
| http, 256 KB file `.pipe(res)` | 2,684 | 2,375 | 2,719 | 0.88x | 1.01x
| 1.15x |
| https, `res.end(64 KB)` | 11,894 | 14,586 | 16,426 | 1.23x | 1.38x |
1.13x |
| http, GET hello (control) | 55,994 | 70,989 | 71,958 | 1.27x | 1.29x |
1.01x |

main was slower than Node in six of these eight cases. This PR is faster
than Node in all eight.

`Bun.serve`, measured on `a771572a8d`, before the larger cork buffer
(req/s, the mean of 2 rounds):

| Case | main | PR | Change |
| --- | --- | --- | --- |
| Direct stream, 4 x 16 KB | 6,826 | 12,479 | +83% |
| 64 KB string | 16,944 | 20,145 | +19% |
| TLS, 64 KB string | 15,045 | 16,892 | +12% |
| hello (control) | 83,957 | 83,137 | -1.0% |

These runs are on loopback, where the kernel send buffer is 2.6 MB and
the work of the receiver runs inside `send()`. That is the best case for
fewer writes. A new connection over a real network takes about 46 KB in
its first write on Linux. The rest waits in the socket buffer, as it
would after separate writes.

#### Small responses are unchanged

A small response is already one `recvfrom` and one `sendto` on both
builds. `perf` puts 66% of the time of a hello-world server in the
kernel, on both builds.

CI release builds on Linux x64: main `97246d044e` (the merge base)
against this PR `8834cd0787`. Both use the same WebKit. The server runs
on one pinned core. `oha` sends 64 connections for 5 s after a 2 s
warm-up. There are 2 rounds, and the order of the builds alternates.
"Change" compares the means of the two rounds.

Framework servers from `bun-perf-tester` (req/s):

| Server | main, round 1 | main, round 2 | PR, round 1 | PR, round 2 |
Change |
| --- | --- | --- | --- | --- | --- |
| express | 49,803 | 51,103 | 49,968 | 50,263 | -0.7% |
| fastify | 61,214 | 61,105 | 60,705 | 60,668 | -0.8% |
| node:http | 70,934 | 71,405 | 73,178 | 71,053 | +1.3% |
| elysia | 84,696 | 85,096 | 85,027 | 84,882 | +0.1% |
| `Bun.serve` | 89,020 | 89,099 | 88,168 | 88,373 | -0.9% |

node:http paths that this PR changes (req/s):

| Case | main, round 1 | main, round 2 | PR, round 1 | PR, round 2 |
Change |
| --- | --- | --- | --- | --- | --- |
| GET hello | 70,226 | 70,341 | 70,151 | 72,359 | +1.4% |
| POST, 16 KB body | 47,446 | 47,789 | 48,191 | 48,785 | +1.8% |
| 64 KB response in four writes | 6,885 | 6,894 | 6,834 | 6,868 | -0.6%
|
| Pipelined keep-alive, depth 8 | 94,063 | 93,294 | 92,909 | 93,809 |
-0.3% |

p99 latency (ms), the higher of the two rounds:

| Server | main | PR |
| --- | --- | --- |
| express | 1.94 | 1.91 |
| fastify | 1.52 | 1.55 |
| node:http | 1.11 | 1.12 |
| elysia | 0.98 | 0.96 |
| `Bun.serve` | 0.82 | 0.83 |

RSS (MB), one pass of 8 s of load:

| Server | Build | Start | Under load | 5 s idle | 15 s idle |
| --- | --- | --- | --- | --- | --- |
| express | main | 39 | 94 | 61 | 57 |
| express | PR | 40 | 92 | 60 | 57 |
| fastify | main | 41 | 91 | 58 | 55 |
| fastify | PR | 41 | 91 | 58 | 55 |
| node:http | main | 20 | 64 | 43 | 40 |
| node:http | PR | 20 | 65 | 45 | 41 |
| elysia | main | 28 | 46 | 36 | 35 |
| elysia | PR | 29 | 46 | 37 | 36 |
| `Bun.serve` | main | 14 | 30 | 22 | 22 |
| `Bun.serve` | PR | 14 | 30 | 22 | 22 |

Every change is within 2%. fastify and `Bun.serve` hello are lower in
both rounds, by about 1%. `Bun.serve` hello shows the same -1.0% in the
control row above, so a small real cost there is possible. The RSS pass
ran at the same time as the throughput runs, on other cores. The commits
after `8834cd0787` change tests and add one version check to the
node:http dispatcher. They were not measured.

### Supersedes

| Theme | Pull requests |
| --- | --- |
| Request body | #43592 #43579 #38196 #43518 #43602 #43408 #43427 #43597
#43555 #43456 #43466 |
| Tunnels | #43570 #43485. #43596 is a duplicate of #43570. |
| Parser | #43182 #43161 #43326 #43327 #40505 #43363 #42532 #42194 |
| Response write | #39386 #43371 #43548 #43499 #43496 #43464 #42008
#43549 |
| Response finish | #40351 #43021 #41822 #43473 #42068 #35207 #43425
#43503 |
| Lifecycle | #43413 #39686 #43028 #42727 #42622 #42610 #35837 #35839
#37825 #37749 #43376 #35268 |
| JS API | #41691 #41738 #38036 #42462 #36527 #39718 #37964. #42947
merged on its own. |

The close drain, `resetAndDestroy()`, the pending write callback
handling and the response `'close'` ordering come from #42622 and #42727
by @steipete. The diagnosis and the tests for the stalled `ws` writes
come from his #42610.

Not included:

| Pull request | Reason |
| --- | --- |
| #33061 | main already enforces `headersTimeout` and `requestTimeout` |
| #41672 | It makes `http.createServer({ key, cert })` stop serving TLS.
That needs a product decision. |
| #37543 | A type refactor with no tests and no user-visible change |
| #35465 | It makes `http.Server` extend `net.Server`. Only the
prototype chains were joined. The `net.Server` constructor never ran, so
`_handle` and `_connections` were `undefined`, and
`_emitCloseIfDrained()` emitted `'close'` on a listening server. The
server is backed by uWS, not `node:net`. |
| The `AutoFlusher` removal in #42622 | It makes `flushHeaders()` flush
at once. That is a performance change with no relation to the rest. |

### Tests

| Check | Result on a debug build (macOS arm64) | Head |
| --- | --- | --- |
| Every test file that this PR touches (28 files) | 1,866 pass, 2 fail.
The 2 failures are `serve.test.ts` "bounds memory when proxying ... to a
stalled client". They fail the same way on a debug build of main. |
`83af4da4a3`, run before the last commit of main came in |
| `test/js/third_party/express` (9 files) and the `body-parser` test |
299 pass, 0 fail | `83af4da4a3`, run before the last commit of main came
in |
| Node 25.6 against Bun, 32 scenarios from two scripts (event order,
framing, lifecycle) | No regression against Bun 1.4.3 | `0ff1a23f61` |
| Every vendored Node `test-http-*` and `test-https-*` file, plus the
`test-net-*` and `test-tls-*` files for pause, write, end and close |
535 of 537 exit 0. `test-http-agent-keepalive.js` and
`test-https-timeout.js` fail on that debug build. Both pass on every CI
lane. | `daee05fcfd` (before the rebase) |
| The tests that depend on what the kernel takes in one send, on Windows
Server 2019 x64 and Windows 11 arm64 | pass | `3eef223328` (x64),
`a29289bcc1` (arm64) |
| CI build 120191 (Linux, macOS and Windows, release and ASAN) | every
lane passed | `daee05fcfd` (before the rebase) |

Each new test fails on Bun 1.4.3, or on the commit before its fix for a
fault that this branch introduced.

The two tests over the limit are `node-http-connect.test.ts` ("tests
should run on bun") and `node-http-syscall-fault.test.ts` ("racing a
queued drain"). Each starts a debug subprocess that needs more than 5 s
on this machine. Both pass on CI.

### Changes in the last push

The branch is rebased on main (`daee05fcfd` was the head before). It is
now linear.

Four regressions against main, each with a test that fails without its
fix:

| Case | main | Before this push | Now (same as Node) |
| --- | --- | --- | --- |
| `emit('connection')` or http2 `allowHTTP1`: `res.end()` on a request
that nobody reads | `'end'`, `'close'` | No events | `'end'`, `'close'`
|
| The same server, an unread 32 MB body | 0 bytes held | 32 MB held | 0
bytes held |
| A NUL in a header value with `httpValidation: "relaxed"` (client,
`HTTPParser`, `emit('connection')` server) | Accepted | The process
spins forever | `HPE_INVALID_HEADER_TOKEN` |
| `Connection: close`, body in the same read as the head, a 20 KB
response before the body is read | `'end'` with an empty body | No
events on `req` | `'end'` with the body, `'close'` |
| An empty line on an idle keep-alive connection, then `server.close()`
| 0 s | About 6 s | 0 s |

| Fix | Where |
| --- | --- |
| The finish listener of a fallback connection dumps an unread request,
like Node's `resOnFinish` | `http1_server_fallback.ts` |
| llhttp patch: `llhttp__internal__c_test_lenient_flags_20` is false for
a NUL. The relaxed state does not consume a NUL, and the next state sent
it back there. 9.4.3 has the same loop. | `llhttp.c`, noted in its
`README.md` |
| A node:http socket that `onData` is parsing gets the close gate of
`onData`, also when a large write released the cork | `HttpResponse.h`
`uncorkCompletedResponse()` |
| A read that starts no message leaves an idle connection idle |
`HttpContext.h` `onData` |

The open review threads are fixed in `8834cd0787`:
`closeAllConnections()`, `Proxy-Connection: close`, five comments cut to
one line, and the test of two overlapping listeners, which now waits on
events. With the generation gate in `emitCloseServer` removed, that test
fails in both cases. `AsyncSocketData` keeps its bools together, which
takes it from 56 to 48 bytes per socket.

`http.Server` no longer extends `net.Server` (see "Not included"). The
special case for it in `Ipc.ts` is gone too. `child.send(msg,
httpServer)` still throws `ERR_INVALID_HANDLE_TYPE`, and its test stays.

<details><summary>Changes since the first revision (2988a61)</summary>

Merged with main at `c8e1f6fa5b`. The one conflict was #43708
(`req.socket` emits `'end'` and `'error'`). Its state bit
`HTTP_NODE_PEER_ENDED` moved to bit 22, because bit 19 is
`HTTP_NODE_NOTIFY_READ_PARSED` here. Its 15 tests run in
`node-http-server-abort-events.test.ts` next to the tests of this branch
(103 pass).

CI on `2988a610c` had ten red tests from four causes. They are fixed:
- `ed882e5e99`: `write()` to a response without a body (HEAD, 204) does
not wait for unsent bytes.
- `811f817704`: an idle tunnel does not hold the event loop after
`server.close()`. Four vendored Node tests timed out on every platform.
- `0697deec11`, `d428824c08`, `7aec062d14`: the write callback tests use
a body that backs up a loopback socket, and accept what Winsock does.
- `c7af1c1615`: two tests from main asserted the old `close()` contract.

Review findings, each reproduced against Node v26.3.0 and fixed with a
test that fails without the fix:
- `d4d2b783ea`, `2cc79939bb`, `45141abca9`, `9a63dbc470`, `80a23a1822`:
the idle rule. A keep-alive connection is idle again when its body ends
after its response. A connection that owes a queued pipelined response,
that still receives a request head or body, or whose response still
drains is not idle.
- `87d8904aa3`, `7eca4f7806`: `close(cb)` followed by `listen()` in the
same tick reports `'close'`, also for an https server whose only
connection was idle.
- `3fd7b25382`: a paused pipelined request behind a response that still
drains stops the connection. The first revision read 512 MiB of 512 MiB
into memory.
- `2d1a5e2d8d`, `33e4683a8a`, `539cb41eb9`, `197c7dfc29`, `0490e3540b`:
raw socket writes stay behind every unsent response byte. The cases were
a CONNECT pipelined behind a response that still drains (its `200`
landed at offset 2.6 MB of a 64 MiB body), a zero-length tunnel write
(it hung the tunnel), the 400 replies above, the zero-copy tail of a
large `res.write()`, the cork buffer, and Windows 11, where the kernel
takes the whole response and refuses the next send.
- `0abd39c133`: an upgrade from the request's `'end'` listener keeps the
body bytes out of the WebSocket. The connection closed with 1006 right
after the 101.
- `5aafc61aa6`: the callback of a small `res.write()` that the kernel
refuses at the uncork runs on the drain. Reproduced on Windows 11 only.
- `a29289bcc1`: a tunnel write that waits for a drain settles its
callbacks when the connection closes.

Known differences from Node that this PR leaves:
- A handler that calls `res.end()` and then `server.close()` closes its
keep-alive connection at once. Node waits for the `keepAliveTimeout`.
- A pipelined Upgrade behind a response that still drains is served as a
plain request.
- An `end()` on a tunnel with no write pending, while the response
before the CONNECT still drains, closes both directions after the flush.
Node half-closes.
- A raw `req.socket.write(big)` and `req.socket.end()` with no
`res.end()` sends every byte but no FIN. main loses bytes here.
- A CONNECT socket that is given back with `server.emit('connection',
socket)` answers only the first of several pipelined requests. main
answers none.
- A large write from an `'upgrade'` listener stalls while the body of
that Upgrade request is still pending. A second `listen()` on a
listening server does not throw. Both are the same on main.
- A tunnel write that fails because the client went away fails its
callbacks but emits no `'error'`. Node emits `ECONNRESET`. A new
`'error'` could end a process that has no listener for it.

</details>

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

---

**no test proof** · iteration 8 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/web/fetch/fetch.stream.test.ts, test/js/node/url/url.test.ts,
test/js/node/tls/tls-syscall-fault.test.ts,
test/js/node/net/node-net-server.test.ts,
test/js/node/http/node-http.test.ts,
test/js/node/http/node-http-syscall-fault.test.ts,
test/js/node/http/node-http-server-close-drain.test.ts,
test/js/node/http/node-http-connect.test.ts,
test/js/node/http/node-http-backpressure.test.ts,
test/js/node/child_process/child_process_ipc_handle.test.ts,
test/js/bun/http/serve.test.ts,
test/js/bun/http/serve-syscall-fault.test.ts,
test/js/bun/http/bun-server.test.ts

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

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Superseded by #43557, which is merged (5d5f03f). It fixes this once for the whole node:http server and carries the tests over.

@robobun

robobun commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

Confirmed on a debug build of main at 5d5f03f. The repro from this PR prints the same output as Node v26.3.0, and the four tests that #43557 carried over pass. Nothing from this branch is still needed.

Two related points, checked on the same build:

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.

2 participants