[robobun]

Part of #43455 (flow 9).

Problem

• A node:http request that the stream destroyer destroyed keeps req.complete === false after its whole body arrived. Node v26.3.0 reports true. A for await loop that breaks and a failed pipeline() destroy a request this way.
• The cause is IncomingMessage.prototype._destroy (src/js/node/_http_incoming.ts:371). It clears handle.ondata, and only that callback sets complete.

Fix

• _destroy leaves a callback armed when the connection outlives the request and the message is not complete. The callback drops the chunks and sets complete at the last one.
• Correct because parserOnMessageComplete in Node sets complete for a destroyed request too. A connection that closes before the body ends leaves it false.
• Verified: test/js/node/http/node-http-req-complete.test.ts, 6 new cases, 4 fail without the fix. The file also runs under Node.

Background

• The stream destroyer (src/js/internal/streams/destroy.ts) sets req.socket = null and calls destroy(). _destroy then keeps the connection for the response.
• handle.ondata is the callback that the native server calls for each chunk of a request body. A flag marks the last chunk.
• node:http: closeIdleConnections() reads req.complete, and a destroyed request sets it #44310 reads req.complete. Considered a destroyed test there: it hides the wrong value from one reader only.

Downsides

• A request that ended normally runs 1 more bytecode instruction in _destroy.
• The body of such a request now holds the event loop until it ends or the connection closes. With server.unref() in the listener, the process stays about 3 s, as in Node. main exits at once.

Notes

Repro. The listener breaks out of a for await loop over the request after the first chunk and answers. The client then sends the rest of the body and one more request.

import http from "node:http";
import net from "node:net";
import { once } from "node:events";
let first;
const server = http.createServer(async (req, res) => {
  if (req.url === "/second") return res.end(String(first.complete));
  first = req;
  for await (const chunk of req) break;
  res.end("first");
});
server.listen(0, "127.0.0.1");
await once(server, "listening");
const socket = net.connect(server.address().port, "127.0.0.1");
let received = "";
socket.on("data", chunk => (received += chunk));
socket.write("POST /first HTTP/1.1\r\nHost: x\r\nContent-Length: 10\r\n\r\nhello");
while (!received.endsWith("first")) await once(socket, "data");
socket.write("worldGET /second HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
await once(socket, "close");
console.log(received.slice(received.lastIndexOf("\r\n\r\n") + 4));
server.close();

Node v26.3.0 prints true. main bf42a52 prints false. This branch prints true.

Rows. 2 ways to destroy the request (for await break, failed pipeline()) by 2 framings (Content-Length, chunked): req.complete after the body ended is true in Node and on this branch for all 4, and false on main for all 4. While the body still arrives, and when the client closes the connection before the body ends, it is false everywhere.

What Node does. parserOnMessageComplete (_http_common.js) sets stream.complete = true and pushes the end of the stream. It does not look at destroyed. parserOnBody drops the chunks of a destroyed stream.

Why the callback tests the event. The native server also calls ondata with the last-chunk flag when the connection closes (abort event). That call must not set complete. The two stays false tests cover it.

A body larger than the stream buffer. Node stops reading such a connection when the request is destroyed and never answers a later request on it (a 4 MiB body, then a second request: Node gets no response in 8 s). Bun keeps reading, as before this change, and answers. req.complete of the destroyed request is then true.

Measurements. Release builds, Linux x64. perf and valgrind are not installed on the machine, so the instruction count for each request is missing.

What	main	this PR	How
_destroy, request that ended normally		+1 bytecode instruction (jfalse on a local)	BUN_JSC_dumpGeneratedBytecodes=1, diff of the listings
_destroy, whole function	100 instructions, 568 bytes	115 instructions, 641 bytes	same
Bundled _http_incoming.js	15,732 bytes	16,082 bytes	wc -c
.text	58,159,221 bytes	58,159,221 bytes	llvm-size -A
Process exit, server.unref() in the listener, body of the destroyed request outstanding, keepAliveTimeout 2 s	110 to 210 ms	3123 to 3251 ms, when the connection closes	3 runs each. Node: 3026 to 3029 ms

Suites. Debug build: node-http-req-complete.test.ts (33 pass). Release builds of main and of #44310, which holds this change, one after the other: see the Notes of #44310.

Observed on Linux x64 only.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 7139b794-9afe-45ce-9579-f6bce53b5a50

📥 Commits

Reviewing files that changed from the base of the PR and between bf42a52 and c53fb63.

📒 Files selected for processing (2)

• src/js/node/_http_incoming.ts
• test/js/node/http/node-http-req-complete.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

Walkthrough

When an incoming request is destroyed before its body ends, the handler can continue tracking body data if the request has no socket and remains incomplete. Tests cover when the body completes and when the client disconnects before it ends.

Changes

Request body completion

Layer / File(s)	Summary
Retain body completion after request destruction src/js/node/_http_incoming.ts, test/js/node/http/node-http-req-complete.test.ts	The handler retains body data handling for eligible destroyed requests and marks the message complete when the final chunk arrives without a response-abort event. Tests cover async-iterator and pipeline consumers, Content-Length and chunked bodies, pipelined requests, and client disconnects.

Suggested reviewers: cirospaciari

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c53fb

The change preserves request completion tracking after stream destruction without marking truncated bodies complete. No concrete merge-blocking risk remains; run normal checks before merging.

🚥 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: setting req.complete when the body of a destroyed request ends.
Description check	✅ Passed	The description explains the problem, fix, verification, behavior, tradeoffs, and test coverage. It does not use the exact template headings, but it provides the required implementation and verificati…

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[robobun]

Status

Reproduced on a release build of main bf42a52. The script in the Notes of the description prints false there. It prints true under Node v26.3.0 and on this branch.

Run: bun bd test test/js/node/http/node-http-req-complete.test.ts

This is the first of two pull requests. #44310 is based on this branch.

[claude]

Beyond the inline findings, I also checked the new callback's wiring: the (chunk, isLast, event) argument shape matches what the native handle already passes to onDataIncomingMessage in src/js/internal/http.ts, and the runtime destructure of the const enum NodeHTTPResponseAbortEvent from require("internal/http") follows the same pattern src/js/node/_http_server.ts already uses, so neither is a concern.

Extended reasoning...

The change alters IncomingMessage.prototype._destroy in src/js/node/_http_incoming.ts to keep a native ondata callback armed after the stream destroyer nulls req.socket, and adds six node:http tests; it touches no auth, crypto, or injection surface. Not approving because verified findings are being posted inline (plus one held back), including a behavioral cost on the early-abandoned-body path and a trailers discrepancy that a maintainer of the node:http server should weigh.

Findings marked 🟡 are optional suggestions and need no follow-up push.

[robobun]

Closing: #44310 carries this change now. It is one pull request against main with both changes.