Skip to content

stream: reject pending reads on iterator throw - #63555

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-push-throw-pending-next
May 27, 2026
Merged

stream: reject pending reads on iterator throw#63555
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-push-throw-pending-next

Conversation

@trivikr

Copy link
Copy Markdown
Member

Fixes: #63554

Stream.push() readable iterators could leave an already pending next()
promise unresolved when iterator.throw() was called.

This updates consumer cancellation handling so pending reads are settled:

  • iterator.throw(error) rejects pending reads with error
  • iterator.return() resolves pending reads with done: true

Assisted-by: openai:gpt-5.5

Settle reads that are already waiting when a push stream iterator is
returned or thrown. This prevents next() promises from hanging after
consumer cancellation.
Fixes: nodejs#63554
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels May 25, 2026
@codecov

codecovBot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (8c495c8) to head (cbf0957).
⚠️ Report is 21 commits behind head on main.

Files with missing linesPatch %Lines
lib/internal/streams/iter/push.js55.55%4 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #63555 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 730 730 Lines 234362 234371 +9 Branches 43908 43920 +12 ==========================================
+ Hits 211708 211713 +5 - Misses 14376 14387 +11 + Partials 8278 8271 -7 
Files with missing linesCoverage Δ
lib/internal/streams/iter/push.js91.57% <55.55%> (-0.44%)⬇️

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

@trivikrtrivikr 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 May 25, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 25, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikrtrivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label May 27, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 27, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 7dbded2 into nodejs:mainMay 27, 2026
86 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 7dbded2

@aduh95aduh95 mentioned this pull request May 27, 2026
aduh95 pushed a commit that referenced this pull request May 27, 2026
Settle reads that are already waiting when a push stream iterator is
returned or thrown. This prevents next() promises from hanging after
consumer cancellation.
Fixes: #63554
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63555Fixes: #63554
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Comment threadtest/parallel/test-stream-iter-push-writer.js
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 30, 2026
Settle reads that are already waiting when a push stream iterator is
returned or thrown. This prevents next() promises from hanging after
consumer cancellation.
Fixes: nodejs#63554
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: nodejs#63555Fixes: nodejs#63554
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
Settle reads that are already waiting when a push stream iterator is
returned or thrown. This prevents next() promises from hanging after
consumer cancellation.
Fixes: #63554
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63555
Backport-PR-URL: #64675Fixes: #63554
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: push() iterator throw() leaves pending next() unresolved

6 participants

@trivikr@nodejs-github-bot@jasnell@MattiasBuelens@aduh95@jakecastelli