Skip to content

fix: reset invalid HTTP/2 sessions#5310

Open
mcollina wants to merge 2 commits into
mainfrom
fix/h2-invalid-session-reset
Open

fix: reset invalid HTTP/2 sessions#5310
mcollina wants to merge 2 commits into
mainfrom
fix/h2-invalid-session-reset

Conversation

@mcollina
Copy link
Copy Markdown
Member

@mcollina mcollina commented May 18, 2026

Summary

  • reset cached HTTP/2 client session/context/socket state when session.request() throws ERR_HTTP2_INVALID_SESSION
  • requeue the unsent request locally in the HTTP/2 client so it can be sent on a fresh session
  • add regression coverage for invalid sessions and server-truncated HTTP/2 sessions

Tests

  • npx borp -p "test/http2-invalid-session.js"
  • npx borp -p "test/http2-invalid-connection-headers.js"
  • npx borp -p "test/http2-*.js"
  • npm run lint -- test/http2-invalid-session.js lib/dispatcher/client-h2.js

@mcollina mcollina force-pushed the fix/h2-invalid-session-reset branch from f06dad1 to 8372e6f Compare May 18, 2026 13:04
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 18, 2026

Codecov Report

❌ Patch coverage is 97.82609% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.22%. Comparing base (f14c445) to head (9cb1a59).

Files with missing lines Patch % Lines
lib/dispatcher/client-h2.js 97.82% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5310      +/-   ##
==========================================
- Coverage   93.23%   93.22%   -0.01%     
==========================================
  Files         110      110              
  Lines       36599    36636      +37     
==========================================
+ Hits        34122    34155      +33     
- Misses       2477     2481       +4     

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

@mcollina mcollina force-pushed the fix/h2-invalid-session-reset branch from 8372e6f to 209d424 Compare May 18, 2026 13:30
@mcollina mcollina requested a review from trivikr May 18, 2026 14:24
Comment thread test/http2-invalid-session.js Outdated
} = require('../lib/core/symbols')

test('invalid HTTP/2 session is reset instead of throwing', async (t) => {
t = tspl(t, { plan: 10 })
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.

Should we reduce usage of tspl since node 20+ has t.plan?

Comment thread test/http2-invalid-session.js Outdated
return session
}

after(() => {
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.

Should this be context.after?

@mcollina mcollina force-pushed the fix/h2-invalid-session-reset branch from 209d424 to 939084a Compare May 18, 2026 14:36
@mcollina mcollina force-pushed the fix/h2-invalid-session-reset branch from 939084a to 384612e Compare May 18, 2026 16:22
@metcoder95
Copy link
Copy Markdown
Member

lint seems unhappy

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.

4 participants