http: onFinish will not be triggered again when finished

[rickyes]

When the .end() is called with the payload again after the end, directly return and pass the ERR_STREAM_WRITE_AFTER_END error to the callback and error event.

Fixes: #35833

/cc @ronag@kaizhu256

Checklist

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

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[kaizhu256]

verified latest pr-commit 6e1c697 fixed regression #35833 in windows (using git-bash):

#!/bin/sh
./out/Release/node.exe -e 'require("http").createServer(function (ignore, res) { res.on("error", function () { return; }); res.end("cc"); res.end("dd");}).listen(8081, function () { require("http").request("http://localhost:8081").end();});setTimeout(process.exit, 5000);'# stderr output - none

[rickyes]

@nodejs/http @ronag@mcollina PTAL, thanks~

[ronag]

Can't this be solved just by putting the if (chunk) branch after the if (this.finished) branch?

[rickyes]

Can't this be solved just by putting the if (chunk) branch after the if (this.finished) branch?

@ronag Thanks for you comment, that's what I thought at first, but I realized that this would behave differently than the v14 version, with two problems：

1. The error code returned when with payload is ERR_STREAM_ALREADY_FINISHED instead of ERR_STREAM_WRITE_AFTER_END
2. When with payload, only the callback is triggered without sending the error event.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[github-actions]

Commit Queue failed

- Loading data for nodejs/node/pull/35845
✔ Done loading data for nodejs/node/pull/35845
----------------------------------- PR info ------------------------------------
Title http: onFinish will not be triggered again when finished (#35845)
Author Ricky Zhou <0x19951125@gmail.com> (@rickyes)
Branch rickyes:fix/http-outgoing-end -> nodejs:master
Labels author ready, commit-queue, dont-land-on-v10.x, dont-land-on-v12.x, dont-land-on-v14.x, http
Commits 5
- http: OnFinish will not be triggered again when finished
- fixup
- fixup
- fixup! got error event before error callback
- fixup! callback order align with stream.Writable
Committers 1
- rickyes <0x19951125@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35845
Fixes: https://github.com/nodejs/node/issues/35833
Reviewed-By: Robert Nagy Reviewed-By: Matteo Collina Reviewed-By: Rich Trott ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/35845
Fixes: https://github.com/nodejs/node/issues/35833
Reviewed-By: Robert Nagy Reviewed-By: Matteo Collina Reviewed-By: Rich Trott --------------------------------------------------------------------------------
✖ Last GitHub CI failed
ℹ Last Full PR CI on 2020-11-12T01:40:04Z: https://ci.nodejs.org/job/node-test-pull-request/34349/
- Querying data for job/node-test-pull-request/34349/
✔ Build data downloaded
✔ Last Jenkins CI successful
ℹ This PR was created on Wed, 28 Oct 2020 07:21:43 GMT
✔ Approvals: 3
✔ - Robert Nagy (@ronag): https://github.com/nodejs/node/pull/35845#pullrequestreview-526035746
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/35845#pullrequestreview-525944547
✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/35845#pullrequestreview-529123959
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

Commit Queue action: https://github.com/nodejs/node/actions/runs/360068464

[rickyes]

Landed in e6e64f7