http2: fix client async storage persistence

[orgads]

Create and store an AsyncResource for each stream, following a similar approach as used in HttpAgent.

Fixes #55376

[nodejs-github-bot]

Review requested:

• @nodejs/http2
• @nodejs/net

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.42%. Comparing base (53b1050) to head (ecc25ca).
Report is 34 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #55460   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files         654      654           
  Lines      187552   187561    +9     
  Branches    36087    36090    +3     
=======================================
+ Hits       165839   165854   +15     
- Misses      14950    14956    +6     
+ Partials     6763     6751   -12     

Files with missing lines	Coverage Δ
lib/internal/http2/core.js	95.53% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

[orgads]

I'll try to improve it and use internals instead of AsyncResource.

[orgads]

I didn't find a better solution. Open for suggestions :)

[orgads]

@mcollina Any comments?

[mcollina]

lgtm

However I'm concerned that this will add overhead.

[orgads]

Are headers received only once per stream? If they are, we can unset the resource after using it in onSessionHeaders.

[orgads]

Renamed asyncRes to reqAsync for consistency.

[orgads]

The failures are unrelated to my changes.
https://ci.nodejs.org/job/node-test-commit-arm/nodes=rhel8-arm64/55481/console
This one is unclear. I saw a similar timeout in another test on the previous run of the same configuration.

11:01:35 not ok 4121 parallel/test-stream-readable-unpipe-resume
11:01:35   ---
11:01:35   duration_ms: 360017.24500
11:01:35   severity: fail
11:01:35   exitcode: -15
11:01:35   stack: |-
11:01:35     timeout
11:01:35   ...

https://ci.nodejs.org/job/node-test-commit-osx-arm/17293/nodes=osx11/console
I didn't touch http, only http2.

11:00:21 not ok 4213 sequential/test-http-server-request-timeouts-mixed
11:00:21   ---
11:00:21   duration_ms: 3218.23100
11:00:21   severity: fail
11:00:21   exitcode: 1
11:00:21   stack: |-
11:00:21     node:internal/assert/utils:281
11:00:21         throw err;
11:00:21         ^
11:00:21     
11:00:21     AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
11:00:21     
11:00:21       assert(request2.completed)
11:00:21     
11:00:21         at Timeout._onTimeout (/Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/test/sequential/test-http-server-request-timeouts-mixed.js:108:5)
11:00:21         at listOnTimeout (node:internal/timers:614:17)
11:00:21         at process.processTimers (node:internal/timers:549:7) {
11:00:21       generatedMessage: true,
11:00:21       code: 'ERR_ASSERTION',
11:00:21       actual: false,
11:00:21       expected: true,
11:00:21       operator: '=='
11:00:21     }

Is it possible to retrigger these?

[orgads]

Rebased.

[orgads]

@lpinca Can you please trigger CI again?

[orgads]

@lpinca the failures look random and unrelated to my changes. How do we proceed from here?

[nodejs-github-bot]

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

[orgads]

Passed! Thank you.

[orgads]

How do we proceed from here?

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/55460
✔  Done loading data for nodejs/node/pull/55460
----------------------------------- PR info ------------------------------------
Title      http2: fix client async storage persistence (#55460)
Author     Orgad Shaneh <orgads@gmail.com> (@orgads)
Branch     orgads:http2-async -> nodejs:main
Labels     http2, needs-ci
Commits    1
 - http2: fix client async storage persistence
Committers 1
 - Orgad Shaneh <orgad.shaneh@audiocodes.com>
PR-URL: https://github.com/nodejs/node/pull/55460
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55460
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - http2: fix client async storage persistence
   ℹ  This PR was created on Sat, 19 Oct 2024 20:46:33 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55460#pullrequestreview-2380687991
   ✔  - Stephen Belanger (@Qard): https://github.com/nodejs/node/pull/55460#pullrequestreview-2384391523
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/55460#pullrequestreview-2382123844
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-27T05:45:51Z: https://ci.nodejs.org/job/node-test-pull-request/63307/
- Querying data for job/node-test-pull-request/63307/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

[nodejs-github-bot]

Landed in f67e45e

[orgads]

Thank you all! What's the process for backporting it to 22.x?

[Flarna]

@orgads Usually this goes automatically. First it will land on 23 and later on 22.

In case cherry-picking this commit from main to the release branch fails for some reason a label + comment will be added here indicating it requires a dedicated manual created backport PR.

[orgads]

Thank you.