Skip to content

stream: fix pipeToSync byte accounting#63564

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-pipetosync-writesync-backpressure
Jun 8, 2026
Merged

stream: fix pipeToSync byte accounting#63564
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-pipetosync-writesync-backpressure

Conversation

@trivikr

@trivikr trivikr commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes: #63562

pipeToSync() ignored explicit false returns from writeSync() and
writevSync(), so it could report bytes for chunks that were not accepted by
the writer.

This updates pipeToSync() to stop counting rejected sync writes while still
preserving writers that intentionally return false after accepting data as a
backpressure signal.


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 25, 2026
@trivikr trivikr force-pushed the stream-iter-pipetosync-writesync-backpressure branch from 0b933ff to 8b50431 Compare May 25, 2026 16:54
@trivikr trivikr changed the title stream/iter: fix pipeToSync byte accounting stream: fix pipeToSync byte accounting May 25, 2026
@codecov

codecov Bot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (49dcaa0) to head (8b50431).
⚠️ Report is 125 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/pull.js 88.88% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #63564    +/-   ##
========================================
  Coverage   90.33%   90.33%            
========================================
  Files         730      730            
  Lines      234362   234487   +125     
  Branches    43906    43932    +26     
========================================
+ Hits       211713   211835   +122     
- Misses      14371    14389    +18     
+ Partials     8278     8263    -15     
Files with missing lines Coverage Δ
lib/internal/streams/iter/pull.js 88.09% <88.88%> (-0.03%) ⬇️

... and 43 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 marked this pull request as draft May 25, 2026 23:01
@trivikr

This comment was marked as outdated.

@trivikr trivikr marked this pull request as ready for review June 8, 2026 01:49
@trivikr trivikr force-pushed the stream-iter-pipetosync-writesync-backpressure branch from 8b50431 to 7daf002 Compare June 8, 2026 01:49
Stop pipeToSync() from counting chunks when writeSync() or writevSync()
returns false without accepting the data. Preserve writers that use
false as an accepted backpressure signal.

Fixes: nodejs#63562

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr trivikr force-pushed the stream-iter-pipetosync-writesync-backpressure branch from 7daf002 to 70cf4f8 Compare June 8, 2026 02:06

@mcollina mcollina 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.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 8, 2026
@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 8, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 8, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 8, 2026
@nodejs-github-bot nodejs-github-bot merged commit 696030d into nodejs:main Jun 8, 2026
75 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 696030d

@trivikr trivikr deleted the stream-iter-pipetosync-writesync-backpressure branch June 8, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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: pipeToSync() ignores writeSync() backpressure and overcounts bytes

3 participants