Skip to content

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Jan 24, 2026

Summary

  • Fixes flaky Dispatcher#Connect test on macOS in test/http2-dispatcher.js
  • The 'end' event handler was incorrectly treating stream state 6 (HALF_CLOSED_REMOTE) as an error based on timing-dependent state checks
  • Added a responseReceived flag to properly distinguish between normal completion and error cases

Problem

The test was failing intermittently on macOS with:

Error [InformationalError]: HTTP/2: stream half-closed (remote)

The root cause was a stream state check stream.state.state < 6 that excluded state 6 (HALF_CLOSED_REMOTE). However, this is a valid state when the server finishes sending its response. Timing differences across platforms caused the stream to be in state 6 when 'end' fired, triggering a false error.

Fix

Instead of checking stream state (which is timing-dependent), track whether we received a response:

  • Added responseReceived flag set to true when 'response' event fires
  • In 'end' handler: if response was received → complete normally
  • If no response was received (e.g., server destroyed stream before sending headers) → throw the "half-closed" error

This properly differentiates between:

  1. Normal completion: Server sends response then closes stream
  2. Error case: Server destroys stream without sending a response

Test plan

  • Dispatcher#Connect test passes (the originally flaky test)
  • Should throw informational error on half-closed streams (remote) passes (error case still works)
  • All H2 tests pass locally
The 'end' event handler in client-h2.js was incorrectly treating stream
state 6 (HALF_CLOSED_REMOTE) as an error condition. This state is the
expected state when the peer sends END_STREAM, per the HTTP/2 spec.

The timing of when the stream reaches this state varies across platforms,
causing the Dispatcher#Connect test in test/http2-dispatcher.js to fail
intermittently on macOS with "HTTP/2: stream half-closed (remote)" error.

This commit simplifies the 'end' event handler by:
- Removing the unnecessary stream state check
- Removing the duplicate kOpenStreams decrement (already handled in 'close')
- Always completing the request normally when 'end' fires

The stream cleanup is properly handled by the 'close' event handler,
which decrements kOpenStreams and manages session references.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina force-pushed the fix/h2-stream-end-state branch from 3fc819f to a5d2c02 Compare January 24, 2026 07:51
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.26%. Comparing base (4920fc4) to head (a5d2c02).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4762      +/-   ##
==========================================
- Coverage   93.26%   93.26%   -0.01%     
==========================================
  Files         109      109              
  Lines       34029    34023       -6     
==========================================
- Hits        31738    31732       -6     
  Misses       2291     2291              

☔ 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 changed the title fix(h2): remove incorrect stream state check in end handler Jan 24, 2026
@metcoder95 metcoder95 merged commit 94d147d into main Jan 25, 2026
36 of 37 checks passed
@github-actions github-actions bot mentioned this pull request Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants