[robobun]

Problem

• A failed HTMLRewriter Response is never collected when the handler's error references it and nothing reads its body. The Notes repro prints retained 50 of 50 protected Error: 50 on main.
• Cause: when nothing can take the error, ByteStream::append (src/runtime/webcore/ByteStream.rs:680) keeps it in a jsc::strong::Optional. A Strong is a GC root, and the error reaches it again through the Response.

Fix

• The Bytes source wrapper gets a visited pendingError slot (streams.classes.ts). A waiting JS error sits there, with no Strong. A native error stays native.
• Readers take it with take_pending_error(). All reach the ByteStream through its wrapper, so the slot lives as long as a reader can exist.
• Verified: test/js/workerd/html-rewriter-leak.test.ts, two leak tests fail on main (Received: 60, expected < 15). Also ran HTMLRewriter, streams, fetch, S3 suites.
• Self-reviewed: 3 concerns raised, 3 addressed.

Background

• A ByteStream is the native source of a streamed body. A producer (HTMLRewriter, fetch, S3) pushes chunks and one terminal error into it.
• Its JS wrapper is the object the ReadableStream holds. Its values in streams.classes.ts become visited WriteBarrier fields.
• Considered a per-producer rule (fetch: fail a native reader of an aborted res.body with signal.reason #43699 has one) and a weak handle. The first drops the real error. The second cannot hold a thrown string.

Downsides

• Size: ByteStream grows from 192 to 208 bytes, the debug binary by 1040. The wrapper cell stays at 80 bytes.
• Still leaks: an error that references its Response when nobody touched .body (Body: do not root the error of a failed body #43521).

Notes

Repro 1 (.body made, never read):

const { heapStats } = require("bun:jsc");
let fired = 0;
const fr = new FinalizationRegistry(() => fired++);
const N = 50;
const tick = () => new Promise(resolve => setImmediate(resolve));
async function once() {
  const holder = {};
  let controller;
  const res = new HTMLRewriter()
    .on("p", { element() { const error = new Error("boom"); error.response = holder.res; throw error; } })
    .transform(new Response(new ReadableStream({ start: c => void (controller = c) })));
  holder.res = res;
  fr.register(res, 0);
  void res.body;
  controller.enqueue(new TextEncoder().encode("<p>x</p>"));
  controller.close();
  for (let i = 0; i < 3; i++) await tick();
}
for (let i = 0; i < N; i++) await once();
for (let i = 0; i < 6; i++) { Bun.gc(true); await tick(); }
console.log("retained", N - fired, "of", N, "protected Error:", heapStats().protectedObjectTypeCounts.Error ?? 0);

• main: retained 50 of 50 protected Error: 50. This branch: retained 1 of 50 protected Error: 0.
• "Nothing can take the error" means: no native sink, no buffer action (text() and the like), no pending pull.
• Repro 2 replaces void res.body with holder.reader = res.body.getReader() and sends a few <b>chunk</b> chunks first. The first chunk fills the reader's queue, the stream stops pulling, and the error finds no pending pull. Same numbers before and after.
• Control: with a read outstanding, nothing leaks on main either. The pending pull rejects at once and the JS stream keeps the error in its own visited m_storedError.

Heap snapshot (generateHeapSnapshotForDebugging()), retainers of the waiting Error:

• main: StrongRootBlock [Internal], which is a root.
• this branch: BytesInternalReadableStreamSource [Property: pendingError], not a root. That wrapper's retainer is ReadableStream [Property: bunNativePtr].

Tests (html-rewriter-leak.test.ts, block "a handler error that nothing has read yet"):

• Two leak tests: .body made and unread, and a reader with a backlog. 60 rewrites each, then Response and protected Error counts from heapStats(). With the merge base's src/ they fail (Received: 60, expected < 15).
• Six guard tests pass on main too (a Strong cannot die). They check that the waiting error survives a full GC plus heap churn and reaches each reader intact: text(), Bun.readableStreamToBytes(), read() after the queued chunk, a second rewriter (wire_native_sink), and an S3 upload (the early check, zero requests made). One of them throws a string: the slot holds any value, where a weak handle holds only objects.

Measurements, merge base 37471e5 against this branch, debug (ASAN) builds:

• size_of::<ByteStream>() 192 to 208, byte_stream::Source 344 to 360. SysError is boxed in the enum (40 bytes inline otherwise).
• sizeof(JSBytesInternalReadableStreamSource) 72 to 80 (lldb). A JSC cell occupies a multiple of 16 bytes (MarkedBlock::atomSize), so the cell took 80 bytes before too.
• size build/debug/bun-debug: text +2640, data -1728, bss +128, total +1040 bytes. No release build was made.
• Per chunk: nothing new. on_data does not change. on_pull and to_any_blob touch the new field only after the last chunk.

The fetch abort reason (#43699, merged after this PR was opened):

• fetch: fail a native reader of an aborted res.body with signal.reason #43699 never parks a JS abort reason in the ByteStream, because of this root. A reader that already holds the source and pulls later (an idle Readable.fromWeb(res.body)) gets a generic AbortError. Its notes name this slot as the missing piece.
• This PR does not change that. It only removes the sentence "A stored reason would be a GC root." from error_native_consumer, which the slot makes false.
• A draft of this PR kept the reason in the slot. That works for a truthy reason. A falsy reason (abort(null)) thrown from the later pull closes the Readable with no 'error' event, because destroy(falsy) is a clean destroy in node:stream. node:stream: Readable.fromWeb emits an AbortError when the web stream fails with a falsy value #43732 maps a falsy pull error to an AbortError. The change can follow when both PRs are merged.
• Heap probes, 40 aborted fetches each, with a reason that references its Response: body never made, body made and never read, body made then res.text(), reader parked, late native reader, idle Readable.fromWeb() resumed, idle Readable.fromWeb() dropped. Retained counts are the same on the merge base and on this branch (40, 40, 0 to 1, 1, 40, 0, 0). The three rows with 40 are the body's Value::Error root (Body: do not root the error of a failed body #43521).

Self-review:

• The readers of the waiting error are on_pull, to_buffered_value, wire_native_sink (ReadableStream.rs) and the S3 upload check (s3/client.rs, it read pending.result directly). Each one and each release site of the old pending.result error (to_any_blob, on_cancel, finalize, the buffer action reject path) behaves as before.
• Concern 1, struct size: SysError is boxed in the enum.
• Concern 2, thrown primitives: a string, number, symbol or bigint waits in the slot and reaches the reader unchanged. One guard test throws a string.
• Concern 3, after the rebase onto fetch: fail a native reader of an aborted res.body with signal.reason #43699: the slot made one sentence in error_native_consumer false. It is gone.

Behaviour that does not change:

• to_any_blob still drops a waiting error, and on_start still reports Empty for it. ByteStream: report a stored producer error and keep the taken prefix with the buffer #38003 changes both. It edits the same readers, so one of the two needs a small rebase. Its has_pending_error() becomes a check of the new field.
• A second error still replaces the first.
• on_cancel still leaves a waiting error in place until the wrapper goes away.

Suites run on the debug build: html-rewriter-leak, html-rewriter, html-rewriter-end-error, fetch-abort-stream-body, body-mixin-errors, body-clone, fetch-stream-cancel-leak, fetch-response-finalizer-sweep, fetch-cyclic-reference, fetch-backpressure, streams, native-source-onclose-leak, web-stream-state, node-stream, s3-stream-error-gc, s3-upload-stream-gc, s3-stream-cancel-leak, and three node *from-web* files.

Not covered here:

• Body::Value::Error(ValueError::JSValue(Strong)) is a second parking spot with the same cycle. A failed rewrite whose .body nobody touched stores the handler's error there (RewriterPipe::fail, the branch without an output stream). Body: do not root the error of a failed body #43521 addresses it. This PR touches neither Body.rs nor html_rewriter.rs.
• Found in review, on main too: append(Err) clears the buffer and leaves offset as it was, so a later take_buffer can panic (range end index 65536 out of range for slice of length 0). This PR does not change that path.
• Three older tests in html-rewriter-leak.test.ts run close to the 5 s timeout on a debug (ASAN) build: "transform(new Response(ascii)) releases the input WTFStringImpl", "a worker exiting with an unobserved transform's pull queued frees the pipe" and "element.attributes iterator does not leak names/values". Each takes 2.1 to 3.5 s on an idle machine. With 24 busy loops on 16 cores, the third one timed out in 2 of 3 runs of the file. The tests this PR adds take 0.6 to 0.9 s each and passed in every such run.

---

[human-review] gate passed · iteration 1 · 4 files touched

fails on main (without fix)

ASAN without fix: 2 failed, 1 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/workerd/html-rewriter-leak.test.ts
bun test v1.4.3 (367d939d9)

test/js/workerd/html-rewriter-leak.test.ts:
(pass) transform(new Response(ascii)) releases the input WTFStringImpl [2636.30ms]
(pass) exceptions thrown from handlers do not leak protected Exception roots [756.78ms]
(pass) onEndTag callbacks are released after the rewrite [491.61ms]
(skip) HTMLRewriter does not leak element/document handler allocations
(pass) async handler reject/cancel on native-stream input releases the pipe [1482.05ms]
(pass) cancelling the output reader from inside a handler releases the pipe [873.47ms]
(pass) suspended rewrites release their parked state once the handler settles [651.82ms]
(pass) never-settling handler promises are abandoned and release their parked state [400.87ms]
(pass) never-settling handler promises on a file-backed input are abandoned [90.87ms]
(pass) a held reader's read() rejects when the handler promise is collected [40.21ms]
(pass) never-settling handler promises with a realized body release their parked state [383.66m
... (truncated)

release without fix: 1 skipped
bun test v1.4.3-canary.1 (8d5449830)

test/js/workerd/html-rewriter-leak.test.ts:
(pass) transform(new Response(ascii)) releases the input WTFStringImpl [67.04ms]
(pass) exceptions thrown from handlers do not leak protected Exception roots [15.13ms]
(pass) onEndTag callbacks are released after the rewrite [7.51ms]
(pass) HTMLRewriter does not leak element/document handler allocations [886.54ms]
(pass) async handler reject/cancel on native-stream input releases the pipe [109.70ms]
(pass) cancelling the output reader from inside a handler releases the pipe [71.59ms]
(pass) suspended rewrites release their parked state once the handler settles [185.17ms]
(pass) never-settling handler promises are abandoned and release their parked state [11.55ms]
(pass) never-settling handler promises on a file-backed input are abandoned [110.15ms]
(pass) a held reader's read() rejects when the handler promise is collected [3.15ms]
(pass) never-settling handler promises with a realized body release their parked state [11.45ms]
(skip) abandoned transforms over a never-closing JS stream survive GC sweep order
(pass) a direct-stream pull parked on flush(true) is released when the handler 
... (truncated)

passes on PR (with fix)

ASAN with fix: 1 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/workerd/html-rewriter-leak.test.ts
bun test v1.4.3 (367d939d9)

test/js/workerd/html-rewriter-leak.test.ts:
(pass) transform(new Response(ascii)) releases the input WTFStringImpl [2841.30ms]
(pass) exceptions thrown from handlers do not leak protected Exception roots [771.41ms]
(pass) onEndTag callbacks are released after the rewrite [445.04ms]
(skip) HTMLRewriter does not leak element/document handler allocations
(pass) async handler reject/cancel on native-stream input releases the pipe [955.66ms]
(pass) cancelling the output reader from inside a handler releases the pipe [867.07ms]
(pass) suspended rewrites release their parked state once the handler settles [676.08ms]
(pass) never-settling handler promises are abandoned and release their parked state [390.82ms]
(pass) never-settling handler promises on a file-backed input are abandoned [848.62ms]
(pass) a held reader's read() rejects when the handler promise is collected [43.00ms]
(pass) never-settling handler promises with a realized body release their parked state [394.12m
... (truncated)

release with fix: 1 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 6195ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/208] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (15 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSystemRouter (5 fields)
  - FrameworkFileSystemRouter (2 fields)
  - MatchedRoute (8 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Glob.classes.ts
  - Glob (5 fields)
Found 1 classes from /workspace/bun/src/runtime/api/h2.classes.ts
  - H2FrameParser (29 fields)
Found 9 classes from /workspace/bun/src/runtime/api/html_rewriter.classes.ts
  - HTMLRewriter (3 fields)
  - TextChunk (7 fields)
  - DocType (5 field
... (truncated)

diff hotspot

src/runtime/api/streams.classes.ts         |  10 +-
 src/runtime/webcore/ByteStream.rs          |  94 +++++++++++++----
 src/runtime/webcore/s3/client.rs           |  17 +--
 test/js/workerd/html-rewriter-leak.test.ts | 163 +++++++++++++++++++++++++++++
 4 files changed, 246 insertions(+), 38 deletions(-)

gate history · 2 passed · 1 rejected · iteration 1

evidence per changed file

file                                        reads  edits  tests
src/runtime/api/streams.classes.ts              1      2     27
src/runtime/webcore/ByteStream.rs               5     15     30
src/runtime/webcore/s3/client.rs                1      1     27
test/js/workerd/html-rewriter-leak.test.ts      4      3     27

[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: d8a9832f-8827-49fe-a686-3824e11557db

📥 Commits

Reviewing files that changed from the base of the PR and between 9cbc99a and 8d54498.

📒 Files selected for processing (1)

• src/runtime/webcore/ByteStream.rs

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

---

Walkthrough

ByteStream now stores terminal errors separately from its pending read result. JavaScript errors use a cached Bytes wrapper slot. Read, buffering, blob-conversion, and S3 upload paths use the separate error state. Tests cover unread HTMLRewriter output, collection behavior, and later error delivery.

Changes

ByteStream Error Handling

Layer / File(s)	Summary
Store terminal errors src/runtime/api/streams.classes.ts, src/runtime/webcore/ByteStream.rs	The Bytes source gains a pendingError slot. ByteStream stores terminal errors separately from its pending read result and caches JavaScript errors in the wrapper slot.
Retrieve and validate terminal errors src/runtime/webcore/ByteStream.rs, src/runtime/webcore/s3/client.rs, test/js/workerd/html-rewriter-leak.test.ts	Read and conversion paths retrieve stored errors through take_pending_error(). Blob conversion clears stored errors. S3 upload rejects with the retrieved error. Tests cover collection behavior and later error delivery for unread HTMLRewriter output.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 8d544

The reviewed change has no established merge-blocking behavior. The inspected producer paths create the readable wrapper before an error can be delivered.

🚥 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 main change: moving pending JavaScript errors from a Strong root into a visited slot.
Description check	✅ Passed	The description explains the problem, the fix, known limitations, and verification results. It does not use the exact template headings, but it provides the required information and is substantially c…

• 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 with the script in the PR description:

$ bun repro1.js   # .body made, never read
retained 50 of 50 protected Error: 50
$ bun repro2.js   # reader attached, backlog, never reads
retained 50 of 50 protected Error: 50

With this branch (debug build): retained 1 of 50 protected Error: 0.

Test proof: test/js/workerd/html-rewriter-leak.test.ts, block "a handler error that nothing has read yet".

• src/ of the merge base (37471e5): the two leak tests fail (Expected: < 15, Received: 60). The six guard tests pass.
• this branch: all eight pass.

CI (build 122408 on 8d54498): 180 of 181 jobs pass. The one red test is test/js/bun/spawn/spawn.test.ts ("an idle reader stopped at the highwater mark", debian 13 x64-asan). This diff does not reach it: a subprocess stdout is a FileReader source (ReadableStream::from_pipe), and the new slot exists only on the Bytes source. The same test is red in the final builds of the merged PRs #44350 (build 122284) and #44345 (build 122179).

PR: #43410

[claude]

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 the sibling ReadableStreamPtr::File arm in src/runtime/webcore/s3/client.rs (still reads pending.result directly) for the same rooted-cycle class — FileReader only ever parks native StreamError::Error values there, never a JS value, so it is not affected. The other candidate examined (a JS error dropped as PendingError::None when the wrapper is already gone at ByteStream.rs:743) was ruled out because no reader can reach the stream without the wrapper.

Extended reasoning...

Three confirmed findings are already posted inline, so this body only records what else was examined. The FileReader arm of upload_stream was left on the old pending.result replace/reset pattern; that is fine because FileReader.rs stores only StreamError::Error(sys::Error) in pending.result, which is not a GC root and cannot close a cycle through a Response. The wrapper-gone drop in set_pending_error is benign since every consumer (on_pull, to_buffered_value, wire_native_sink, the S3 check) reaches the ByteStream through its wrapper cell.

[robobun]

^{Updated 11:36 AM PT - Oct 1st, 2026}

✅ @robobun, your commit ec56c3a85869819428276cc92070e3bbc780b4a1 passed in Build #122474! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43410

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

bun-43410 --bun

[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.