streams: forward errors correctly for duplexPair endpoints by aelhor · Pull Request #61098 · nodejs/node

aelhor · 2025-12-17T17:25:57Z

fix the duplexPair implementation so that when one side is destroyed with an error, the other side also receives the error or a close event as appropriate.

previous behavior caused sideA to never emit an 'error' or 'close' when sideB errored, which prevented users from observing or handling the paired stream failure.

Fixes: #61015

nodejs-github-bot · 2025-12-17T17:26:02Z

Review requested:

@nodejs/streams

mcollina

lgtm

codecov · 2025-12-19T01:13:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.67%. Comparing base (e47df44) to head (d78ccb3).
⚠️ Report is 179 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61098      +/-   ##
==========================================
- Coverage   89.68%   89.67%   -0.02%     
==========================================
  Files         676      676              
  Lines      206451   206475      +24     
  Branches    39523    39531       +8     
==========================================
- Hits       185161   185156       -5     
- Misses      13432    13455      +23     
- Partials     7858     7864       +6

Files with missing lines	Coverage Δ
lib/internal/streams/duplexpair.js	`96.51% <100.00%> (+1.35%)`	⬆️

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

nodejs-github-bot · 2025-12-19T13:02:49Z

CI: https://ci.nodejs.org/job/node-test-pull-request/70509/

mcollina · 2025-12-19T16:36:31Z

This is a relevant fail:

---
duration_ms: 114.266
exitcode: 1
severity: fail
stack: "node:events:486\n      throw er; // Unhandled 'error' event\n      ^\n\nError:\
  \ sometimes the code just doesn\u2019t work\n    at DuplexSide.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-http-sync-write-error-during-continue.js:46:26)\n\
  \    at DuplexSide._write (/home/iojs/build/workspace/node-test-commit-linuxone/test/common/index.js:506:15)\n\
  \    at doWrite (node:internal/streams/writable:596:12)\n    at clearBuffer (node:internal/streams/writable:781:7)\n\
  \    at Writable.uncork (node:internal/streams/writable:529:7)\n    at ClientRequest.end\
  \ (node:_http_outgoing:1090:19)\n    at ClientRequest.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-http-sync-write-error-during-continue.js:49:9)\n\
  \    at ClientRequest.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/common/index.js:506:15)\n\
  \    at ClientRequest.emit (node:events:508:20)\n    at HTTPParser.parserOnIncomingClient\
  \ [as onIncoming] (node:_http_client:723:11)\nEmitted 'error' event on DuplexSide\
  \ instance at:\n    at emitErrorNT (node:internal/streams/destroy:170:8)\n    at\
  \ emitErrorCloseNT (node:internal/streams/destroy:129:3)\n    at process.processTicksAndRejections\
  \ (node:internal/process/task_queues:89:21)\n\nNode.js v26.0.0-pre"
...

Renegade334 · 2025-12-20T18:36:03Z

This would also be semver-major, unless hidden behind an option.

aelhor · 2025-12-22T13:40:16Z

Thanks for flagging this.
I see the failure in test-http-sync-write-error-during-continue.js, and I’ll investigate the regression caused by this change and report back.

test/parallel/test-duplex-error.js

#61098 (comment)

aelhor · 2026-01-02T19:11:24Z

I have added a new commit that addresses the review comments and fixes the regression in test-http-sync-write-error-during-continue.js.

I used process.nextTick to ensure destruction happens after the current execution stack, and I am only propagating the destruction signal (not the error object) to avoid breaking existing code that lacks error listeners on both sides of the pair. All tests and lints now pass locally.

aelhor · 2026-01-19T21:09:04Z

Hi @lpinca @mcollina @Renegade334 👋
Just a gentle ping in case this PR slipped through the cracks.
No rush at all — I’d really appreciate any feedback or guidance when you have time.
Thanks!

mcollina

lgtm

nodejs-github-bot · 2026-01-19T21:52:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/70896/

mcollina · 2026-01-20T19:24:53Z

Can you fix the commit messagec

aelhor · 2026-01-21T15:25:17Z

@mcollina
I have updated the commit messages to use the correct stream: prefix and squashed the review suggestions and lint fixes into the main commits to keep the history clean. Ready for another look!

mcollina

lgtm

aelhor · 2026-02-08T14:48:30Z

I have reviewed the CI logs for the current failures:

AIX failure is in test-watch-mode-restart-esm-loading-error.mjs
Linux/Alpine failures are in test-inspector-network-http2-compressed.js

These appear to be unrelated flaky tests. My new test (test-stream-duplexpair-destroy.js) and the regression test (test-http-sync-write-error-during-continue.js) passed successfully across all platforms.

Is it possible to get a re-ci to confirm a clean pass?

nodejs-github-bot · 2026-02-08T15:49:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/71252/

nodejs-github-bot · 2026-02-08T23:18:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/71264/

aelhor · 2026-02-10T21:54:19Z

CI is now green after the latest fix. Since this addresses the previous failure, would it make sense to re-evaluate the semver-major label to see if it’s still applicable?

Renegade334 · 2026-02-10T22:03:57Z

If this no longer triggers errors on the other side then it should be semver-minor. It'd be helpful if you squashed down your commits so that stream: propagate destruction in duplexPair was the preserved commit message in the changelog.

mcollina

lgtm

gurgunday

lgtm

Ensure destroying one side of a duplexPair triggers destruction of the other side via process.nextTick(). Only the destruction signal is sent to avoid breaking changes. Fixes: nodejs#61015

nodejs-github-bot · 2026-03-11T23:23:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/71707/

aelhor · 2026-04-01T17:39:35Z

This failure appears to be a flake in test-debugger-exceptions (OSX) and a potential timeout on Windows ARM64, as the code remains unchanged from the previous green CI run. Can we trigger a re-run to confirm?

nodejs-github-bot · 2026-04-07T08:52:08Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72529/

StefanStojanovic · 2026-04-07T11:11:54Z

I've retrigged CI and all passed, reran the failed GHA as well.

nodejs-github-bot · 2026-04-07T17:38:27Z

Commit Queue failed

- Loading data for nodejs/node/pull/61098
✔  Done loading data for nodejs/node/pull/61098
----------------------------------- PR info ------------------------------------
Title      streams: forward errors correctly for duplexPair endpoints (#61098)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aelhor:streams/fix-duplexpair -> nodejs:main
Labels     stream, semver-minor, needs-ci
Commits    1
 - stream: propagate destruction in duplexPair
Committers 1
 - Ahmed Elhor <aelhor90@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/61098
Fixes: https://github.com/nodejs/node/issues/61015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/61098
Fixes: https://github.com/nodejs/node/issues/61015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 17 Dec 2025 17:25:57 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/61098#pullrequestreview-3800111566
   ✔  - Gürgün Dayıoğlu (@gurgunday): https://github.com/nodejs/node/pull/61098#pullrequestreview-3805285530
   ✔  - René (@Renegade334): https://github.com/nodejs/node/pull/61098#pullrequestreview-3933061949
   ✘  21 GitHub CI job(s) cancelled:
   ✘    - coverage-windows: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778707/job/66711491167)
   ✘    - test-linux (ubuntu-24.04): CANCELLED (https://github.com/nodejs/node/actions/runs/22977778708/job/66711491363)
   ✘    - test-linux (ubuntu-24.04-arm): CANCELLED (https://github.com/nodejs/node/actions/runs/22977778708/job/66711491425)
   ✘    - lint-js-and-md: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492465)
   ✘    - lint-cpp: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492466)
   ✘    - lint-codeowners: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492469)
   ✘    - lint-nix: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492474)
   ✘    - lint-readme: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492475)
   ✘    - lint-yaml: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492476)
   ✘    - lint-pr-url: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492477)
   ✘    - lint-py: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492478)
   ✘    - format-cpp: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492480)
   ✘    - lint-sh: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492496)
   ✘    - lint-addon-docs: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778723/job/66711492502)
   ✘    - coverage-linux: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778719/job/66711491665)
   ✘    - coverage-linux-without-intl: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778724/job/66711492826)
   ✘    - build-tarball: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778725/job/66711491865)
   ✘    - test-tarball-linux: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778725/job/66711495004)
   ✘    - build-docs: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778738/job/66711494836)
   ✘    - Build slim tarball: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778739/job/66711493504)
   ✘    - ${{ matrix.system }}: with shared libraries: CANCELLED (https://github.com/nodejs/node/actions/runs/22977778739/job/66711497168)
   ℹ  Last Full PR CI on 2026-04-07T08:52:08Z: https://ci.nodejs.org/job/node-test-pull-request/72529/
- Querying data for job/node-test-pull-request/72529/
✔  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/24095491560

Renegade334 · 2026-04-07T17:52:25Z

Landed in 68e5f87

nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Dec 17, 2025

mcollina approved these changes Dec 18, 2025

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 18, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025

mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Dec 22, 2025

lpinca previously approved these changes Dec 24, 2025

View reviewed changes

test/parallel/test-duplex-error.js Outdated Show resolved Hide resolved

test/parallel/test-duplex-error.js Outdated Show resolved Hide resolved

test/parallel/test-duplex-error.js Outdated Show resolved Hide resolved

test/parallel/test-duplex-error.js Outdated Show resolved Hide resolved

mcollina approved these changes Jan 19, 2026

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2026

aelhor force-pushed the streams/fix-duplexpair branch from 61f1443 to 3c1376e Compare January 21, 2026 15:22

mcollina approved these changes Jan 22, 2026

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 22, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 22, 2026

This comment has been minimized.

Sign in to view

mcollina approved these changes Feb 13, 2026

View reviewed changes

mcollina added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Feb 13, 2026

gurgunday approved these changes Feb 15, 2026

View reviewed changes

aelhor closed this Mar 11, 2026

aelhor force-pushed the streams/fix-duplexpair branch from 3c1376e to e57d5bb Compare March 11, 2026 21:04

stream: propagate destruction in duplexPair

d78ccb3

Ensure destroying one side of a duplexPair triggers destruction of the other side via process.nextTick(). Only the destruction signal is sent to avoid breaking changes. Fixes: nodejs#61015

aelhor reopened this Mar 11, 2026

Renegade334 approved these changes Mar 11, 2026

View reviewed changes

Renegade334 added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 11, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 11, 2026

Renegade334 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 7, 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 Apr 7, 2026

Renegade334 merged commit 68e5f87 into nodejs:main Apr 7, 2026
72 of 96 checks passed

Renegade334 removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 7, 2026

Uh oh!

Conversation

aelhor commented Dec 17, 2025

Uh oh!

nodejs-github-bot commented Dec 17, 2025

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Dec 19, 2025

Uh oh!

mcollina commented Dec 19, 2025

Uh oh!

Renegade334 commented Dec 20, 2025

Uh oh!

aelhor commented Dec 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aelhor commented Jan 2, 2026

Uh oh!

aelhor commented Jan 19, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jan 19, 2026

Uh oh!

mcollina commented Jan 20, 2026

Uh oh!

aelhor commented Jan 21, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

aelhor commented Feb 8, 2026

Uh oh!

nodejs-github-bot commented Feb 8, 2026

Uh oh!

nodejs-github-bot commented Feb 8, 2026

Uh oh!

aelhor commented Feb 10, 2026

Uh oh!

Renegade334 commented Feb 10, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

gurgunday left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 11, 2026

Uh oh!

aelhor commented Apr 1, 2026

Uh oh!

nodejs-github-bot commented Apr 7, 2026

Uh oh!

StefanStojanovic commented Apr 7, 2026

Uh oh!

nodejs-github-bot commented Apr 7, 2026

Uh oh!

Uh oh!

Renegade334 commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented Dec 19, 2025 •

edited

Loading