Skip to content

stream: reject pull() reads on abort#63498

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-pull-abort-signal-source-next
Open

stream: reject pull() reads on abort#63498
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-pull-abort-signal-source-next

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 23, 2026

This updates stream/iter pull() so pending source reads are abort-aware.
When { signal } is provided, a pending source next() now rejects when the
signal aborts instead of waiting for the source to eventually yield.

The abort-aware wrapper returns the original source unchanged when no signal is
provided, so the normal no-signal path does not add an extra async iterator
layer.

The regression tests cover aborting while the source next() is pending in
both the no-transform path and a transform pipeline.

Fixes: #63497


Assisted-by: openai:gpt-5.5

@nodejs-github-bot
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 the stream subsystem. labels May 23, 2026
Make pull() race pending source reads against the provided AbortSignal
so aborting can reject a pending next() even when the source is waiting
before yielding data.

Fixes: nodejs#63497

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr trivikr force-pushed the stream-iter-pull-abort-signal-source-next branch from 77d3f55 to 9e427dd Compare May 23, 2026 02:53
@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (a7d5446) to head (9e427dd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/pull.js 90.90% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63498      +/-   ##
==========================================
- Coverage   90.15%   90.14%   -0.01%     
==========================================
  Files         718      718              
  Lines      227920   227991      +71     
  Branches    42824    42835      +11     
==========================================
+ Hits       205472   205524      +52     
- Misses      14225    14240      +15     
- Partials     8223     8227       +4     
Files with missing lines Coverage Δ
lib/internal/streams/iter/pull.js 85.47% <90.90%> (+0.32%) ⬆️

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

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

Labels

needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: pull() with AbortSignal hangs while awaiting source next()

2 participants