streams: add the ERR_INVALID_STATE code to the locked-stream errors of cancel, pipeTo and pipeThrough - #43714
Merged
Jarred-Sumner merged 3 commits intoSep 21, 2026
Conversation
…f cancel, pipeTo and pipeThrough ReadableStream.prototype.cancel, pipeTo and pipeThrough reject or throw a plain TypeError when the stream, or the destination WritableStream, is locked. Bun 1.3.14 and Node.js give this error the ERR_INVALID_STATE code. tee() and getReader() already do. Use the same error here, with the message that Node.js uses.
Contributor
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 4 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Comment |
Collaborator
Author
|
Status: the fix is pushed (3 commits). CI is in progress. How to reproduce the missing code: const rs = new ReadableStream();
rs.getReader();
await rs.cancel().catch(e => console.log(e.name, e.code, e.message));
How to reproduce the swallowed error: const res = new Response((async function* () {
yield "first;";
const locked = new ReadableStream();
locked.getReader();
locked.getReader();
})());
console.log(await res.text().then(v => "resolved " + v, e => "rejected " + e.code));
The three new tests in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…able body asyncIterFinishWithError resolved the pull of an async-iterable body when the error had the ERR_INVALID_STATE code, and it did not close the stream. The consumer then got a truncated body, or a read() that never settles. The check replaced the test for a closed direct controller that the JS implementation had. A closed direct controller does not throw now, and no sink throws this code, so the check matched only errors from the iterator. A cancelled stream still resolves the pull through m_cancelled.
Collaborator
Author
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
Author
|
I checked the head of this PR (93fa7cd merged onto main 9cba903, debug build) with a wider probe of the swallow in The probe throws an
The consumers are The |
Jarred-Sumner
deleted the
robobun/5f7b76a0/locked-stream-invalid-state
branch
September 21, 2026 22:33
This was referenced Sep 22, 2026
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 22, 2026
…ble body (#43758) ### Problem - An async-iterable body (`new Response(asyncGen())`, a `fetch` upload, a `Bun.serve` response) swallows each iterator error with `code: "ERR_INVALID_THIS"`, for example from `ReadableStream.prototype.tee.call({})`. `text()` resolves truncated, a server accepts the upload as complete, and a `read()` loop never settles. - The cause is the by-code check in `asyncIterFinishWithError` (`src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp:180`). #43714 removed the same check for `ERR_INVALID_STATE`. ### Fix - Remove the check and `errorCodeIs()`, which has no caller left. The error now rejects the pull like every other error. - Correct because no sink sends this code to the pump now. A detached sink threw it on `write()` when #8941 added the check. Since #15234 it throws an `Error` with no code, or returns 0. - Verified: `test/js/bun/http/async-iterator-stream.test.ts`. The `ERR_INVALID_STATE` test of #43714 is now a `test.each` over both codes, with an upload whose server must read an aborted body. Main fails the `ERR_INVALID_THIS` case. Also `body-async-iterator`, `serve-direct-readable-stream`, `streams`, `body-stream`. Self-reviewed: 3 concerns raised, 3 addressed. ### Background - `BunAsyncIterableSource.cpp` feeds a `ReadableStream` from the iterator. Its pump calls `iterator.next()` and writes each value to the consumer's controller. - The pull is the promise that the pump returns to the stream. A resolved pull ends the body, a rejected pull fails it. - A sink is a native consumer (an HTTP response, a file, an upload). A detached sink lost its destination. ### Downsides - A generator that ends its body early with an `ERR_INVALID_THIS` error now fails the body. The repo has no such use. - Such an error now resets a `Bun.serve` response. Before, the client got a complete 200. <details><summary>Notes</summary> Repro (runs under Node.js and Bun): ```js const ways = { "own error with the code": () => { throw Object.assign(new Error("mine"), { code: "ERR_INVALID_THIS" }); }, "ReadableStream.prototype.tee.call({})": () => { ReadableStream.prototype.tee.call({}); }, "control: plain TypeError": () => { throw new TypeError("plain"); }, }; const body = way => (async function* () { yield "first;"; ways[way](); yield "NEVER"; })(); const settle = p => Promise.race([p.then(v => "resolved " + JSON.stringify(v), e => "rejected " + (e.code ?? e.name)), new Promise(r => setTimeout(() => r("NEVER SETTLES"), 3000))]); for (const way of Object.keys(ways)) { console.log(way); console.log(" text() ", await settle(new Response(body(way)).text())); console.log(" reader loop ", await settle((async () => { const r = new Response(body(way)).body.getReader(); while (!(await r.read()).done); return "drained"; })())); } ``` | way | consumer | Bun 1.3.14 | Bun 1.4.3-canary (367d939) | this PR and Node.js v26.3.0 | | --- | --- | --- | --- | --- | | own error with the code, `tee.call({})` | `text()` | never settles | resolves `"first;"` | rejects | | own error with the code, `tee.call({})` | reader loop | never settles | never settles | rejects | | `locked` getter of `ReadableStream` or `WritableStream` on `{}` | both | rejects (the error had no code) | as the rows above | rejects | So the swallow is older than the C++ streams rewrite (#33193) for an error that already carried the code. It is new in 1.4.0 for the two `locked` getters, which got the code with the rewrite. Wider probes, all on an ASAN debug build with `Malloc=1`, no sanitizer report: - 8 things to throw (own error, frozen error, a plain object with the code, three wrong-receiver calls, and a control) x 7 places (after the first `yield`, before it, after the last one, in a `finally` at the normal end, a custom iterator whose `next()` rejects or throws, an async generator function as the body) x 8 consumers (`text()`, `arrayBuffer()`, reader loop, `for await`, `pipeTo()`, `Bun.write()`, `fetch` upload, `Bun.serve` response). The 1.4.3 canary swallows 336 of 336 cells with the code. This PR: each cell has the outcome of the plain `TypeError` control, none hangs, and there is no unhandled rejection. - 11 consumers (the above plus `request.text()`, `Readable.fromWeb()`, a node:http response fed through `pipeline`) after a 6 byte and an 8 MiB prefix: the 1.4.3 canary swallows 22 of 22 cells, this PR 0. - Cases where the consumer goes away: `reader.cancel()` with and without a reason, a `for await` that breaks, a client that aborts a `Bun.serve` stream, a raw socket destroyed in the middle of the response, an upload whose server closes early, an upload aborted by its signal. In each case the generator ends quietly, throws a plain `Error` from its `finally`, or throws an error with the code from its `finally`. The 21 outcomes are the same before and after this change, with no unhandled rejection. These cases do not reach the error tail: the `cancel()` or `close()` hook of the source sets `m_cancelled` first. Producers of the code. Under `src/jsc/bindings/webcore/streams` only the brand checks of the spec classes throw it (`ReadableStream`, `WritableStream`, `TransformStream`, their readers, writers and controllers, the queuing strategies, the compression and text streams). The pump calls none of them. `JSDirectStreamController.cpp`, `BunStreamSource.cpp`, `BunStreamConsumers.cpp` and the generated `JSSink.cpp` do not throw it. On the sink side only `JSSink::get_this` (`src/runtime/webcore/Sink.rs:473`) throws it, for `CAST_FAILED`. `${name}__fromJS` (`src/codegen/generate-jssink.ts`) returns that value only when `this` is neither the sink nor its controller, and the pump calls `write`, `flush` and `end` with the controller as `this`. The five methods of the JS-facing `JSDirectStreamController` are bound functions and are no-ops once the source ended. The test: the generator of the upload throws once its request is at the server, so the server side is deterministic. With the `src/` of main (4ada08b) the `ERR_INVALID_THIS` case fails: `text()` and the upload resolve, the server reads `complete, 6 bytes`, and `read()` never settles. The `ERR_INVALID_STATE` case passes there. With this change both pass, 20 of 20 runs on a Linux ASAN build and 20 of 20 on a Windows x64 debug build. An upload that never settles ends in the timeout of the test runner. The test has no timer of its own. Self-review, three concerns, all addressed. (1) Can a sink or controller still send the code to the pump when the consumer is gone? No, see the list of producers above. (2) The first version of the test repeated the #43714 test. The two are now one `test.each`. (3) The upload raced the throw against the connect. The generator now throws once its request is at the server, and the test asserts what the server read. The review bot raised two optional points on the test. The server-side assertion covers the first. For the second (an upload that never settles has no diagnostics) I changed the comment and did not add a timer, because the test rules of this repo do not allow one. Suites run with the debug build: `test/js/bun/http/async-iterator-stream.test.ts` (97 pass), `test/js/web/fetch/body-async-iterator.test.ts`, `test/js/bun/http/serve-direct-readable-stream.test.ts` (169 pass), `test/js/bun/http/serve-error-handler-stream.test.ts`, `test/js/web/fetch/body-clone.test.ts`, `test/js/web/streams/streams.test.js` (614 pass), `test/js/web/fetch/body.test.ts`, `test/js/web/fetch/body-stream.test.ts` (9086 pass), `test/js/bun/http/bun-server.test.ts`. The two coded-error tests also pass with `BUN_JSC_validateExceptionChecks=1`. </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ReadableStream.prototype.cancel(),pipeTo()andpipeThrough()on a locked stream fail with aTypeErrorthat has nocode:Cannot cancel a locked ReadableStream,Cannot pipe a locked ReadableStream,Cannot pipe to a locked WritableStream.code: 'ERR_INVALID_STATE'here, as Node.js v26.3.0 does. The C++ streams rewrite (webstreams: rewrite ReadableStream, WritableStream, and TransformStream in C++ (zero JS builtins) #33193) dropped it insrc/jsc/bindings/webcore/streams/JSReadableStream.cpp(lines 597, 670, 672, 697, 699).asyncIterFinishWithError(BunAsyncIterableSource.cpp:182) swallows an error with that code from an async-iterable body.text()resolves truncated andread()never settles. On Bun 1.4.3 agetReader()error already does this.Fix
Bun::createErrororBun::throwErrorwithERR_INVALID_STATE_TypeErrorand the Node.js messages, for exampleInvalid state: The ReadableStream is locked.m_cancelledflag still covers a cancelled stream.TypeError, which is all that WPT requires. The check order does not change.test/js/web/streams/streams.test.jsandtest/js/bun/http/async-iterator-stream.test.ts(three new tests, Bun 1.4.3 fails all). Also WPT streams and the vendored Node.js stream tests. Self-reviewed: 2 concerns raised, 2 addressed.Background
ReadableStreamis locked while a reader holds it.ERR_INVALID_STATEis the Node.js errorcodefor each "stream is locked" error.new Response(async function* () {})) is aReadableStreamthatBunAsyncIterableSource.cppfeeds from the iterator.asyncIterFinishWithErrorruns when the iterator rejects.Notes
Repro for the missing code:
cancel()ERR_INVALID_STATE,Invalid state: ReadableStream is lockedCannot cancel a locked ReadableStreamERR_INVALID_STATE,Invalid state: ReadableStream is lockedpipeTo(),pipeThrough()ERR_INVALID_STATE,Invalid state: ReadableStream is lockedCannot pipe a locked ReadableStreamERR_INVALID_STATE,Invalid state: The ReadableStream is lockedpipeTo(),pipeThrough()into a lockedWritableStreamWritableStream is lockedCannot pipe to a locked WritableStreamERR_INVALID_STATE,Invalid state: The WritableStream is lockedThe check order is the same as in Node.js (
lib/internal/webstreams/readablestream.js): argument validation first, then the locked source, then the locked destination. Nine doubly-invalid calls give the same winner and the same delivery (throw or rejection) under Node.js v26.3.0 and this build. No test, doc or source file matches the old message text.The swallow in
asyncIterFinishWithError. Before #33193 the JS implementation (readableStreamFromAsyncIterator) dropped the error of the iterator only whencontroller.write === $onReadableStreamDirectControllerClosed, that is, when the consumer was gone. #33193 changed that to "the error has theERR_INVALID_STATEcode". The closed controller threw a plainTypeErroreven then, so the code check never matched it. It matched only errors that the iterator itself lets out:res.text()"first;"res.body.getReader()loopThe first push of this PR added the code to
cancel(),pipeTo()andpipeThrough()only, so these errors joined that set. The review thread onJSReadableStream.cppreported it. Now the swallow is removed and the handleronAsyncIterableSourceErrorSwallowedwith it.Why no consumer-gone case needs it. I traced each case with a temporary log in
asyncIterFinishWithError. A client that aborts aBun.serveresponse,reader.cancel(), afor awaitthat breaks, and a fetch upload whose server closes never reach the error tail: thecancel()orclose()hook of the source setsm_cancelledfirst. The five methods of a closed direct controller are no-ops (sourceEnded()). A native sink whose destination closed returns 0 (CONTROLLER_DETACHEDinSink.rs). A sink that the source already ended throws anErrorwith no code. A wrong receiver throwsERR_INVALID_THIS, which the function still handles. No sink or controller path throwsERR_INVALID_STATE. With the log on, none ofasync-iterator-stream,body-async-iterator,serve-direct-readable-stream,body,streams,serve,bun-serverandfetchtook the swallow branch.Self-review: two read-only passes over the final diff. One tried to find a producer of
ERR_INVALID_STATEthat reaches the error tail when the consumer is gone, and found none. The other compared the five sites with the Node.js source and checked the tests. It raised two concerns about the new subprocess test (process.oncould print twice, and an empty stdout gave a poor failure message). Both are addressed: the test usesprocess.onceand compares lines.Suites run with the debug build:
test/js/web/streams/streams.test.js(614 pass),test/js/bun/http/async-iterator-stream.test.ts(96 pass),test/js/third_party/wpt-streams/wpt-streams.test.ts(1175 pass), the 31 vendoredtest-whatwg-*andtest-webstream*files intest/js/node/test/parallel/,test/js/web/fetch/body.test.ts,body-async-iterator.test.ts,body-clone.test.ts,client-fetch.test.ts,fetch-args.test.ts,test/js/bun/http/serve-direct-readable-stream.test.ts,serve-error-handler-stream.test.ts,bun-server.test.ts,proxy.test.ts,test/js/node/http/node-fetch.test.js,test/js/first_party/undici/undici.test.ts,test/js/node/stream/node-stream.test.js,web-stream-state.test.ts,test/js/web/streams/compression.test.ts, the twopipeTo-*tests, andtest/regression/issue/014187.test.tsand26225.test.ts.test/js/bun/http/serve.test.tshas two failures that the released Bun also has in this container (a root-only port test and a loopback test). The new tests pass withBUN_JSC_validateExceptionChecks=1.The first block of Node's
test/parallel/test-whatwg-webstreams-adapters-to-streamreadable.jsasserts these codes on the stream thatReadable.fromWeb()locked. This PR does not vendor that file. On main it fails earlier, onassert(readableStream.locked), becauseReadable.fromWeb()takes the reader on the first read and not at once. That is a separate change insrc/js/internal/webstreams_adapters.ts. With a local stand-in for that change, the first two blocks of the file pass with this PR.Found outside this change, with no open PR:
ERR_INVALID_STATEand Bun throws a plainTypeError. Bun 1.3.14 had no code there, so these are not regressions:pipeTo()into a closedWritableStream(JSStreamPipeToOperation.cpp:267),writer.write()while the stream closes (JSWritableStreamDefaultWriter.cpp:148), andTransformStreamDefaultController.enqueue()afterterminate()or after the readable side is cancelled (JSTransformStreamDefaultController.cpp:403).ReadableStream.prototype.cancel,pipeTo,WritableStream.prototype.abort,close) reject a wrongthiswith a plainTypeError. Node.js rejects withERR_INVALID_THIS. The argument checks ofpipeTo()andpipeThrough()(destination, transform, options,signal) have noERR_INVALID_ARG_TYPE.asyncIterFinishWithErroralso treats eachERR_INVALID_THISerror from the iterator as "the consumer is gone".ReadableStream.prototype.tee.call({})in an async-generator body makesres.text()resolve truncated on Bun 1.4.3 and on this branch. Bun 1.3.14 had the same check. A sink throws this code for a wrong receiver, so that check needs a test of the controller state and not of the code.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