Skip to content

fix(http2): preserve accepted streams after GOAWAY#5090

Merged
metcoder95 merged 11 commits into
nodejs:mainfrom
trivikr:http2-graceful-goaway
Apr 26, 2026
Merged

fix(http2): preserve accepted streams after GOAWAY#5090
metcoder95 merged 11 commits into
nodejs:mainfrom
trivikr:http2-graceful-goaway

Conversation

@trivikr

@trivikr trivikr commented Apr 22, 2026

Copy link
Copy Markdown
Member

This relates to...

Fixes: #5089

Rationale

HTTP/2 GOAWAY with NO_ERROR should not fail streams the server has already accepted.

This change preserves in-flight accepted streams, retries work beyond lastStreamID, and updates the tests to cover the intended graceful shutdown behavior.

Changes

  • Track request stream IDs in the HTTP/2 client
  • Treat GOAWAY with code 0 as informational rather than a socket failure
  • Only retire/requeue requests whose stream IDs are beyond lastStreamID
  • Allow already-accepted streams to complete normally after GOAWAY

Features

N/A

Bug Fixes

  • Fix handling of HTTP/2 GOAWAY so accepted streams are not failed prematurely
  • Fix client reconnection behavior after graceful GOAWAY

Breaking Changes and Deprecations

N/A

Status

Assisted-by: openai:gpt-5.4

Assisted-by: openai:gpt-5.4
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Apr 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.47312% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.13%. Comparing base (2a6f9c7) to head (53376f2).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
lib/dispatcher/client-h2.js 92.47% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5090      +/-   ##
==========================================
- Coverage   93.13%   93.13%   -0.01%     
==========================================
  Files         110      110              
  Lines       35816    36090     +274     
==========================================
+ Hits        33356    33611     +255     
- Misses       2460     2479      +19     

☔ View full report in Codecov by Sentry.
📢 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.

Comment thread lib/dispatcher/client-h2.js
Comment thread lib/dispatcher/client-h2.js Outdated
Comment thread lib/dispatcher/client-h2.js
@trivikr trivikr requested review from mcollina and metcoder95 April 25, 2026 02:23

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@metcoder95 metcoder95 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work! lgtm

@metcoder95 metcoder95 merged commit 5878f54 into nodejs:main Apr 26, 2026
60 of 61 checks passed
@trivikr trivikr deleted the http2-graceful-goaway branch April 28, 2026 03:45
@github-actions github-actions Bot mentioned this pull request May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Graceful http2 GOAWAY is handled as hard connection failure

4 participants