stream: fix ERR_INVALID_STATE when cancelling Readable.toWeb() - #62773
stream: fix ERR_INVALID_STATE when cancelling Readable.toWeb()#62773richardscarrott wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #62773 +/- ##
=======================================
Coverage 89.69% 89.69%
=======================================
Files 706 706
Lines 218127 218144 +17
Branches 41734 41743 +9
=======================================
+ Hits 195651 195669 +18
+ Misses 14400 14396 -4
- Partials 8076 8079 +3
... and 41 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
|
Thank you for your contribution @richardscarrott! Could you please fix the linting errors? |
Sorry, something went wrong.
|
Howdy folks! I think we're running into this as well — @richardscarrott if you don't have time to take this over the finish line, happy to take it over and fix the linting errors! |
Sorry, something went wrong.
|
@smoores-dev @efekrskl soz, forgot about this. I have some time tomorrow to fix this up. |
Sorry, something went wrong.
When a web ReadableStream returned by Readable.toWeb() is cancelled while the underlying Readable is actively producing data, a pending onData callback can still fire after the controller has been closed and attempt to enqueue a chunk, throwing ERR_INVALID_STATE. Check wasCanceled in the onData handler and return early to avoid calling controller.enqueue() on a closed controller. Refs: nodejs#54205
|
Rebased onto main and fixed the lint error. Worth flagging for re-review: #62394 rewrote newReadableStreamFromStreamReadable after this PR was opened, so the patch needed porting — the onData logic now lives in an inline 'data' listener. I confirmed the race is still present on current main; without the guard, test-stream-readable-to-web-termination.js fails with: TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
at ReadableStreamDefaultController.enqueue (node:internal/webstreams/readablestream:1189:13)
at Readable.onData (node:internal/webstreams/adapters:547:18)
at flow (node:internal/streams/readable:1319:53)
at resume_ (node:internal/streams/readable:1295:3)
Note this is the enqueue path, distinct from the close path in #54205 (which #54206 already fixed) — hence Refs: rather than Fixes:. Upstream's rewritten test file is preserved as-is with the new case appended. @meixg your approval predates the rebase, so this likely needs another look. |
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
When a web ReadableStream returned by Readable.toWeb() is cancelled while the underlying Readable is actively producing data, a pending onData callback can still fire after the controller has been closed and attempt to enqueue a chunk, throwing ERR_INVALID_STATE.
Check wasCanceled in the onData handler and return early to avoid calling controller.enqueue() on a closed controller.
Refs: #54205