Skip to content

stream: fix unhandled rejection for errored pipeTo writes - #64580

Closed
dushyant-hada-90 wants to merge 1 commit into
nodejs:mainfrom
dushyant-hada-90:fix-pipeto-unhandled-rejection
Closed

stream: fix unhandled rejection for errored pipeTo writes#64580
dushyant-hada-90 wants to merge 1 commit into
nodejs:mainfrom
dushyant-hada-90:fix-pipeto-unhandled-rejection

Conversation

@dushyant-hada-90

@dushyant-hada-90dushyant-hada-90 commented Jul 18, 2026

Copy link
Copy Markdown

Fixes a v26.4.0 regression where ReadableStream.pipeTo() could crash the
process with an unhandled rejection even when the pipeTo() promise
rejection was already caught.

In #63572, per-chunk setPromiseHandled() was replaced with
markPromiseAsHandled(). That is safe for pending write promises, but when
a later chunk is written after the destination has already errored,
writableStreamDefaultWriterWrite() returns an already-rejected promise.
Marking that promise handled afterwards does not cancel V8's pending
unhandled-rejection notification.

This keeps markPromiseAsHandled() on the writable fast path and falls
back to setPromiseHandled() when the destination is no longer writable
(or close is queued/in flight).

No documentation changes; behavior is restored to match Node.js ≤ v26.3.1,
Deno, Bun, and browsers.

Checklist
  • make -j4 test (UNIX) or vcbuild test (Windows) passes / CI will validate
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Fixes: #64561
Refs: #63572

markPromiseAsHandled does not cancel V8's unhandled-rejection
notification when the write promise is already rejected. Fall back
to setPromiseHandled when the destination is no longer writable.
Fixes: nodejs#64561
Refs: nodejs#63572
Signed-off-by: dushyant <dushyanthada90@gmail.com>
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jul 18, 2026
@codecov

codecovBot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (fb5d01f) to head (3cfa60c).
⚠️ Report is 109 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #64580 +/- ##
==========================================
- Coverage 90.24% 90.24% -0.01% 
==========================================
Files 739 739 Lines 241772 241787 +15 Branches 45561 45571 +10 ==========================================
- Hits 218194 218192 -2 - Misses 15111 15120 +9 - Partials 8467 8475 +8 
Files with missing linesCoverage Δ
lib/internal/webstreams/readablestream.js98.19% <100.00%> (+<0.01%)⬆️

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

@avivkeller

Copy link
Copy Markdown
Member

@nodejs/streams

@dushyant-hada-90

Copy link
Copy Markdown
Author

Friendly ping @nodejs/streams @nodejs-github-bot

@dushyant-hada-90

Copy link
Copy Markdown
Author

The issue has been fixed in this PR #64890

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

Labels

needs-ciPRs that need a full CI run.web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TransformStream error during pipeTo crashes the process despite being caught (v26.4.0 regression)

3 participants

@dushyant-hada-90@avivkeller@nodejs-github-bot