streams: invoke underlyingSource.cancel() when a type:"direct" ReadableStream is cancelled

[robobun]

Cancelling a type: "direct" ReadableStream never invoked the underlying source's cancel() callback. A producer that relies on cancel() for upstream teardown never learns the consumer went away.

let called = false;
const stream = new ReadableStream({
  type: "direct",
  pull(c) { c.write("hello"); c.flush(); },
  cancel() { called = true; },
});
const r = stream.getReader();
await r.read();
await r.cancel("bye");
console.log(called); // false (should be true; default streams print true)

readableStreamCancel's ControllerKind::Direct arm called onClose(), which early-returned because readableStreamClose had already moved the stream out of Readable, and then resolved with undefined. The source callback was never reached.

This calls underlyingSource.cancel(reason) under the stream's captured async context and chains its completion into the cancel promise, matching the default/byte controller behaviour. It also:

• handles stream.cancel() on a not-yet-materialized direct stream (ControllerKind::None with a pending direct source), and
• sets m_closed so controller.write() throws after cancellation instead of silently buffering.

The previously test.todo AsyncLocalStorage "readable stream direct .cancel" case is now enabled.

---

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/web/streams/streams.test.js

Fixes #18315

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3f504df8-2e07-4032-a2d1-72cd2d018208

📥 Commits

Reviewing files that changed from the base of the PR and between b18d5df and 87d2e5d.

📒 Files selected for processing (3)

• src/jsc/bindings/webcore/streams/ReadableStreamOperations.cpp
• test/js/node/async_hooks/AsyncLocalStorage.test.ts
• test/js/web/streams/streams.test.js

---

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

[robobun]

Reproduced with:

bun bd test test/js/web/streams/streams.test.js -t "canceling a direct stream invokes"
bun bd test test/js/node/async_hooks/AsyncLocalStorage.test.ts -t "readable stream direct .cancel"

Both fail on main (the first on the expect(reason).toBe("bye") assertion, the second by timing out waiting for the cancel callback to fire) and pass with this change.

[robobun]

^{Updated 5:16 PM PT - Jul 20th, 2026}

✅ @robobun, your commit 87d2e5de143defd11377c3ae16e3c55d27a53ac3 passed in Build #76444! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 34854

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

bun-34854 --bun

[github-actions]

Found 3 issues this PR may fix:

1. cancel callback of "direct" readable stream doesn't get called #18315 - Exact bug: cancel callback of type: "direct" ReadableStream is never called when the stream is cancelled
2. ReadableStream with new Response produces cancel reason: undefined #17175 - Direct stream cancel fires spuriously with reason: undefined after controller.close(); the reworked cancel routing and m_closed flag address this
3. Bun crashed while trying to run an sse example #18654 - SSE crash in JSReadableHTTPResponseSinkController::destroy after cancel; proper cancellation lifecycle (setting m_closed, preventing writes after cancel) addresses the use-after-cancel state

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

Fixes #18315
Fixes #17175
Fixes #18654

🤖 Generated with Claude Code

[robobun]

Of the three suggested issues, only #18315 is addressed here (verified its repro now prints cancelled: true).

#17175 and #18654 are on the Bun.serve native-sink path (JSReadableHTTPResponseSinkController / readDirectStreamCloseImpl), not the JS-consumption JSDirectStreamController this PR touches, so they are left open.

[claude]

Beyond the inline nit, I also checked: (1) GC safety of underlyingSource after m_directUnderlyingSource.clear() in the ControllerKind::None arm — it's stack-rooted and the pattern matches fromIterableCancelAlgorithm; (2) whether dropping the onClose() call loses sink teardown or the Bun-only .close() callback — it was already a dead call (early-returned on the Readable check), and setting m_closed = true now gates onClose/handleError against later double-invocation; (3) exception-scope discipline in directUnderlyingSourceCancel — the TOP_EXCEPTION_SCOPE + takeAbruptCompletion + result.isEmpty() termination path mirrors fromIterableCancelAlgorithm exactly.

Extended reasoning...

This is a ~40-line C++ change to JSC stream bindings that adds a new user-JS invocation point on the direct-stream cancel path. It follows the file's established patterns closely (StreamAsyncContextScope, TOP_EXCEPTION_SCOPE/takeAbruptCompletion, promiseResolvedWith), and the removed onClose() was provably dead. Deferring to human review given it's JSC bindings with observable semantic changes; the note above records the non-obvious concerns I traced so they aren't re-derived.

[claude]

No issues found; the previous .toThrow() nit is addressed in 87d2e5d. Deferring to a human because this reworks the ControllerKind::Direct/None cancellation lifecycle in C++ JSC bindings — worth a maintainer glance at the sink-teardown / m_deferClose interactions.

What was reviewed:

• directUnderlyingSourceCancel — exception-scope layering matches fromIterableCancelAlgorithm; VM-termination path (takeAbruptCompletion → empty) falls through to return nullptr correctly.
• Removed controller->onClose() was already a no-op (early-returns on state != Readable after readableStreamClose), so no sink-teardown regression vs. before.
• ControllerKind::None arm: clearing m_directUnderlyingSource + resetting m_bunMode leaves materializeIfNeeded() a no-op afterwards; underlyingSource stays stack-rooted across the cleared WriteBarrier.

Extended reasoning...

Overview

Adds a directUnderlyingSourceCancel helper in ReadableStreamOperations.cpp that invokes underlyingSource.cancel(reason) under the stream's captured async context and wraps the result in a promise. Wires it into readableStreamCancel for both ControllerKind::Direct (materialized) and ControllerKind::None with a pending m_directUnderlyingSource (un-materialized). Also sets controller->m_closed = true so post-cancel write() throws. Adds tests in streams.test.js covering all three sub-cases and un-todos the AsyncLocalStorage direct-cancel test.

Security risks

None. No untrusted-input parsing, auth, or network-facing surface. The user-JS callback invocation follows the same TOP_EXCEPTION_SCOPE + takeAbruptCompletion pattern as sibling algorithms.

Level of scrutiny

Moderate-to-high. This is C++ JSC bindings code in the streams cancellation lifecycle — REVIEW.md flags memory safety and "anything that can run user JS can synchronously free your state" as the most-blocked category. The helper calls a user-provided cancel() while holding raw pointers to stream/controller/underlyingSource; I verified these remain stack-rooted (conservative scan) and that m_closed/m_pendingRead are settled before the callout, but a maintainer familiar with JSDirectStreamController should confirm the removed onClose() doesn't leave the ArrayBufferSink or m_deferClose state in a shape later paths don't expect.

Other factors

• The new helper's shape (dynamic ->get() of cancel, StreamAsyncContextScope, catch-scope wrapping, promiseRejectedWith/promiseResolvedWith) mirrors performDefaultControllerCancelAlgorithm and fromIterableCancelAlgorithm in the same file.
• The removed onClose() call was demonstrably dead: it early-returns on state != Readable, and readableStreamClose runs first.
• Test coverage is good: reader.cancel with reason, stream.cancel before materialization, promise-chaining onto the source's returned promise, write-after-cancel throws the specific TypeError, and async-context propagation via the un-todo'd ALS test.
• The prior review's bare-.toThrow() nit was fixed in 87d2e5d and the thread is resolved.

[robobun]

Covered by #41757, which runs a direct stream's cancel(reason) on reader.cancel() / stream.cancel() / for await ... break through the new cancelSteps path (and directly on the source for a never-pulled stream), in the stream's async context. All of this PR's test cases pass on that branch: it already tests cancel(reason) after a read and before the first read and un-todos the same AsyncLocalStorage test, and the async-hook ordering case was added there in 46036f8. The write-after-cancel TypeError assertion no longer applies: on main a closed direct controller's write() returns 0 instead of throwing. Closing in favor of #41757.