stream: fix legacy pipe error handling

[ronag]

Fixes: #35237

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

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

[ronag]

@nodejs/streams

[Trott]

Needs a rebase. Looks good to me, but I'm usually hesitant to 👍 a streams change without someone from @nodejs/streams 👍 it first.

[mcollina]

lgtm

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[github-actions]

Commit Queue failed

- Loading data for nodejs/node/pull/35257 ✔ Done loading data for nodejs/node/pull/35257 ----------------------------------- PR info ------------------------------------ Title stream: fix legacy pipe error handling (#35257) Author Robert Nagy (@ronag) Branch ronag:legacy-error-handling -> nodejs:master Labels lts-watch-v12.x, stream Commits 2 - stream: fix legacy pipe error handling - fixup Committers 1 - Robert Nagy PR-URL: https://github.com/nodejs/node/pull/35257 Fixes: https://github.com/nodejs/node/issues/35237 Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/35257 Fixes: https://github.com/nodejs/node/issues/35237 Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - fixup ✖ GitHub CI is still running ℹ Last Full PR CI on 2020-09-22T20:58:51Z: https://ci.nodejs.org/job/node-test-pull-request/33202/ - Querying data for job/node-test-pull-request/33202/ ✔ Build data downloaded ✔ Last Jenkins CI successful ℹ This PR was created on Fri, 18 Sep 2020 13:39:55 GMT ✔ Approvals: 2 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/35257#pullrequestreview-493519435 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/35257#pullrequestreview-493754259 -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

[ronag]

@mcollina do you know if the commit queue auto squashes?

[Trott]

Landed in 6be80e1

[lundibundi]

@ronag no it doesn't, it will only autosquash if used with --fixup git command
see #34969 (comment) for more information.

I've implemented nodejs/node-core-utils#490 but it is not yet integrated into this repo.
(also - #34770)

[MylesBorins]

This doesn't land cleanly on v14.x

@ronag would you be able to backport?

[richardlau]

This has a lts-watch-v12.x label on but doesn't cherry-pick cleanly onto v12.x-staging so will require a manual backport. FWIW the test currently fails on v12.x-staging:

$ ./out/Release/node test/parallel/test-stream-pipe-error-handling.js internal/streams/legacy.js:61 throw er; // Unhandled stream error in pipe. ^ Error: this should be handled at Object.<anonymous> (/home/rlau/sandbox/github/trees/v12.x-staging/test/parallel/test-stream-pipe-error-handling.js:113:16) at Module._compile (internal/modules/cjs/loader.js:999:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10) at Module.load (internal/modules/cjs/loader.js:863:32) at Function.Module._load (internal/modules/cjs/loader.js:708:14) at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12) at internal/main/run_main_module.js:17:47 $