Skip to content

stream: reject pull() reads on abort#63498

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

stream: reject pull() reads on abort#63498
trivikr wants to merge 3 commits 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 95.29412% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (a7d5446) to head (5db38ba).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/pull.js 95.29% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63498      +/-   ##
==========================================
+ Coverage   90.15%   90.33%   +0.17%     
==========================================
  Files         718      730      +12     
  Lines      227920   234284    +6364     
  Branches    42824    43942    +1118     
==========================================
+ Hits       205472   211629    +6157     
- Misses      14225    14377     +152     
- Partials     8223     8278      +55     
Files with missing lines Coverage Δ
lib/internal/streams/iter/pull.js 88.33% <95.29%> (+3.18%) ⬆️

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

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2026
Comment thread lib/internal/streams/iter/pull.js Outdated
Comment thread lib/internal/streams/iter/pull.js Outdated
@trivikr trivikr requested a review from jasnell May 24, 2026 00:17
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. request-ci Add this label to start a Jenkins CI on a PR. 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()

3 participants