Skip to content

stream: fix ERR_INVALID_STATE when cancelling Readable.toWeb() - #62773

Open
richardscarrott wants to merge 1 commit into
nodejs:mainfrom
richardscarrott:fix/readable-to-web-cancel-race
Open

stream: fix ERR_INVALID_STATE when cancelling Readable.toWeb()#62773
richardscarrott wants to merge 1 commit into
nodejs:mainfrom
richardscarrott:fix/readable-to-web-cancel-race

Conversation

@richardscarrott

Copy link
Copy Markdown

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

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Apr 16, 2026
@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.69%. Comparing base (ed05549) to head (96e546b).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@           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     
Files with missing lines Coverage Δ
lib/internal/webstreams/adapters.js 86.61% <100.00%> (+0.01%) ⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@efekrskl

efekrskl commented Jul 17, 2026

Copy link
Copy Markdown
Member

Thank you for your contribution @richardscarrott!

Could you please fix the linting errors?

@smoores-dev

Copy link
Copy Markdown

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!

@richardscarrott

Copy link
Copy Markdown
Author

@smoores-dev @efekrskl soz, forgot about this. I have some time tomorrow to fix this up.

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
@richardscarrott
richardscarrott force-pushed the fix/readable-to-web-cancel-race branch from 96e546b to 93b1db6 Compare August 23, 2026 10:13
@richardscarrott

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants