PR #42573 - oven-sh/bun

Problem

With the built-in node-fetch and undici shims, res.body emits 'error' after a body method: Invalid state: ReadableStream is locked from resume(), pipe(), for await, and Cannot cancel a locked ReadableStream from destroy() (for example finally { body.destroy() }). Bun 1.4.2 and Node emit 'end', 'close'. Regression from #42116 (not released). No user reported it.
Cause: ReadableFromWeb (src/js/internal/webstreams_adapters.ts) calls stream.getReader() in the first _read() and stream.cancel() in _destroy(). Since #42116 a body method keeps the web stream locked, so both throw.

Fix

ReadableFromWeb takes a responseBody option. Both shims set it, Readable.fromWeb() does not. If the wrapper has not opened the web stream and the stream is locked, the Response took the body: _read() pushes null, _destroy() skips cancel().
The check reads stream.locked, not bodyUsed: after an aborted text(), bodyUsed is false and the stream is locked.
node-fetch: the body methods no longer read this.body first, so the five overrides are deleted. clone() drops the cached node stream, because the body moves to a new web stream.
Verified: test/js/node/http/node-fetch.test.js, test/js/first_party/undici/undici.test.ts (27 new tests, 25 fail with main's src/). Self-reviewed: 3 concerns raised, 3 addressed.

Background

Both shims hand out the body as a node Readable (ReadableFromWeb) around the web ReadableStream of a Bun Response. text(), json(), ... are the native methods and read the web stream directly.
ReadableFromWeb opens the web stream lazily, so a body method can take it while the wrapper has no reader.
Real node-fetch and undici read that node stream in text(), so it is ended afterwards.

Notes

Origin. Found during the review of #42516. That PR lists this symptom as tracked separately.

Reproduction.

import nodeFetch from "node-fetch";
await using server = Bun.serve({ port: 0, fetch: () => new Response("hello") });
const res = await nodeFetch(server.url);
const body = res.body;
await res.arrayBuffer();
body.on("error", e => console.log("error:", e.message)).on("end", () => console.log("end"));
body.resume();
// 1.4.2: end
// main:  error: Invalid state: ReadableStream is locked

Behaviour per case, both shims, checked against a 1.4.2 binary. "clean" means end, close for resume(), close for destroy(), and no chunks for for await.

case	1.4.2	main	this PR
`body` taken, then `text()` / `json()` / `arrayBuffer()` / `blob()` / `formData()` / `buffer()` / `bytes()`, then `resume()`, `destroy()` or `for await`	clean	`error`	clean
body method first, then `res.body` for the first time	clean (`bytes()` throws)	`error`	clean
`text()` aborted by the signal (or `AbortSignal.timeout()`), then `resume()` / `destroy()`	clean	`error`	clean
`formData()` rejects the content type, then `resume()` / `destroy()`	clean	`error`	clean
`json()` rejects with a `SyntaxError`, then `resume()` / `destroy()`	clean	`error`	clean
`text()` still pending, then `resume()` / `destroy()`	`error` (locked)	`error`	clean, and `text()` resolves with the full body
`body` taken, then `res.clone()`, then read `res.body`	throws (locked by `tee()`)	throws	`res.body` is a new node stream and delivers the body. The old node stream ends.
`res.body` streamed with `for await`, `on("data")`, `pipe()`	works	works	works
`destroy()` on a body that nothing read	cancels the fetch	same	same

Why stream.locked. The first version of this change asked the Response for bodyUsed. That misses every body method that rejects after it took the stream: on main bodyUsed stays false there while the stream stays locked (node reports true and locked). The lock of the stream itself is what makes getReader() and cancel() throw, so the wrapper checks that.

Why the overrides are deleted. #6983 (for #6200) made text(), json(), arrayBuffer(), blob(), formData() read this.body first, because a wrapper made after the consume crashed in the stream code of that time. Such a wrapper now ends without an error, and the test for each body method reads res.body for the first time after the method. Without the preload a plain await res.json() does not create a web stream and a node stream that nothing reads.

clone(). super.clone() moves the body of the original to a new web stream (a tee() branch, or a fresh stream over the same bytes). The cached wrapper held the old stream, so res.body taken before clone() threw ReadableStream is locked on read after it, also on 1.4.2. With the lock check it would have ended without data instead. clone() now clears the cache. Real node-fetch also replaces body with a new PassThrough in clone().

A body method that fails (abort, network error, bad JSON) leaves the node stream ended without an error. The rejected promise of the body method carries the error.

Not in this PR. On main bodyUsed is false after a body method that rejects when .body was read before it. That is native (src/runtime/webcore/Body.rs) and #35847 covers the formData() part.

Suites run on the debug build: node-fetch.test.js, undici.test.ts, node-fetch-cjs.test.js, node-fetch-primordials.test.ts, undici-primordials.test.ts, node-stream.test.js, node-http-agent-free-socket.test.ts, regression 26225, 014865, 04947, and the Node test-stream-readable-from-web-termination.js, test-readable-from-web-enqueue-then-close.js, test-whatwg-webstreams-adapters-to-*.js files.

[human-review] gate passed · iteration 2 · 5 files touched

fails on main (without fix)

ASAN without fix: 25 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/first_party/undici/undici.test.ts test/js/node/http/node-fetch.test.js
bun test v1.4.3 (6a92015fc)

test/js/first_party/undici/undici.test.ts:
(pass) undici > request > should make a GET request when passed a URL string [68.88ms]
(pass) undici > request > should error when body has already been consumed [13.68ms]
 97 |           const url = new URL(method, server.url).href;
 98 | 
 99 |           for (const [action, expected] of bodyActions) {
100 |             const { body } = await request(url);
101 |             await body[method]();
102 |             expect(await eventsUntilClose(body, () => body[action]())).toEqual([...expected]);
                                                                             ^
error: expect(received).toEqual(expected)

  [
-   "end",
+   [TypeError: Invalid state: ReadableStream is locked],
    "close",
  ]

- Expected  - 1
+ Received  + 1

      at <anonymous> (/workspace/bun/test/js/first_party/undici/undici.test.ts:102:72)
(fail) undici > request > body stream when a body method took the r
... (truncated)

release without fix: all passed
bun test v1.4.3-canary.1 (7b5b9c101)

test/js/first_party/undici/undici.test.ts:
(pass) undici > request > should make a GET request when passed a URL string [2.61ms]
(pass) undici > request > should error when body has already been consumed [0.37ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after arrayBuffer() consumed it [5.40ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after blob() consumed it [1.87ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after formData() consumed it [2.11ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after json() consumed it [2.05ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after text() consumed it [1.75ms]
(pass) undici > request > body stream when a body method took the response > resume() leaves a body method that is still reading alone [2.04ms]
(pass) undici > request > body stream when a body method took the response > destroy() leaves a body method that 
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/first_party/undici/undici.test.ts test/js/node/http/node-fetch.test.js
bun test v1.4.3 (6a92015fc)

test/js/first_party/undici/undici.test.ts:
(pass) undici > request > should make a GET request when passed a URL string [78.45ms]
(pass) undici > request > should error when body has already been consumed [15.95ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after arrayBuffer() consumed it [258.02ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after blob() consumed it [57.58ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after formData() consumed it [50.54ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after json() consumed it [48.81ms]
(pass) undici > request > body stream when a body method took the response > ends without an error after text() consumed it [48.04ms]
(pass) undici > request > body stream when a bo
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 639ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/21] gen JS modules (bundle-modules)
Preprocess modules (7038ms)
Bundle modules (48ms)
Postprocesss modules (24ms)
Bundle Functions (489ms)
Generate Code (42ms)

[7.65s] Bundled "src/js" for production
  2600 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[1/6] cargo bun_runtime → libbun_runtime.a
[1m[92m   Compiling[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
[1m[92m   Compiling[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
[1m[92m   Compiling[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
[1m[92m   Compiling[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
[1m[92m   Compiling[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
[1m[92m   Compiling[0m bun_base64 v0.0.0 (/workspace/bun/src/base64)
[1m[92m   Compiling[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
[1m[92m   Compiling[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
[1m[92m   Compiling[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
[1m[92m   Compili
... (truncated)

diff hotspot

src/js/internal/webstreams_adapters.ts    |  30 ++++--
 src/js/thirdparty/node-fetch.ts           |  36 +------
 src/js/thirdparty/undici.js               |   2 +-
 test/js/first_party/undici/undici.test.ts | 127 +++++++++++++++++++++++++
 test/js/node/http/node-fetch.test.js      | 150 ++++++++++++++++++++++++++++++
 5 files changed, 304 insertions(+), 41 deletions(-)

gate history · 2 passed · 0 rejected · iteration 2

evidence per changed file

file                                       reads  edits  tests
src/js/internal/webstreams_adapters.ts         5     11     17
src/js/thirdparty/node-fetch.ts                3      3     17
src/js/thirdparty/undici.js                    2      3     17
test/js/first_party/undici/undici.test.ts      1      1     12
test/js/node/http/node-fetch.test.js           2      3     17

#42573 node-fetch, undici: end the body stream when a body method took the response

Problem

Fix

Background

node-fetch, undici: end the body stream when a body method took the response