Skip to content

stream: preserve toReadableSync batch after backpressure#63276

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-to-readable-stream-backpressure
Open

stream: preserve toReadableSync batch after backpressure#63276
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-to-readable-stream-backpressure

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 13, 2026

toReadableSync() could drop chunks from a batch when classic stream
backpressure was applied.

When _read() pulled a batch with multiple chunks and push() returned
false, the method returned immediately without saving the remaining batch
items. A later _read() call advanced to the next iterator result, so the
unpushed chunks were lost.

This stores the current batch and index across _read() calls, allowing
toReadableSync() to resume the same batch after backpressure clears.

Fixes: #63275


Assisted-by: openai:gpt-5.5

Keep the current batch and index across _read() calls so chunks that
remain after push() returns false are emitted on later reads.

Fixes: nodejs#63275

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-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 13, 2026
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 13, 2026
@trivikr trivikr self-assigned this May 13, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (78bbee3) to head (2c7eb4b).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #63276    +/-   ##
========================================
  Coverage   90.04%   90.05%            
========================================
  Files         713      714     +1     
  Lines      225003   225256   +253     
  Branches    42536    42573    +37     
========================================
+ Hits       202606   202849   +243     
- Misses      14177    14181     +4     
- Partials     8220     8226     +6     
Files with missing lines Coverage Δ
lib/internal/streams/iter/classic.js 89.52% <100.00%> (+0.17%) ⬆️

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

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 13, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

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: toReadableSync() drops chunks after backpressure

4 participants