feat(qmux): add server-side Session.accept() (JS)#324
Conversation
js/qmux's Session could only ever be a client: the protocol logic was already role-aware, but `#isServer` was never assigned and the socket was not injectable, so there was no way to build a server session from an accepted WebSocket. A JS accept server had no equivalent of the ws:// fallback moq-relay serves alongside WebTransport. Add `Session.accept(socket, options)`, mirroring the Rust crate's `qmux::ws::Server::accept`. It takes a plain WebSocket, a WebSocketStream, or anything WebSocketStreamLike, so Deno.upgradeWebSocket, `ws`, and Bun.serve all work. `new Session(url)` keeps the WebTransport constructor signature, so install() is unaffected. Supporting changes: - `WebSocketStream.adopt()` wraps a socket that already exists. An accepted socket is typically already OPEN, so `onopen` has already fired and waiting for it would hang; `opened` now settles from readyState. - `selectSubprotocol()` is the accept-side mirror of resolveSubprotocols. The host runs the handshake, so it picks the subprotocol; this expands the same protocols+versions pair and returns the first wire form the client offered. Preference is ours, not the client's, matching qmux::ws::Server. Returns undefined on no match so the caller fails the upgrade — accepting with no subprotocol silently negotiates the legacy `webtransport` wire format and misreads every QMux frame. - `AcceptOptions.protocol` stands in for hosts that don't report the negotiated subprotocol on the socket (Bun is one). Verified by a new interop-client Rust example, the mirror of the existing interop-server: a real Rust client drives a JS Session.accept server across qmux-00/01/02 — 1.2 MB each way with flow-control replenishment, bidi streams, datagrams, and a clean close. It runs in CI via `just test`, and fails with "Invalid stream ID direction" without the role assignment. Closes #314 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`openWebSocketStream` normalizes the native WebSocketStream's ArrayBuffer chunks into Uint8Array via a TransformStream, so the dial path always reaches the decoder with the right type. A transport passed straight to the constructor or to Session.accept never goes through it — and Chromium's native WebSocketStream yields ArrayBuffer — so the varint decoder threw "Expected ArrayBuffer for the first argument" and the session never opened. Normalize in the read loop instead: one branch per message, no extra stream hop on the hot path, and it defends every inbound transport rather than only the one openWebSocketStream built. Also drop the "for browsers" framing from the qmux README now that the package serves both roles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes #314.
js/qmux'sSessioncould only ever be a client. The protocol logic was already role-aware, but#isServerwas never assigned and the socket was not injectable, so there was no way to build a server session from an accepted WebSocket — a JS accept server had no equivalent of thews://fallbackmoq-relayserves alongside WebTransport.API
Session.acceptmirrors the Rust crate'sqmux::ws::Server::accept.new Session(url)keeps theWebTransportconstructor signature, soinstall()is unaffected.Changes
Session.accept(socket, options)takes a plainWebSocket, aWebSocketStream, or anythingWebSocketStreamLike— soDeno.upgradeWebSocket,ws, andBun.serveall work. The constructor now accepts a socket in place of a URL, which is what makes the role injectable.WebSocketStream.adopt()(@moq/web-socket-stream) wraps a socket that already exists. An accepted socket is typically alreadyOPEN, soonopenhas already fired and waiting for it would hang forever;openednow settles fromreadyState.selectSubprotocol()is the accept-side mirror ofresolveSubprotocols. The host runs the handshake, so it — not the library — picks the subprotocol; this expands the sameprotocols+versionspair and returns the first wire form the client offered. Preference is ours, not the client's, matchingqmux::ws::Server. It returnsundefinedon no match so the caller fails the upgrade: accepting with no subprotocol silently negotiates the legacywebtransportwire format and misreads every QMux frame.AcceptOptions.protocolstands in for hosts that don't report the negotiated subprotocol on the socket (Bun is one).Verification
New
interop-clientRust example, the mirror of the existinginterop-server: a real Rust client drives a JSSession.acceptserver across qmux-00/01/02 — 1.2 MB in each direction with flow-control replenishment, bidi streams, datagrams, and a clean close. It runs in CI viajust test.Reverting
#isServer = truefails that test immediately withInvalid stream ID direction, along with four unit tests, so the coverage is anchored to the fix rather than passing incidentally.just checkandjust testare green.Notes
exportshalf of js/qmux: no server-side Session — can't accept a WebSocket (#isServer hardcoded, socket not injectable) #314 is not included.selectSubprotocolships from the main entry, which removes the reason to reach forframe/credit/stream/varint; those stay private, consistent with Narrow QMux public API #313.🤖 Generated with Claude Code