FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: avoid duplicate writes in toWritable by trivikr · Pull Request #63360 · nodejs/node · GitHub

/ node Public

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 commented May 16, 2026

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

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

nodejs-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 Bot commented May 16, 2026
edited
Loading

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 lines Patch % Lines
lib/internal/streams/iter/classic.js 80.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 lines Coverage Δ
lib/internal/streams/iter/push.js 92.20% <100.00%> (+0.12%) ⬆️
lib/internal/streams/iter/types.js 100.00% <100.00%> (ø)
lib/internal/streams/iter/classic.js 88.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.

trivikr self-assigned this May 16, 2026
trivikr force-pushed the stream-iter-towritable-write branch from a03df89 to 52ffb94 Compare May 16, 2026 15:12
trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2026
trivikr force-pushed the stream-iter-towritable-write branch from 52ffb94 to 7fc1888 Compare May 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 force-pushed the stream-iter-towritable-write branch from 7fc1888 to c5132e7 Compare May 17, 2026 23:38
trivikr removed their assignment May 21, 2026
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 22, 2026

nodejs-github-bot commented May 22, 2026
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

jasnell added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels May 22, 2026
nodejs-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

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

trivikr 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-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2026
trivikr 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 merged commit 8d3245e into nodejs:main May 23, 2026
99 of 100 checks passed

Copy link
Copy Markdown
Collaborator

Landed in 8d3245e

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: #63360
Fixes: #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#63360
Fixes: 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: #64675
Fixes: #63359
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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: toWritable() duplicates writes when writeSync() returns false after accepting chunk

3 participants


Back | FazBrowse Home | New Git URL