WebSocket and WebTransport support#72
Conversation
Allow WebSocket and WebTransport connections to be established using the Happy Eyeballs algorithm, racing various transport options. #22
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
- Coverage 97.03% 95.07% -1.97%
==========================================
Files 2 2
Lines 776 873 +97
Branches 776 873 +97
==========================================
+ Hits 753 830 +77
- Misses 21 41 +20
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Summary
Clean, well-structured extension of the state machine to drive WebSocket and WebTransport session negotiation on top of Happy Eyeballs racing. The multi-phase flow (transport → settings → session) is a natural fit for the existing Input/Output architecture, and the spec references (RFC 8441, RFC 9220, draft-ietf-webtrans-http3/http2) are thorough.
⚠️ Transport cleanup gap in establish_protocol_session
src/lib.rs — establish_protocol_session, the else branch (H2/H3 settings not supported) and the Session::WebTransport over H1 branch
When settings indicate the server doesn't support the requested session, establish_protocol_session transitions the connection from Connected to Failed without emitting CancelConnection. The caller opened a transport for this connection (it received AttemptConnection and reported ConnectionResult::Success), and is now waiting for either EstablishProtocolSession or CancelConnection — neither arrives.
If a different connection later succeeds, cancel_remaining_attempts skips this one because it's already in a terminal state (Failed). The caller is left with an open transport it doesn't know to close.
Consider emitting CancelConnection for these connections instead of silently marking them Failed, or documenting in the Output contract that the caller must close all transports upon receiving Succeeded/Failed.
📝 ProtocolSessionResult::Failure semantics
src/lib.rs — on_protocol_session_result, the Failure match arm
ProtocolSessionResult::Failure unconditionally sets protocol_not_supported = true, but its doc says "the server rejected the WebSocket upgrade … or the handshake failed for another reason". A server that supports Extended CONNECT but rejects a specific request (wrong subprotocol, auth failure, resource limits) is semantically different from one that doesn't support the protocol at all.
This conflation means FailureReason::ProtocolNotSupported can be reported even when the protocol is supported — just the particular session was rejected. Consider either splitting the variant (e.g. SessionRejected vs ProtocolNotSupported) or renaming the flag to something broader like session_failed.
📝 Breaking public API change
src/lib.rs — ConnectionResult::Success
ConnectionResult::Success changed from a unit variant to Success { http_version: HttpVersion }. Downstream consumers matching ConnectionResult::Success (without the field) will no longer compile. Worth calling out in the PR description or a changelog entry so integrators know to update their match arms.
💡 Test coverage suggestions
tests/session.rs
Good coverage of the core flows. A few scenarios that would strengthen confidence:
- WebTransport over H2 happy path (only H3 is tested today).
- Settings-failure + second-connection-succeeds: one connection gets
Connected→ settings without Extended CONNECT → silentlyFailed, while a second connection fully succeeds. This exercises theCancelConnectiongap described above. - Events after cancellation: feed
HttpSettingsorProtocolSessionResultfor a connection that was already cancelled, to cover theCancelledbranches inon_http_settings/on_protocol_session_result.
Allow WebSocket and WebTransport connections to be established using the Happy Eyeballs algorithm, racing various transport options.
#22