Skip to content

stream: avoid duplicate writes in toWritable - #63360

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:stream-iter-towritable-write
May 23, 2026
Merged

stream: avoid duplicate writes in toWritable#63360
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:stream-iter-towritable-write

Conversation

@trivikr

Copy link
Copy Markdown
Member

toWritable() currently treats writeSync() / writevSync() returning
false as meaning the data was not accepted and retries through the async
writer method.

For PushWriter with backpressure: 'block', false can instead mean the
chunk was accepted and backpressure is active. Retrying through
write() / writev() writes the same data twice.

This updates the adapter to recognize that accepted-backpressure case and wait
for drain instead of retrying the chunk.

Fixes: #63359


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 16, 2026
@codecov

codecovBot commented May 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.14286% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (1f371fc) to head (c5132e7).
⚠️ Report is 78 commits behind head on main.

Files with missing linesPatch %Lines
lib/internal/streams/iter/classic.js80.00%9 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #63360 +/- ##
=======================================
Coverage 90.05% 90.05% =======================================
Files 714 714 Lines 225704 225768 +64 Branches 42714 42727 +13 =======================================
+ Hits 203250 203321 +71 - Misses 14225 14242 +17 + Partials 8229 8205 -24 
Files with missing linesCoverage Δ
lib/internal/streams/iter/push.js92.20% <100.00%> (+0.12%)⬆️
lib/internal/streams/iter/types.js100.00% <100.00%> (ø)
lib/internal/streams/iter/classic.js88.91% <80.00%> (-0.54%)⬇️

... and 42 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 self-assigned this May 16, 2026
@trivikr
trivikrforce-pushed the stream-iter-towritable-write branch from a03df89 to 52ffb94CompareMay 16, 2026 15:12
@trivikrtrivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2026
@trivikr
trivikrforce-pushed the stream-iter-towritable-write branch from 52ffb94 to 7fc1888CompareMay 17, 2026 15:50
trivikr added 2 commits May 17, 2026 16:38
PushWriter can return false after accepting a chunk when block
backpressure is active. Teach the classic Writable adapter to treat
that case as accepted backpressure instead of retrying through the
async write path.
Fixes: nodejs#63359
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr
trivikrforce-pushed the stream-iter-towritable-write branch from 7fc1888 to c5132e7CompareMay 17, 2026 23:38
@trivikrtrivikr removed their assignment May 21, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 22, 2026
@nodejs-github-bot

nodejs-github-bot commented May 22, 2026

Copy link
Copy Markdown
Collaborator

@jasnelljasnell added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels May 22, 2026
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/63360
✔ Done loading data for nodejs/node/pull/63360
----------------------------------- PR info ------------------------------------
Title stream: avoid duplicate writes in toWritable (#63360)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch trivikr:stream-iter-towritable-write -> nodejs:main
Labels stream, author ready, needs-ci
Commits 2
- stream: avoid duplicate writes in toWritable
- doc: describe kSyncWriteAcceptedOnFalse stream sentinel
Committers 1
- Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/63360
Fixes: https://github.com/nodejs/node/issues/63359
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/63360
Fixes: https://github.com/nodejs/node/issues/63359
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
ℹ This PR was created on Sat, 16 May 2026 02:41:57 GMT
✔ Approvals: 1
✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/63360#pullrequestreview-4342429217
✘ This PR needs to wait 13 more hours to land (or 0 minutes if there is one more approval)
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2026-05-22T04:50:23Z: https://ci.nodejs.org/job/node-test-pull-request/73613/
- Querying data for job/node-test-pull-request/73613/
✔ Build data downloaded
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/26292186844

@trivikrtrivikr added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 22, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2026
@trivikrtrivikr added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 23, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 8d3245e into nodejs:mainMay 23, 2026
99 of 100 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 8d3245e

@trivikr
trivikr deleted the stream-iter-towritable-write branch May 23, 2026 03:19
aduh95 pushed a commit that referenced this pull request May 27, 2026
PushWriter can return false after accepting a chunk when block
backpressure is active. Teach the classic Writable adapter to treat
that case as accepted backpressure instead of retrying through the
async write path.
Fixes: #63359
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63360Fixes: #63359
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 30, 2026
PushWriter can return false after accepting a chunk when block
backpressure is active. Teach the classic Writable adapter to treat
that case as accepted backpressure instead of retrying through the
async write path.
Fixes: nodejs#63359
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: nodejs#63360Fixes: nodejs#63359
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
PushWriter can return false after accepting a chunk when block
backpressure is active. Teach the classic Writable adapter to treat
that case as accepted backpressure instead of retrying through the
async write path.
Fixes: #63359
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63360
Backport-PR-URL: #64675Fixes: #63359
Reviewed-By: James M Snell <jasnell@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.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.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: toWritable() duplicates writes when writeSync() returns false after accepting chunk

3 participants

@trivikr@nodejs-github-bot@jasnell