Skip to content

stream: improve handling of falsy errors in stream/iter - #65864

Open
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/stream-iter-fixes-3
Open

stream: improve handling of falsy errors in stream/iter#65864
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/stream-iter-fixes-3

Conversation

@jasnell

@jasnell jasnell commented Sep 6, 2026

Copy link
Copy Markdown
Member
  • Preserves arbitrary failure and cancellation reasons, including falsy values such as undefined, null, false, 0, and ''.
  • Distinguishes omitted cancellation reasons from explicitly passed undefined.
  • Removes internal coercion of non-Error reasons.
  • Wraps non-Error values only at classic stream boundaries, retaining the original value in error.reason.
  • Improves pending write/end cleanup for file, QUIC, broadcast, push, share, and classic stream writers.
  • Ensures terminal writer errors take precedence over per-operation abort signals.
  • Adds extensive reason-propagation and lifecycle tests.
  • Updates stream/iter documentation accordingly.

@jasnell
jasnell requested review from mcollina and trivikr September 6, 2026 20:47
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic
  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 6, 2026
@jasnell
jasnell requested a review from pimterry September 6, 2026 20:51
@jasnell jasnell added stream Issues and PRs related to Node.js streams. quic Issues and PRs related to the QUIC transport implementation. labels Sep 6, 2026
@trivikr

trivikr commented Sep 6, 2026

Copy link
Copy Markdown
Member

Lint error needs fixing

/home/runner/work/node/node/lib/internal/streams/iter/push.js
Error:   57:7  error  'kNoFailReason' is assigned a value but never used  no-unused-vars

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.14888% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.20%. Comparing base (8af7545) to head (bd2e9b4).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/fs/promises.js 72.78% 39 Missing and 1 partial ⚠️
lib/internal/streams/iter/classic.js 80.27% 25 Missing and 4 partials ⚠️
lib/internal/streams/iter/push.js 81.48% 5 Missing ⚠️
lib/internal/streams/iter/share.js 92.68% 3 Missing ⚠️
lib/internal/streams/iter/broadcast.js 91.66% 2 Missing ⚠️
lib/internal/streams/iter/transform.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65864      +/-   ##
==========================================
+ Coverage   90.18%   90.20%   +0.01%     
==========================================
  Files         771      771              
  Lines      264619   264779     +160     
  Branches    50231    50282      +51     
==========================================
+ Hits       238653   238837     +184     
+ Misses      16961    16921      -40     
- Partials     9005     9021      +16     
Files with missing lines Coverage Δ
lib/internal/abort_controller.js 95.35% <100.00%> (+<0.01%) ⬆️
lib/internal/quic/quic.js 100.00% <100.00%> (ø)
lib/internal/streams/iter/pull.js 83.75% <100.00%> (+0.23%) ⬆️
lib/internal/streams/iter/utils.js 98.39% <ø> (-0.05%) ⬇️
lib/internal/streams/iter/transform.js 95.83% <0.00%> (+0.11%) ⬆️
lib/internal/streams/iter/broadcast.js 90.19% <91.66%> (+0.70%) ⬆️
lib/internal/streams/iter/share.js 87.80% <92.68%> (+0.34%) ⬆️
lib/internal/streams/iter/push.js 92.98% <81.48%> (+0.02%) ⬆️
lib/internal/streams/iter/classic.js 88.96% <80.27%> (-0.26%) ⬇️
lib/internal/fs/promises.js 90.93% <72.78%> (-0.17%) ⬇️

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

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
jasnell force-pushed the jasnell/stream-iter-fixes-3 branch from bd2e9b4 to 2794c65 Compare September 6, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants