http2: fix no response event on continue request

[ofirbarak]

When sending a continue request, server response with null, but it does not fires the response event type

Fixes: #38258
Ref: #38561

[nodejs-github-bot]

Review requested:

• @nodejs/http2
• @nodejs/net

[benjamingr]

Hey @ofirbarak thanks for tackling this, I am not sure the fix is quite correct - I see there are some relevant test failures?

[nodejs-github-bot]

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

[mcollina]

lgtm

[Trott]

Welcome, @ofirbarak and thanks for this pull request as well! As with the other one, please change the start of the first commit message from lib: to http2:. Thanks!

If another collaborator wants to land this before that change, feel free to dismiss this review but please change the commit message while landing. Thanks!

[mcollina]

@jasnell could you take a look before we land?

commit message updated

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/41739 ✔ Done loading data for nodejs/node/pull/41739 ----------------------------------- PR info ------------------------------------ Title http2: fix no response event on continue request (#41739) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch ofirbarak:http2-null-response -> nodejs:master Labels http2 Commits 4 - http2: fix no response event on continue request - fix liniting issues - correct the fix - simplify condition Committers 1 - ofir PR-URL: https://github.com/nodejs/node/pull/41739 Fixes: https://github.com/nodejs/node/issues/38258 Refs: https://github.com/nodejs/node/pull/38561 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41739 Fixes: https://github.com/nodejs/node/issues/38258 Refs: https://github.com/nodejs/node/pull/38561 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 29 Jan 2022 00:22:36 GMT ✔ Approvals: 3 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41739#pullrequestreview-866996187 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41739#pullrequestreview-867008165 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41739#pullrequestreview-871911901 ⚠ GitHub cannot link the author of 'http2: fix no response event on continue request' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ⚠ GitHub cannot link the author of 'fix liniting issues' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ⚠ GitHub cannot link the author of 'correct the fix' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ⚠ GitHub cannot link the author of 'simplify condition' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-02-03T09:48:20Z: https://ci.nodejs.org/job/node-test-pull-request/42332/ - Querying data for job/node-test-pull-request/42332/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/master up to date... From https://github.com/nodejs/node * branch master -> FETCH_HEAD ✔ origin/master is now up-to-date - Downloading patch for 41739 From https://github.com/nodejs/node * branch refs/pull/41739/merge -> FETCH_HEAD ✔ Fetched commits as 28989b4103d7..91db9b4d8b22 -------------------------------------------------------------------------------- [master 69c13726d0] http2: fix no response event on continue request Author: ofir Date: Sat Jan 29 02:14:00 2022 +0200 2 files changed, 98 insertions(+), 55 deletions(-) rewrite test/parallel/test-http2-compat-expect-continue.js (85%) [master a05210f0c0] fix liniting issues Author: ofir Date: Sat Jan 29 03:11:06 2022 +0200 2 files changed, 2 insertions(+), 2 deletions(-) [master 9168f2a135] correct the fix Author: ofir Date: Sat Jan 29 13:39:06 2022 +0200 2 files changed, 4 insertions(+), 4 deletions(-) [master 02e3fa3a4e] simplify condition Author: ofir Date: Sun Jan 30 15:22:04 2022 +0200 1 file changed, 1 insertion(+), 4 deletions(-) ✔ Patches applied There are 4 commits in the PR. Attempting autorebase. Rebasing (2/8) 
Executing: git node land --amend --yes
 ⚠ Found Fixes: #38258, skipping..
 --------------------------------- New Message ----------------------------------
 http2: fix no response event on continue request
When sending a continue request, server response with null,
 it does not fires the response event type
Fixes: #38258
PR-URL: #41739
 Refs: #38561
 Reviewed-By: Benjamin Gruenbaum [email protected]
 Reviewed-By: Matteo Collina [email protected]
 Reviewed-By: James M Snell [email protected]
[detached HEAD 2c9b0aaa0b] http2: fix no response event on continue request
 Author: ofir [email protected]
 Date: Sat Jan 29 02:14:00 2022 +0200
 2 files changed, 98 insertions(+), 55 deletions(-)
 rewrite test/parallel/test-http2-compat-expect-continue.js (85%)
 Rebasing (3/8)
 Rebasing (4/8)
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 fix liniting issues
PR-URL: #41739
 Fixes: #38258
 Refs: #38561
 Reviewed-By: Benjamin Gruenbaum [email protected]
 Reviewed-By: Matteo Collina [email protected]
 Reviewed-By: James M Snell [email protected]
[detached HEAD 4ba5dec622] fix liniting issues
 Author: ofir [email protected]
 Date: Sat Jan 29 03:11:06 2022 +0200
 2 files changed, 2 insertions(+), 2 deletions(-)
 Rebasing (5/8)
 Rebasing (6/8)
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 correct the fix
PR-URL: #41739
 Fixes: #38258
 Refs: #38561
 Reviewed-By: Benjamin Gruenbaum [email protected]
 Reviewed-By: Matteo Collina [email protected]
 Reviewed-By: James M Snell [email protected]
[detached HEAD bdf250b737] correct the fix
 Author: ofir [email protected]
 Date: Sat Jan 29 13:39:06 2022 +0200
 2 files changed, 4 insertions(+), 4 deletions(-)
 Rebasing (7/8)
 Rebasing (8/8)
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 simplify condition
PR-URL: #41739
 Fixes: #38258
 Refs: #38561
 Reviewed-By: Benjamin Gruenbaum [email protected]
 Reviewed-By: Matteo Collina [email protected]
 Reviewed-By: James M Snell [email protected]
[detached HEAD c33f61e577] simplify condition
 Author: ofir [email protected]
 Date: Sun Jan 30 15:22:04 2022 +0200
 1 file changed, 1 insertion(+), 4 deletions(-)
Successfully rebased and updated refs/heads/master.
ℹ Use --fixupAll option, squash the PR manually or land the PR from the command line.

https://github.com/nodejs/node/actions/runs/1789949374

[nodejs-github-bot]

Landed in be6844d

[benjamingr]

Congrats on your first contribution to Node core @ofirbarak 🎉

[ofirbarak]

Thank you!