fix(h2): retire the request that completed, not the head of the queue

[mcollina]

Backport of #5410, with the in-order fast path from #5569. Neither is on v7.x — the h2 queue commits after #4881 never came across.

The bug

HTTP/2 completes out of order, but every completion site advanced the running index blindly:

client[kQueue][client[kRunningIdx]++] = null

That retires whichever request happens to sit at the head, not the one that actually finished. With two streams in flight (/first never answered, /second and /third served):

                 before                          after this change
after /second    queue=[null, "/second"]         queue=["/first"]
                 runningIdx=1 pendingIdx=2       runningIdx=0 pendingIdx=1
after /third     queue=[null, null, "/third"]    queue=["/first"]
                 runningIdx=2 pendingIdx=3       runningIdx=0 pendingIdx=1

The still-running /first has been dropped from the queue, and two finished requests are counted as running for good. kRunning never returns to zero, and since _resume() stops dispatching once kRunning reaches the concurrency limit, a client that accumulates these eventually stops sending anything at all.

The change

completeRequest() ported from main: retire the request by identity, keeping the O(1) fast path when it is the head, and splice it out when it finished out of order. Cleared slots can now appear in the queue, so the paths that walk it skip them — the same if (request != null) guards #5410 added to client.js, which cherry-picked cleanly.

Reachability on v7.x

Worth knowing for triage: v7 only dispatches h2 requests concurrently up to pipelining, which defaults to 1, so the out-of-order case needs pipelining (or a pool where several requests share a client) to show up. main added getMaxConcurrent() to dispatch up to maxConcurrentStreams for h2, which is why this bites much harder there. The ported test therefore sets pipelining: 2 — without it, /second and /third simply queue behind /first and never run.

Tests

test/issue-5404.js backported from #5410, plus pipelining: 2. Fails on the current branch (queue is [null, null, '/third'] instead of ['/first']), passes with this change.

test/+(http2|h2)*.js: 50 pass. Full unit suite: 1304 pass, 0 fail.

A seeded chaos harness against v7 currently aborts within a round or two of starting, reporting requests left in flight with nothing outstanding. With this change it no longer drifts; stacked with #5607 it runs all 25 rounds clean (~200 requests per seed, stuck=0) on every seed tried.

Relationship to the other v7 PRs

Independent of #5604, #5605 and #5607 — different bugs, no overlapping hunks. #5607 is the one that makes the harness above reach stuck=0; this one is what stops the queue drifting.

🤖 Generated with Claude Code

https://claude.ai/code/session_01A49JamgF2TkZHu5h58ChUM

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.15%. Comparing base (8d347dd) to head (68ca790).
⚠️ Report is 2 commits behind head on v7.x.

Additional details and impacted files

@@            Coverage Diff             @@
##             v7.x    #5618      +/-   ##
==========================================
- Coverage   93.17%   93.15%   -0.02%     
==========================================
  Files         112      112              
  Lines       36751    36802      +51     
==========================================
+ Hits        34241    34283      +42     
- Misses       2510     2519       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.