fix(node): reject body reads after the body is consumed by pi0x · Pull Request #254 · h3js/srvx

pi0x · 2026-07-16T18:00:20Z

Fixes #247.

The bug

_request serves a null-body native Request once srvx has consumed the body. That request's body is pristine, so undici's own Body is unusable guard never fires and arrayBuffer() / bytes() / blob() / formData() read an empty body and resolve happily. Only text() / json() had explicit guards. This silently masks double-read bugs that throw on every other runtime.

The "related" item in the issue turned out to be worse than reported. Draining req.body directly left bodyUsed false, so a later read looked like a first read, reached the _request getter, and threw Response body object should not be disturbed or locked synchronously out of the handler — not a rejection, a crash:

TypeError: Response body object should not be disturbed or locked
    at extractBody (node:internal/deps/undici/undici:6912:17)
    at new Request (node:internal/deps/undici/undici:12269:48)
    at get _request (src/adapters/_node/request.ts:292:25)
    at desc.value [as arrayBuffer] (src/_inherit.ts:32:20)

The fix

Guard the remaining body methods. arrayBuffer / bytes / blob / formData reject via a shared #consumeNative helper, which also wraps the delegation in try/catch so a synchronous throw while materializing _request over a locked stream becomes a rejection. That's what fixes the handler crash above.
Check the spec's "disturbed" bit lazily via node:stream's isDisturbed() — the same primitive undici's own guards use. #isBodyUsed() consults it on the stream body handed out and latches the answer into #bodyUsed, with zero cost on the streaming path itself (no wrapper stream, no per-chunk hop). Merely touching request.body doesn't disturb it, so that stays free too. Pinned by a test.
The latch runs before _request releases the stream to the native Request. Past that point undici owns the accounting: it tees the stream on clone(), so its reads must not count against this request. Without this, req.clone() then reading the clone would wrongly poison the original — breaking the common clone-to-inspect middleware pattern. Pinned by a test.

Verification

srvx now matches native undici on every case — consumed-body reads, direct streaming, cancel, body-touch, first/second read, and clone:

case	before	after / native
`arrayBuffer()` after `text()`	`resolved(0)`	`TypeError: Body is unusable`
`bytes()` / `blob()` after `text()`	`resolved(0)`	`TypeError: Body is unusable`
`arrayBuffer()` after draining `body`	sync throw out of handler	`TypeError: Body is unusable`
`bodyUsed` after draining `body`	`false`	`true`
`bodyUsed` after `body.cancel()`	`false`	`true`
touch `body`, then `text()`	`resolved`	`resolved` (unchanged)
`clone().text()`, then `text()`	`resolved`	`resolved` (unchanged)

7 regression tests added. Lint/format clean; typecheck unchanged.

Summary by CodeRabbit

Bug Fixes
- Improved request body handling to better match standard Fetch behavior.
- bodyUsed now updates correctly when the body is disturbed or consumed.
- Request body readers (e.g., text, json, arrayBuffer, formData) now reject consistently after the body has already been used.
- Cloned requests can be read independently without unintentionally consuming the original.
Tests
- Added regression coverage for body-consumption semantics and single-read behavior.

`_request` serves a null-body native Request once srvx has consumed the body, so undici's own "Body is unusable" guard saw a pristine body and `arrayBuffer()` / `bytes()` / `blob()` / `formData()` resolved empty instead of rejecting, silently masking double-read bugs that throw on every other runtime. Only `text()` / `json()` guarded against this. Guard the remaining body methods, and reject rather than throw when materializing `_request` over a locked stream fails synchronously. Consuming `request.body` directly also never flipped `bodyUsed`, so a later read looked like a first read and threw "... disturbed or locked" synchronously out of the handler. Track the fetch spec's "disturbed" bit by wrapping the body stream. The wrapper uses `highWaterMark: 0` so it does not pull a chunk on construction and mark a body used merely because a handler touched `request.body`; `onDisturb` is inert once `_request` holds the body, since undici tees on `clone()` and a pull may then be the clone being read rather than this request's body. Co-Authored-By: Claude Opus 4.8 <[email protected]>

coderabbitai · 2026-07-16T18:00:30Z

📝 Walkthrough

Walkthrough

The Node Request adapter now derives body usage from the stream’s disturbed state, centralizes native body consumption error handling, updates request construction, and adds regression tests for repeated reads, direct draining, cancellation, cloning, and single-use methods.

Changes

Request body semantics

Layer / File(s)	Summary
Disturbance tracking and consumption guards `src/adapters/_node/request.ts`	`bodyUsed` and repeat-read checks now use `isDisturbed`; native body methods share `#consumeNative()`, and `_request` treats disturbed bodies as unusable.
Regression coverage `test/node-adapters.test.ts`	Tests cover repeated body reads, direct stream consumption, untouched bodies, cancellation, cloning, and single-read `arrayBuffer()` behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Node: arrayBuffer()/bytes()/blob()/formData() resolve empty after body is consumed instead of rejecting #247 — Directly tracks the request-body consumption behavior addressed here.

Possibly related PRs

h3js/srvx#219 — Modifies the same request-body parsing paths used by text() and json().
h3js/srvx#243 — Addresses synchronous locked or disturbed stream failures in the same adapter.
h3js/srvx#155 — Overlaps with the updated _request body construction logic.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding each stream,
One nibble marks it used, it seems.
Clone one branch, the other stays,
Read once, then no repeat plays.
Tests now hop through every maze.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: Node request body reads now reject after consumption.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/node-body-unusable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

pkg-pr-new · 2026-07-16T18:01:28Z

Open in StackBlitz

npm i https://pkg.pr.new/srvx@254

commit: 5acacd7

…wrapping the stream Replaces the trackDisturbed wrapper with node:stream's isDisturbed() (the same primitive undici's own guards use), consulted lazily and latched into #bodyUsed by #isBodyUsed(). This drops the extra ReadableStream + reader per body access, the per-chunk pull/enqueue microtask hop, and the highWaterMark: 0 read-ahead loss on streamed uploads. The latch happens before _request releases #bodyStream to the native Request, past which undici owns the accounting — so its tee reading the stream on clone() never counts against this request, which the wrapper previously needed the !this.#request guard for. Adds a cancel-path regression test: cancelling req.body disturbs it like a read (bodyUsed flips, later reads reject). Co-Authored-By: Claude Fable 5 <[email protected]>

…bility `@types/node` doesn't declare the module-level `isDisturbed` export of `node:stream`, only the identical `Readable.isDisturbed` static, so the named import fails typecheck in CI. Co-Authored-By: Claude Fable 5 <[email protected]>

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/node-adapters.test.ts (1)
854-857: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap this test in try...finally so cleanup always runs.
If fetch or the assertion throws, server.close(true) is skipped and the open server can keep the worker alive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/node-adapters.test.ts` around lines 854 - 857, Wrap the test body
containing the fetch request and response assertion in a try...finally block,
and move server.close(true) into the finally block so cleanup runs even when
fetch or the assertion fails.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/adapters/_node/request.ts`:
- Line 8: Update src/adapters/_node/request.ts at lines 8-8 and 236-242: remove
the top-level isDisturbed import, and change its usage in the request body
stream logic to call Readable.isDisturbed(this.#bodyStream) instead. No other
changes are needed.

---

Nitpick comments:
In `@test/node-adapters.test.ts`:
- Around line 854-857: Wrap the test body containing the fetch request and
response assertion in a try...finally block, and move server.close(true) into
the finally block so cleanup runs even when fetch or the assertion fails.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a3283d43-1f0d-4aa6-91f4-fdb42835e0f6

📥 Commits

Reviewing files that changed from the base of the PR and between 08bfe7e and 8c8f5a2.

📒 Files selected for processing (2)

src/adapters/_node/request.ts
test/node-adapters.test.ts

coderabbitai · 2026-07-16T22:27:42Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3e8f84a0-3bf6-4039-87d7-6c81c442ea93

📥 Commits

Reviewing files that changed from the base of the PR and between 8c8f5a2 and 5acacd7.

📒 Files selected for processing (1)

src/adapters/_node/request.ts

 _______________________________________
< CI/CD: Code Inspection/Catch Defects. >
 ---------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

📝 Walkthrough

Walkthrough

The Node Request adapter now derives body usage from the stream’s disturbed state, centralizes native body consumption error handling, updates request construction, and adds regression tests for repeated reads, direct draining, cancellation, cloning, and single-use methods.

Changes

Request body semantics

Layer / File(s)	Summary
Disturbance tracking and consumption guards `src/adapters/_node/request.ts`	`bodyUsed` and repeat-read checks now use `isDisturbed`; native body methods share `#consumeNative()`, and `_request` treats disturbed bodies as unusable.
Regression coverage `test/node-adapters.test.ts`	Tests cover repeated body reads, direct stream consumption, untouched bodies, cancellation, cloning, and single-read `arrayBuffer()` behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Node: arrayBuffer()/bytes()/blob()/formData() resolve empty after body is consumed instead of rejecting #247 — Directly tracks the request-body consumption behavior addressed here.

Possibly related PRs

h3js/srvx#219 — Modifies the same request-body parsing paths used by text() and json().
h3js/srvx#243 — Addresses synchronous locked or disturbed stream failures in the same adapter.
h3js/srvx#155 — Overlaps with the updated _request body construction logic.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding each stream,
One nibble marks it used, it seems.
Clone one branch, the other stays,
Read once, then no repeat plays.
Tests now hop through every maze.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: Node body reads now reject after the body has been consumed.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/node-body-unusable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai · 2026-07-16T22:32:16Z

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

The Node Request adapter now derives body usage from the stream’s disturbed state, centralizes native body consumption error handling, updates request construction, and adds regression tests for repeated reads, direct draining, cancellation, cloning, and single-use methods.

Changes

Request body semantics

Layer / File(s)	Summary
Disturbance tracking and consumption guards `src/adapters/_node/request.ts`	`bodyUsed` and repeat-read checks now use `isDisturbed`; native body methods share `#consumeNative()`, and `_request` treats disturbed bodies as unusable.
Regression coverage `test/node-adapters.test.ts`	Tests cover repeated body reads, direct stream consumption, untouched bodies, cancellation, cloning, and single-read `arrayBuffer()` behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Node: arrayBuffer()/bytes()/blob()/formData() resolve empty after body is consumed instead of rejecting #247 — Directly tracks the request-body consumption behavior addressed here.

Possibly related PRs

h3js/srvx#219 — Modifies the same request-body parsing paths used by text() and json().
h3js/srvx#243 — Addresses synchronous locked or disturbed stream failures in the same adapter.
h3js/srvx#155 — Overlaps with the updated _request body construction logic.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding each stream,
One nibble marks it used, it seems.
Clone one branch, the other stays,
Read once, then no repeat plays.
Tests now hop through every maze.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: Node body reads now reject after the body has been consumed.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/node-body-unusable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai · 2026-07-16T22:33:23Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3e8f84a0-3bf6-4039-87d7-6c81c442ea93

📥 Commits

Reviewing files that changed from the base of the PR and between 8c8f5a2 and 5acacd7.

📒 Files selected for processing (1)

src/adapters/_node/request.ts

 ____________________________________________________________________________________________________________________________________________________
< There are no final decisions. No decision is cast in stone. Instead, consider each as being written in the sand at the beach, and plan for change. >
 ----------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

📝 Walkthrough

Walkthrough

The Node Request adapter now derives body usage from the stream’s disturbed state, centralizes native body consumption error handling, updates request construction, and adds regression tests for repeated reads, direct draining, cancellation, cloning, and single-use methods.

Changes

Request body semantics

Layer / File(s)	Summary
Disturbance tracking and consumption guards `src/adapters/_node/request.ts`	`bodyUsed` and repeat-read checks now use `isDisturbed`; native body methods share `#consumeNative()`, and `_request` treats disturbed bodies as unusable.
Regression coverage `test/node-adapters.test.ts`	Tests cover repeated body reads, direct stream consumption, untouched bodies, cancellation, cloning, and single-read `arrayBuffer()` behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Node: arrayBuffer()/bytes()/blob()/formData() resolve empty after body is consumed instead of rejecting #247 — Directly tracks the request-body consumption behavior addressed here.

Possibly related PRs

h3js/srvx#219 — Modifies the same request-body parsing paths used by text() and json().
h3js/srvx#243 — Addresses synchronous locked or disturbed stream failures in the same adapter.
h3js/srvx#155 — Overlaps with the updated _request body construction logic.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding each stream,
One nibble marks it used, it seems.
Clone one branch, the other stays,
Read once, then no repeat plays.
Tests now hop through every maze.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: Node body reads now reject after the body has been consumed.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/node-body-unusable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

pi0x requested a review from pi0 as a code owner July 16, 2026 18:00

pi0 and others added 3 commits July 17, 2026 00:12

Merge branch 'main' into fix/node-body-unusable

08bfe7e

coderabbitai Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/adapters/_node/request.ts Outdated

pi0 approved these changes Jul 16, 2026

View reviewed changes

pi0 approved these changes Jul 17, 2026

View reviewed changes

pi0 merged commit 61518fc into main Jul 17, 2026
54 of 87 checks passed

pi0 deleted the fix/node-body-unusable branch July 17, 2026 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(node): reject body reads after the body is consumed#254

fix(node): reject body reads after the body is consumed#254
pi0 merged 4 commits into
mainfrom
fix/node-body-unusable

pi0x commented Jul 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

pkg-pr-new Bot commented Jul 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Review failed

Walkthrough

Changes

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

pi0x commented Jul 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug

The fix

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

pkg-pr-new Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Review failed

Walkthrough

Changes

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pi0x commented Jul 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

pkg-pr-new Bot commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading