Skip to content

Fix crash when calling text() on consumed ReadableStream body#28740

Open
robobun wants to merge 2 commits intomainfrom
farm/84ddf005/fix-consumed-body-stream-crash
Open

Fix crash when calling text() on consumed ReadableStream body#28740
robobun wants to merge 2 commits intomainfrom
farm/84ddf005/fix-consumed-body-stream-crash

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented Apr 1, 2026

What: ByteBlobLoader.toBufferedValue returned .zero (null JSValue) without throwing a JS exception when the blob store had already been detached, violating the host function contract and triggering an assertion failure.

How to reproduce:

const resp = new Response("Hello");
const body = resp.body;
resp.bytes();
body.text(); // crash: "Expected an exception to be thrown"

When Response.bytes() is called, it detaches the underlying blob store from the ByteBlobLoader. A subsequent body.text() call on the already-obtained ReadableStream reference reaches toBufferedValue where toAnyBlob() returns null. The function then returned .zero — but the toJSHostCall wrapper asserts that .zero means a JS exception was set, which it wasn't.

Fix: Throw a proper JS error ("Body already consumed") instead of returning .zero, matching the pattern used by ByteStream.toBufferedValue for similar error cases.


Crash fingerprint: 8528c32f3d75dc9f

ByteBlobLoader.toBufferedValue returned .zero without throwing a JS
exception when the blob store was already detached (e.g. after calling
Response.bytes() while holding a reference to Response.body). This
violated the host function contract that .zero means an exception was
set, triggering an assertion failure.

Throw a proper JS error instead.
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

@github-actions github-actions bot added the claude label Apr 1, 2026
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Apr 1, 2026

Updated 1:27 AM PT - Apr 1st, 2026

@autofix-ci[bot], your commit 3eca4ae has 4 failures in Build #43083 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 28740

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

bun-28740 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: db5623a5-edca-44fa-9021-5e31cbf3da41

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed4186 and 3eca4ae.

📒 Files selected for processing (2)
  • src/bun.js/webcore/ByteBlobLoader.zig
  • test/js/web/streams/body-already-consumed.test.ts

Walkthrough

Modifies ByteBlobLoader.zig to throw a JavaScript error "Body already consumed" when attempting to access an already-consumed Response body, replacing previous behavior of returning an empty value. Adds a test verifying this error is thrown correctly.

Changes

Cohort / File(s) Summary
Body Consumption Error Handling
src/bun.js/webcore/ByteBlobLoader.zig
Modified toBufferedValue() to throw JavaScript error "Body already consumed" via explicit error path instead of returning .zero when body has been consumed.
Body Consumption Test
test/js/web/streams/body-already-consumed.test.ts
New test verifying that calling body.text() on a consumed Response stream throws error with message "Body already consumed".
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main fix: preventing a crash when calling text() on a consumed ReadableStream body.
Description check ✅ Passed The description covers both required template sections: clearly explains what the PR does (the bug and fix) and how to reproduce/verify the issue with a concrete code example.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

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.

1 participant