Skip to content

streams: more iter streams fixes - #66028

Open
jasnell wants to merge 6 commits into
nodejs:mainfrom
jasnell:jasnell/moar-iter-streams
Open

streams: more iter streams fixes#66028
jasnell wants to merge 6 commits into
nodejs:mainfrom
jasnell:jasnell/moar-iter-streams

Conversation

@jasnell

@jasnell jasnell commented Sep 15, 2026

Copy link
Copy Markdown
Member

Another batch of iterable streams fixes. See each commit.

@jasnell
jasnell requested review from panva and trivikr September 15, 2026 03:39
@nodejs-github-bot

nodejs-github-bot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams. labels Sep 15, 2026
@jasnell jasnell mentioned this pull request Sep 15, 2026
Draft
@codecov

codecov Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.23%. Comparing base (dbfa70a) to head (1c49e7e).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/broadcast.js 87.50% 3 Missing and 1 partial ⚠️
lib/internal/streams/iter/share.js 90.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #66028      +/-   ##
==========================================
- Coverage   92.76%   90.23%   -2.54%     
==========================================
  Files         420      789     +369     
  Lines      189145   270654   +81509     
  Branches    29054    51815   +22761     
==========================================
+ Hits       175460   244219   +68759     
- Misses      13362    16916    +3554     
- Partials      323     9519    +9196
Files with missing lines Coverage Δ
lib/internal/quic/quic.js 100.00% <100.00%> (ø)
lib/internal/streams/iter/share.js 89.51% <90.00%> (+1.71%) ⬆️
lib/internal/streams/iter/broadcast.js 90.17% <87.50%> (-0.03%) ⬇️

... and 502 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 reviewed Sep 15, 2026
View reviewed changes

@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 Assisted-by: Codex

Comment thread lib/internal/streams/iter/share.js Outdated
@jasnell
stream: update broadcast to retain buffered data with zero consumers
1e3eaca 
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
@jasnell
stream: make Broadcast.from abort its background pump
b2747f2 
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
stream: make share budget failures detach before throwing
33292f8 
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
stream: reject unbounded at SyncShare construction
d42f18a 
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
stream: fixup stream/iter to drop at most one entry per share call
f0764b4 
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
jasnell force-pushed the jasnell/moar-iter-streams branch from 7befc15 to f0764b4 Compare September 15, 2026 16:16
@jasnell
jasnell requested a review from panva September 15, 2026 16:17
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 15, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 15, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@jasnell
quic: fix timeout regression from 11ed325
1c49e7e 
Commit 11ed325 introduced a regression in quic

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

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/77538/

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

Reviewers

@panva panva panva approved these changes
@trivikr trivikr Awaiting requested review from trivikr

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jasnell @nodejs-github-bot @panva