Skip to content

stream: prevent enqueue after cancel in Readable.toWeb() - #64766

Open
kairosci wants to merge 1 commit into
nodejs:mainfrom
kairosci:fix/webstreams-readable-cancel-backpressure
Open

stream: prevent enqueue after cancel in Readable.toWeb()#64766
kairosci wants to merge 1 commit into
nodejs:mainfrom
kairosci:fix/webstreams-readable-cancel-backpressure

Conversation

@kairosci

@kairosci kairosci commented Jul 26, 2026

Copy link
Copy Markdown

This pull request resolves an issue where converting a Node readable stream to a web stream using the toWeb method could result in an uncatchable exception when the stream is cancelled during a backpressure resume cycle. When the cancel method is invoked on the underlying source, the controller transitions to a closed state but the data listener remains attached to the source stream. Consequently, a scheduled flow tick may deliver an additional data event which triggers an invalid state error upon attempting to enqueue the chunk into the already closed controller. To fix this behavior structurally, a cancellation guard is added directly inside the data event listener to intercept and discard any incoming chunks once the stream has been marked as cancelled. A regression test is also included to ensure that disconnecting a pipeline under backpressure properly tears down the stream without throwing uncaught exceptions.

Fixes: #64529

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jul 26, 2026
@avivkeller avivkeller added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 26, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.16%. Comparing base (54a5095) to head (3710995).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/webstreams/adapters.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64766      +/-   ##
==========================================
+ Coverage   90.14%   90.16%   +0.01%     
==========================================
  Files         744      744              
  Lines      242518   242519       +1     
  Branches    45685    45691       +6     
==========================================
+ Hits       218611   218660      +49     
+ Misses      15396    15361      -35     
+ Partials     8511     8498      -13     
Files with missing lines Coverage Δ
lib/internal/webstreams/adapters.js 86.62% <0.00%> (-0.08%) ⬇️

... and 37 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.

@kairosci
kairosci force-pushed the fix/webstreams-readable-cancel-backpressure branch from 3710995 to 28f4c68 Compare July 27, 2026 15:32
@kairosci kairosci closed this Jul 27, 2026
@kairosci kairosci reopened this Jul 27, 2026
@kairosci

Copy link
Copy Markdown
Author

Sorry, missclicked.

Anyway, there were a few linting issues in test/parallel/test-webstreams-adapters-cancel-backpressure.js.

I removed some trailing spaces on empty lines.

I wrapped the .then() callbacks and the test exit in common.mustCall() to satisfy the node-core/must-call-assert ESLint rule, which requires Node's test runner to track promise settlements explicitly.

I properly declared const common = require('../common'); instead of just requiring it.

Commit message used webstreams:, but the correct subsystem prefix for this file is stream:.

I amended commit message from webstreams: prevent enqueue after cancel in Readable.toWeb() to stream: prevent enqueue after cancel in Readable.toWeb().

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Readable.toWeb(): uncaughtException (ERR_INVALID_STATE "Controller is already closed") when the stream is canceled during backpressure resume

3 participants