[emerzon]

Keep the native session close pending when an active socket write prevents the terminal GOAWAY from being serialized. Resume shutdown after the write completes, preserving the requested HTTP/2 error code. On write failure, finish shutdown without retrying protocol writes and retain the listener needed to observe socket closure.

Add a deterministic regression using a controlled Duplex transport. It covers destruction inside and outside the receive callback with default and custom GOAWAY codes.

Validation: 296 HTTP/2 tests pass. The new regression and the existing TLS session reuse test each pass 50 repeated runs. The issue's original reproducer changes from sawGoaway=false to sawGoaway=true while the earlier write is still active. JavaScript and C++ lint and import checks pass.

Two local connection tests (test-http2-client-port-80 and test-http2-connect) also hang on unmodified HEAD; they were excluded from the successful 296-test run. Full Node.js CI remains pending.

Fixes: #66832

AI assistance: Codex assisted with implementation, testing, and this draft.

[nodejs-github-bot]

Review requested:

• @nodejs/http2
• @nodejs/net

[github-actions]

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

• the guide for first-time contributors
• the contribution and automation policies
• the pull request guide
• the AI use policy
• the Code of Conduct

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

[inoway46]

LGTM based on local testing. I applied this patch on top of main (5b5f384) and built it on macOS x64. All 295 HTTP/2 parallel tests passed.

This also fixes the file-serving case I reported: GOAWAY is now received in 3/3 runs each with session.destroy() and session.destroy(new Error(...)).

One non-blocking suggestion: could the regression test also cover the held write completing with an error? I tested this locally and shutdown completed without further writes, but adding coverage would help protect the new error-handling path.

[inoway46]

@nodejs/http2 @nodejs/net Could someone review and run CI for this PR?

[mcollina]

Linter CI is red

[emerzon]

Fixed in emerzon@63a17e0. The failure was parameter alignment in Http2Session::FinishClose. I ran the repository’s formatter across the full PR range; it now reports no further changes. C++ lint and import checks also pass locally.

[mcollina]

lgtm

[codecov]

Codecov Report

❌ Patch coverage is 76.19048% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.70%. Comparing base (e463b48) to head (63a17e0).
⚠️ Report is 227 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_http2.cc	76.19%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66834      +/-   ##
==========================================
- Coverage   92.85%   90.70%   -2.15%     
==========================================
  Files         428      797     +369     
  Lines      194833   278891   +84058     
  Branches    30108    53629   +23521     
==========================================
+ Hits       180907   252977   +72070     
- Misses      13599    16269    +2670     
- Partials      327     9645    +9318     

Files with missing lines	Coverage Δ
src/node_http2.h	92.47% <ø> (ø)
src/node_http2.cc	81.97% <76.19%> (ø)

... and 521 files with indirect coverage changes

🚀 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.

[nodejs-github-bot]

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