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 fix(h2): fix flaky stream end handling on macOS Jan 24, 2026
@metcoder95 metcoder95 merged commit 94d147d into main Jan 25, 2026
36 of 37 checks passed
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