[robobun]

What does this PR do?

Fixes four bugs in type: "direct" ReadableStreams and gives every whole-body consumer the same rule for when such a stream ends.

Fixes #17175
Closes #41917

Bugs fixed

1. controller.close() sent Content-Length: 0.
A pull() that wrote and called close() in the same tick answered 200 with an empty body. HTTPServerWritable::end() left the buffered tail to the auto-flusher, but do_render_stream frees the sink as soon as a sync pull() returns.
Fix: end() (what controller.close() reaches) now takes the end_from_js() path. It flushes and ends the response, or parks pending_flush under backpressure. close(error) goes to fail(), which drops what is buffered and finishes the sink without ending the response; the owner then closes it as incomplete.

2. The chunked last-chunk was written twice.
When finalize() sent a parked tail, res.end(tail) wrote 0\r\n\r\n without recording ended_response, so end_stream() wrote it again. The next keep-alive response then fails to parse (HPE_INVALID_CONSTANT).
Fix: ended_response is set in one place, mark_response_ended(), at the point where uWS end()/try_end() completes. The other paths assert it.

3. cancel(undefined) ran after every completed response (#17175).
The sink's onClose callback fires both when the source closes itself and when the peer goes away, and the handler could not tell them apart.
Fix: onClose carries sinkClosed, true only from JSSinkController__onClose. directStreamOnClose calls the source's cancel(reason) only then. HTTPServerWritable::abort passes CommonAbortReason::ConnectionClosed as the reason.

4. An async pull() that resolved without close() ended differently per consumer.
Bun.serve finished the response. Bun.write(file, new Response(stream)) and Bun.spawn({ stdin }) closed the fd and dropped the unflushed tail. .text() / .bytes() / Bun.readableStreamTo*() never settled. Bun.write also freed the FileSink with its controller still attached, a use-after-free at the next GC.
Fix: one rule, below. FileSink ends with the flushing end(), and Bun.write pipes JS streams through FileSink::pipe_stream, which detaches the controller before the sink is freed.

The rule

A consumer that takes the whole body (Bun.serve, fetch request bodies, Bun.write, Bun.spawn stdin, .text() / .bytes() / Bun.readableStreamTo*()) calls pull() once.

• If it returns a promise, the stream stays open while the promise is pending, ends when it resolves, and errors when it rejects.
• If it returns synchronously without closing, the stream stays open until close() / end().
• controller.close(error) fails the stream. Falsy arguments (undefined, 0, "") are a clean close. end(x) is always a clean close.

readDirectStream (native sinks) and the one-shot consumers (.text() etc., via m_closeOnPullSettled) follow it.

The JS reader path (getReader(), for await, pipeTo(), tee()) is unchanged: pull() is a demand signal there, called again for a later read once the previous call has settled (#33782). A reader has a demand signal, a whole-body consumer does not.

Behavior changes

Change	Before
An async pull() that resolves without close() ends the stream on every whole-body consumer	Bun.serve already did. .text() etc. hung; Bun.write truncated or crashed
controller.close() flushes buffered bytes on the HTTP sink	Sync pull() + close() sent Content-Length: 0
controller.close(error) fails the stream everywhere: readers reject, Bun.serve aborts the response with no chunked terminator, fetch() with such a request body rejects	The reader path treated it as a clean close
cancel(reason) runs only when the consumer goes away. On a client disconnect the reason is the connection-closed AbortError	Also ran after the source's own close() / end(), with undefined
On a native sink, controller.write() / flush() return 0 after the client, file, or child process went away	Threw "This HTTPResponseSink has already been closed…"
A pull() that rejects after the stream already closed cleanly, or after the peer left, is ignored	.bytes() and fetch() request bodies surfaced it as a late rejection

Not changed

• Bun.serve error reporting for a body that fails after the status line is committed. error() stays bypassed (Bunch of streams fixes #4251), the failure prints to stderr, and the production async path stays quiet.
• controller.write() / flush() on a native sink still throw the "already been closed… once async pull() returns" message after the source closed the stream itself. That message is what catches a pull() that forgot to await.
• The JS reader path still calls pull() again per read (never while a previous call is pending). write() after close returns 0 there, as before.
• Standalone Bun.ArrayBufferSink and Bun.file().writer() still throw after end().
• Async generator and Response bodies. New tests pin the existing behavior.

Refactors in this PR (no behavior change intended)

• JSReadableSinkControllerBase holds the source cell and the close promise itself. detach(), JSSinkController__onClose, and JSSinkController__onReady call Bun::WebStreams::sinkControllerOnClose / OnReady directly, and end() dispatches on SinkID. The JSDirectSinkCloseState cell and three JSBoundFunction handlers are deleted. Every consumer of those handlers was C++.
• Native stream sources (Blob / File / Bytes) notify their C++ adapter through Bun__NativeStreamSourceAdapter__onClose instead of a bound function stored through a JS setter. handle.onDrain had no caller and is deleted, along with close_handler / close_ctx.
• Blob.rs FileStreamWrapper is deleted. FileSink::pipe_stream covers that path.

How did you verify your code works?

• test/js/web/streams/direct-stream-contract.ts is a source-shapes × consumers matrix. Each cell asserts that pull() ran once, the bytes delivered, the error, and that cancel() ran only on a consumer abort.
  • Shapes whose pull() returns or resolves without closing are marked oneShotOnly and skip the reader consumers, which would call pull() again.
  • Consumers in streams.test.js: .text() / .bytes() / .blob(), readableStreamTo*, getReader(), for await, pipeTo, pipeThrough, tee, Bun.write, Bun.spawn stdin.
  • Consumers in serve-direct-readable-stream.test.ts: Bun.serve HTTP and HTTPS bodies, fetch request bodies.
• serve-direct-readable-stream.test.ts also covers the close() / end() matrix over HTTP, HTTPS, and HTTP/3, the last-chunk orderings, close() under transport backpressure, keep-alive after close(), and client-abort races.
• On 1.4.3, 30 cases in serve-direct-readable-stream.test.ts fail: 6 sync close() cases over HTTP and HTTPS, 2 over HTTP/3, 7 last-chunk orderings, 4 backpressure cases, 8 cancel() cases, the keep-alive close() test, and 2 stop(true) after end() expectations.
• bun-write.test.js and spawn-stdin-readable-stream.test.ts cover the FileSink truncation and the GC-after-free case.
• Run locally on the debug ASAN build: streams.test.js, bun-write.test.js, spawn-stdin-readable-stream.test.ts. The Bun.serve suites run in CI.

Notes

Folded PRs

• Bun.serve: send the buffered tail when a direct stream controller is close()d #37696: close() with a buffered tail sends it at once. Same change as bug 1. Its HTTP / HTTPS matrix and two HTTP/3 cases are in the test file. Closed in favor of this PR.
• Bun.serve: write one chunked last-chunk when a direct stream close()s with a buffered tail after streaming began #41893: set ended_response where the uWS end happens. Same as bug 2. Its seven orderings are in the test file, plus a close(error) ordering that pins only the framing. Closed in favor of this PR.

Two existing expectations change

• server.stop(true) from inside pull() after end() no longer records a cancel() event.
• The closeThenPark fixture yields once before close(), because a synchronous close() now completes the request inside the dispatch, as a synchronous end() already did.

Follow-on from that

When a synchronous close() inside an async pull() completes the response before pull() first yields, do_render_stream takes its has_responded() path and releases the request while pull() keeps running. end() already behaved this way. A pull() that throws after that point is an unhandled rejection. Holding the request until pull() settles would keep the context alive for as long as pull() runs, so it is left out.

Implementation notes

• fail() never sends the buffered tail. The previous end() body parked it with requested_end set, and the auto-flusher (which do_render_stream's drain_microtasks() runs) then sent it with res.end(), completing a failed body. A sync pull(c){ c.write(x); c.close(err) } hit that.
• A sync pull() that calls close(error) makes readDirectStream return a promise rejected with the error (marked handled: every caller is a native owner that reads its state), so owners take the same reject path as the async shape.
• JsSinkType::end is reached only from js_close with an empty reason, so its err argument was always None here. global_this is set when do_render_stream constructs the sink, before assign_to_stream runs.
• A parked pending_flush from a sync close() under backpressure is picked up by do_render_stream and handle_resolve_stream, the same way a sync end() already worked over HTTP/3.
• Bun.serve: close the connection when a body stream fails before its first flush #41581, Bun.write: stop reporting a caught direct stream failure as an unhandled rejection #41721, streams: run every callback of a direct source in the stream's AsyncLocalStorage context #41561, and Direct streams read from JS: own the buffer, signal backpressure, run cancel() #41757 touch the same lines and will need a rebase on whichever lands first.

---

no test proof · iteration 3 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/streams/streams.test.js, test/js/bun/spawn/spawn-stdin-readable-stream.test.ts, test/js/bun/io/bun-write.test.js

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Changes

Direct stream lifecycle

Layer / File(s)	Summary
Sink controller contract src/codegen/generate-jssink.ts, src/jsc/bindings/webcore/streams/*	Sink controllers now store source metadata and close-promise state. Close and ready events use centralized WebStreams handlers.
Direct stream and pump flow src/jsc/bindings/webcore/streams/BunStreamSource.cpp, src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp	Direct streams end after settled asynchronous pulls. Pumps use sink-specific close and readiness paths.
Source and HTTP completion src/runtime/webcore/streams.rs	SourceHandle::abort forwards connection-closed reasons. HTTP response completion uses mark_response_ended, while fail handles errored completion.
File sink integration src/runtime/webcore/FileSink.rs, src/runtime/webcore/Blob.rs	File piping uses shared completion and error paths for native streams and JavaScript pumps.
Behavior validation test/js/bun/http/serve-direct-readable-stream.test.ts, test/js/bun/io/bun-write.test.js, test/js/bun/spawn/spawn-stdin-readable-stream.test.ts, test/js/web/streams/streams.test.js	Tests cover implicit direct-stream completion, error propagation, cleanup, HTTP framing, backpressure, keep-alive responses, and disconnect cancellation.
Stream API documentation docs/runtime/streams.mdx, packages/bun-types/bun.d.ts	Documentation describes direct-stream close, end, promise settlement, and reader pull behavior.

Suggested reviewers: jarred-sumner

Priority: ⚪ Not assessed

Severity of issue fixed: Low

Merge Risk: 🟡 Moderate · up to 4862f

A stream error during Bun.write or related piping can trigger unsafe FileSink lifetime handling and potentially crash the process. Add the local reference guard before merging.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue [#17175] by preventing cancel(undefined) after normal direct-stream completion while preserving cancellation with a connection-closed reason after client aborts.
Out of Scope Changes check	✅ Passed	The additional sink-controller, FileSink, direct-stream consumer, documentation, and regression-test changes support the direct-stream lifecycle and response-completion fixes. No unrelated changes are…
Title check	✅ Passed	The title clearly summarizes the primary direct-stream fixes: flushing on close, preventing duplicate completion, and limiting cancel() to client aborts. It is specific and related to the changes.
Description check	✅ Passed	The description includes both required template sections. It provides detailed behavior changes, implementation context, linked issues, test coverage, verification results, and known CI limitations.

---

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

[robobun]

Status: this PR now also carries #37696 and #41893, which reworked the same close()/end()/finalize() paths of HTTPServerWritable. Reproduced all three symptoms on 1.4.3, fixed, and tested on a debug ASAN build.

Reproduction, with the released binary:

// bun repro.mjs
import net from "node:net";
const ev = [];
using server = Bun.serve({ port: 0, fetch(req) {
  const path = new URL(req.url).pathname;
  if (path === "/plain") return new Response("ok");
  if (path === "/close") return new Response(new ReadableStream({ type: "direct",
    pull(c) { c.write("hello"); c.close(); }, cancel(r) { ev.push("close cancel(" + r + ")"); } }));
  return new Response(new ReadableStream({ type: "direct",
    async pull(c) { c.write("hello"); await c.flush(); await new Promise(r => setImmediate(r)); c.write("-world"); c.close(); } }));
} });
ev.push("close body=" + await (await fetch(`http://127.0.0.1:${server.port}/close`)).text());
// One keep-alive connection: the chunked /late response, then /plain.
const raw = await new Promise(resolve => {
  let data = "", second = false;
  const sock = net.connect(server.port, "127.0.0.1", () => sock.write("GET /late HTTP/1.1\r\nHost: x\r\n\r\n"));
  sock.on("data", d => { data += d; if (!second && data.includes("\r\n0\r\n\r\n")) { second = true; sock.write("GET /plain HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); } });
  sock.on("close", () => resolve(data));
});
ev.push("late wire=" + JSON.stringify(raw.slice(raw.indexOf("\r\n\r\n") + 4, raw.indexOf("HTTP/1.1 200", 10))));
await Bun.sleep(50);
console.log(ev);

Before: [ "close cancel(undefined)", "close body=", "late wire=\"5\\r\\nhello\\r\\n6\\r\\n-world\\r\\n0\\r\\n\\r\\n0\\r\\n\\r\\n\"" ]. The /close body is empty and calls cancel, and the /late response ends with two last-chunks, so the five stray bytes land in front of the next response.
After: [ "close body=hello", "late wire=\"5\\r\\nhello\\r\\n6\\r\\n-world\\r\\n0\\r\\n\\r\\n\"" ].

A client that disconnects mid-body still calls cancel, now with the AbortError DOMException that request.signal.reason carries.

Test: test/js/bun/http/serve-direct-readable-stream.test.ts. 64 pass with the change. 30 of those cases fail on the released binary.

#37696 and #41893 are now closed in favor of this PR. Their repros pass on a debug build of this branch (2123e40) and still fail on main at 85e9ddc. This PR also fixes #17175 (cancel(undefined) after a normal completion), which main still reproduces after #41757.

[robobun]

^{Updated 9:22 PM PT - Sep 8th, 2026}

✅ @Jarred-Sumner, your commit c70a379c8f599737900500cf53e65bb836fc54ce passed in Build #113432! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 41894

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

bun-41894 --bun

[claude]

I reviewed this PR and didn't find any bugs. Because it changes the user-observable cancel() contract for direct-stream response bodies, rewires HTTPServerWritable::end() onto the flushing path, and modifies two existing test expectations in a close path with UAF history and three other open PRs touching it, a human look is still worthwhile.

What was reviewed:

• The sinkClosed third argument threaded through detach() (false) vs JSSinkController__onClose (true) and consumed in readDirectStreamCloseImpl — the two call sites cover the source-initiated vs sink-initiated split as described.
• HTTPServerWritable::end() now delegating to end_from_js via self.global_this.expect(...): traced the only remaining end() caller to js_close with an empty reason (Sink.rs:667), which runs after start() has set global_this; the previous default close_with_error caller is now overridden to fail().
• SourceHandle::abort() mirrors close()'s exception guard and JSController dispatch; non-JSController variants fall through to close(None) unchanged.
• The two edited existing assertions (dropped "cancel()" event, added microtask yield in closeThenPark) follow directly from the new semantics rather than weakening coverage.

Extended reasoning...

Overview

This PR changes when a type: "direct" ReadableStream's cancel() hook fires when used as a Bun.serve response body, and makes controller.close() flush buffered bytes. It touches four files: the JSSink codegen script (adds a third sinkClosed boolean to the onClose callback protocol), BunStreamSource.cpp (gates the underlyingSource.cancel() call on sinkClosed), streams.rs (rewires HTTPServerWritable::end() to end_from_js(), adds fail() for the errored path, adds SourceHandle::abort() to pass ConnectionClosed as the cancel reason, overrides close_with_error), and the test file (adds ~280 lines of new coverage plus edits two existing assertions).

Security risks

No direct security surface. The change is in HTTP response-body streaming lifecycle — no auth, crypto, or input parsing. The area does have a documented UAF history (the test file's own comments describe an ASAN-gated regression test for end() after a settled pull()), and this PR reroutes end() onto a path that calls finalize(), res.end(), and source.close() where the old end() (with buffered bytes) only set flags. The fail() path preserves the old no-flush behavior for errored sources. The abort() addition follows the existing pattern of copying source out before calling into JS to avoid holding a borrow across a potential free.

Level of scrutiny

High. This is a user-visible behavior change to a Web-adjacent API (cancel() semantics on direct streams), it modifies two existing test expectations (one drops an asserted event, one adds a yield to keep a timing window open), and the PR description itself names three other open PRs (#41721, #41757, #34854) touching the same close path that will need coordination. The end() → end_from_js() delegation introduces an .expect() on self.global_this; I traced the remaining call site (Sink.rs js_close line 667, reached only after start() populates it) and the default close_with_error is now overridden, so it looks sound — but a maintainer who knows every entry into HTTPServerWritable::end should confirm.

Other factors

Test coverage is thorough: a six-shape completion matrix asserting cancel() is not called, a direct/default client-disconnect test asserting the AbortError reason matches request.signal.reason, and a two-round keep-alive raw-socket test with a strict chunked/content-length parser to catch stray terminators or lost bytes. The tests await conditions rather than sleeping, use port: 0, and Buffer.alloc(n, fill) per repo conventions. The bug hunt exited on dry_streak with no findings and no ruled-out candidates. Given the behavioral change, the modified existing assertions, and the cross-PR coordination the author flags, a maintainer sign-off is appropriate before merge.

[robobun]

On the .expect() in HTTPServerWritable::end(): here is the full entry set, so it is one grep to check.

JsSinkType::end has two callers, both in src/runtime/webcore/Sink.rs:

• js_close (line 667), the body of ${name}__close, when reason is the empty value (a clean controller.close()).
• the default close_with_error (line 332). HTTPServerWritable now overrides that, so it no longer reaches end().

writer.end() from JS goes to js_end (line 575), which calls end_from_js directly. SinkHandle::end is a no-op for the HttpResponse/HttpsResponse variants. ResponseStream has one construction site, do_render_stream (src/runtime/server/RequestContext.rs:2124), and it sets global_this in the struct literal before assign_to_stream runs. So every path that can reach end() runs after the field is set.

The message is the one the existing global_this() accessor already uses for the same invariant, and unregister_auto_flusher relies on it on the teardown paths.

[robobun]

CI on build 112496: no failure is new for this change.

• test/js/node/test/parallel/test-crypto-dh-leak.js on x64-asan is pre-existing (an RSS assertion, also red on main). It is reported separately.
• The other six are flaky on lanes this diff does not touch: bun-inspector-protocol, bun-install-registry, bun-patch on Windows, structuredClone-classes, test-watch-mode-kill-signal-default, and serve-http2-lifecycle. Each one passed on a retry or passed alone in its batch.
• serve-http2-lifecycle.test.ts is the only one in this area, so I ran it against this build locally: 23 pass, 0 fail.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

I worked the backpressure variant of the close() tail drop in parallel (branch robobun/7549ec96/direct-stream-close-backpressure-tail, same end() takes the end_from_js() path change plus a close_with_error override and a do_render_stream tweak). This PR covers it, so I am not opening mine. Two things from that branch that may be worth folding in here:

1. Regression tests for the backpressure shape: an async pull() writes 1 MiB chunks until controller.write() returns a promise, then controller.write("tail"); controller.close() from the same tick or a later task. On 1.4.3 the chunked body (HTTP/1.1, paused raw socket) or the DATA stream (h2, raw frame client that withholds WINDOW_UPDATE) arrives terminated but 4 bytes short, because finalize()/on_auto_flush skip the send on has_backpressure && end_len > 0 and then write the terminator. All four cases pass on this branch (checked against d499cad) and fail on 1.4.3: serve-direct-readable-stream.test.ts#L1151-L1262. They only import RawH2, frame, T, baseHeaders from serve-http2-helpers.ts.

2. A behavior change to be aware of: once close() completes the response inside the dispatch, do_render_stream takes its has_responded() shortcut and releases the request while pull() is still running. A pull() that throws afterwards (c.write(x); c.close(); await later; throw e) then surfaces e as an unhandled rejection. On 1.4.3 that rejection was consumed by handle_reject_stream for close() (the request stayed pending until pull() settled), while end() already had the unhandled-rejection behavior. Checked on d499cad: both close() and end() now print the late error as unhandled. My branch gated the shortcut on "assignToStream() did not return a pending promise" (RequestContext.rs hunk in 78b31d7629) with a test for both (#L657-L706), which also keeps the Bun.serve: arm the streaming response's uWS callbacks before attaching the stream #39510 closeThenPark fixtures unchanged. Caveat from review of that approach: keeping the context while pull() runs means a pull() that awaits the request body after ending the response (c.end(); await req.text()) holds the context until that read settles, and on HTTP/1 nothing aborts it after markDone(). Detaching the request-body producer when the sink ends the response would cover that, if you take this route.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

One edge of the folded Bun.write path regressed against 1.4.3-canary (f42e98025). Checked on main at 8f33aaa with a debug build:

const rs = new ReadableStream({ type: "direct", pull(c) { c.write("x"); throw "not an Error"; } });
await Bun.write("/tmp/out.txt", new Response(rs));
// canary: rejects with "not an Error". main: never settles.

A sync pull() that throws a non-Error value (a string, a plain object) leaves the Bun.write promise pending. An Error instance, or any value thrown after an await, rejects as before. Bun.spawn({ stdin }) with the same stream hangs on both builds, so this looks like the existing assign_to_js_stream behavior that Bun.write now shares.

The rest checks out on main: the GC repro from #41895, #41897 and #41917 is clean under ASAN, the ten new bun-write.test.js cases pass, and fileSinkInternals.liveCount() stays flat across rejected stdin streams.

[robobun]

Follow-up for one path this change left open: a type: "direct" input to HTMLRewriter.transform() no longer got cancel() when the output reader cancelled, a handler threw, or the client behind a transformed Bun.serve response disconnected, because RewriterPipe only detached the controller (sinkClosed = false). #42130 closes the controller with a reason on those paths so cancel(reason) runs and later writes return 0.