Review requested:

• @nodejs/streams

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (4f24aff) to head (3c1376e).
⚠️ Report is 408 commits behind head on main.

@@           Coverage Diff            @@
##             main   #61098    +/-   ##
========================================
  Coverage   88.53%   88.53%            
========================================
  Files         703      704     +1     
  Lines      208546   209019   +473     
  Branches    40217    40351   +134     
========================================
+ Hits       184634   185061   +427     
- Misses      15926    15930     +4     
- Partials     7986     8028    +42     

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

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"
...

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

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.

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.

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!

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

Can you fix the commit messagec

@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!

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?

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

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

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?

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.