Skip to content

stream: more iter streams fixes - #66030

Draft
jasnell wants to merge 10 commits into
nodejs:mainfrom
jasnell:jasnell/moar-iter-streams-2
Draft

jasnell wants to merge 10 commits into
nodejs:mainfrom
jasnell:jasnell/moar-iter-streams-2

Conversation

@jasnell

@jasnell jasnell commented Sep 15, 2026

Copy link
Copy Markdown
Member

Another round of streams/iter fixes.

Builds on: #66028 ... The first four commits are from that PR. This will remain a draft until that one lands

While the Broadcast is still open, maintain buffered data when
consumer count reaches zero

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
…st paths

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @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 15, 2026
@jasnell
jasnell requested review from panva and trivikr September 15, 2026 03:54
@jasnell
jasnell marked this pull request as draft September 15, 2026 03:54
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.54635% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.22%. Comparing base (c081d10) to head (195c895).

Files with missing lines Patch % Lines
lib/internal/streams/iter/from.js 86.25% 32 Missing and 4 partials ⚠️
lib/internal/streams/iter/pull.js 88.23% 6 Missing ⚠️
lib/internal/streams/iter/broadcast.js 92.53% 4 Missing and 1 partial ⚠️
lib/internal/streams/iter/share.js 93.87% 3 Missing ⚠️
lib/internal/streams/iter/utils.js 93.61% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66030      +/-   ##
==========================================
- Coverage   90.22%   90.22%   -0.01%     
==========================================
  Files         787      787              
  Lines      270307   270614     +307     
  Branches    51726    51806      +80     
==========================================
+ Hits       243890   244166     +276     
- Misses      16899    16920      +21     
- Partials     9518     9528      +10     
Files with missing lines Coverage Δ
lib/internal/streams/iter/consumers.js 97.09% <100.00%> (+0.14%) ⬆️
lib/internal/streams/iter/duplex.js 97.47% <100.00%> (+0.25%) ⬆️
lib/internal/streams/iter/share.js 89.42% <93.87%> (+1.62%) ⬆️
lib/internal/streams/iter/utils.js 97.04% <93.61%> (-1.36%) ⬇️
lib/internal/streams/iter/broadcast.js 90.50% <92.53%> (+0.31%) ⬆️
lib/internal/streams/iter/pull.js 83.82% <88.23%> (+0.06%) ⬆️
lib/internal/streams/iter/from.js 87.99% <86.25%> (-0.85%) ⬇️

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

@panva panva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Asssisted-by: Codex

this.#sourceError = error;
this.#sourceExhausted = true;
} finally {
if (this.#sourceExhausted) this.#cleanupFactorySignal();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source exhaustion does not mean all consumers have finished. This removes the factory abort listener as soon as the fastest consumer reaches the end, while slower consumers can still have buffered data.

For example, create share('x', { signal }) with two consumers, exhaust the first, then abort. The second consumer's next() now returns 'x' instead of rejecting with the abort reason. Please retain the listener until the remaining active consumers finish, and add that case to the signal lifetime test.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants