js/qmux: `session.closed` never rejects — abnormal termination is indistinguishable from a graceful close

[fperex]

@moq/qmux's Session advertises itself as a drop-in WebTransport replacement (implements WebTransport, README: "Use as a drop-in WebTransport replacement"), but its closed promise diverges from the WebTransport contract: it always resolves and never rejects, even when the session ends abnormally.

Per the W3C WebTransport spec, closed is "fulfilled when the associated WebTransport object is closed gracefully, or rejected when it is closed abruptly or failed on initialization" — abrupt termination rejects with a WebTransportError (source: "session"). Native browser WebTransport behaves this way.

What qmux does instead

All refs against main @ 524449b, js/qmux/src/session.ts:

closed is constructed resolve-only — only the resolver is ever captured (session.ts#L452-L454); there is no reject counterpart anywhere in the class (contrast ready, which captures both). Every termination path funnels into the idempotent #close(code, reason) (L1360), which unconditionally resolves closed with {closeCode, reason} (L1372-L1375).

That includes every abnormal ending, which a spec-conformant closed would reject:

Path	Site	Resolves with
WebSocket failed to open	L494	closeCode: 1006
WebSocket closed rejected (error)	L504	closeCode: 1006
WebSocket dropped (onclose, no CONNECTION_CLOSE)	L500	ws code / 1006
Read-loop error mid-session	L524	closeCode: 1006
Protocol violations (decode failure, frame too large, stream limit, flow control)	L586, L914, L940, L965, L1044	closeCode: 1002
Text frame received	L517	closeCode: 1003
Idle timeout	L749	closeCode: 0, "idle timeout"

Only local transport.close() (L1446) and a peer CONNECTION_CLOSE frame (L609) are genuinely graceful closes that should resolve.

Why it matters

The standard consumer idiom is:

try {
	await session.closed; // clean shutdown
} catch (e) {
	scheduleReconnect(e); // abnormal drop
}

On qmux the catch arm is unreachable, so any code written against the native contract silently treats every network drop / protocol error / idle timeout as a clean close. Concretely this broke reconnect in moq: Safari and Firefox use the qmux WebSocket fallback, and a relay drop left a permanently dead session with status "connected" and no retry — see moq-dev/moq#2183 (closed with "this sounds like a qmux bug… file an issue or submit a PR to qmux", hence this issue).

Notably the Rust side already makes this distinction: web-transport-trait's closed() yields an Error whose variant separates graceful (Error::Closed, ConnectionClosed{code,reason}) from abnormal (IdleTimeout, ProtocolViolation, …). The JS port flattens that into an always-resolved WebTransportCloseInfo.

Suggested fix

Capture closed.reject alongside the resolver and have #close() (or its callers) distinguish the two classes:

• Resolve with {closeCode, reason}: local close(), peer CONNECTION_CLOSE.
• Reject with a WebTransportError-shaped error (source: "session"): WebSocket open failure/error/unexpected close, read errors, protocol violations, idle timeout. (Where the global WebTransportError constructor is unavailable, an Error subclass with source = "session" matching the interface would keep the polyfill self-contained.)

The existing tests in js/qmux/src/session.test.ts currently assert the resolve-always behavior (e.g. protocol-violation cases await session.closed and check closeCode 1002/1003), so they'd flip to .rejects assertions for the abnormal cases.

Happy to submit a PR for this if the approach sounds right.

[coderabbitai]

🔗 Related PRs

#247 - feat(qmux): prioritized send scheduler + @moq/web-socket-stream polyfill [closed]
#252 - fix(quiche): don't tear down the connection when resetting a closed stream [closed]
#281 - fix(qmux): tear down writer parked in an in-flight send() [merged]
#282 - fix(qmux): move idle timeout + keep-alive to a dedicated timer task [merged]
#285 - fix(qmux): stream-close race + propagate close reason to blocked Credit.claim() waiters (JS) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[fperex]

Some additional evidence after digging further. The strongest argument for this isn't the W3C spec — it's that rs/qmux in this same repo already implements the correct semantics, and the JS port throws that information away.

The two qmux implementations disagree

rs/web-transport-trait/src/lib.rs:126 defines the session-level contract as:

fn closed(&self) -> impl Future<Output = Self::Error> + MaybeSend;

…and rs/qmux gives it a real taxonomy (rs/qmux/src/error.rs): Closed (:61), IdleTimeout (:64), HandshakeTimeout (:67), ProtocolViolation (:43), FlowControlError (:31), FrameEncoding (:37), ConnectionClosed { code, reason } (:19) — wired up at rs/qmux/src/session.rs:66 (graceful teardown → Error::Closed), :659 (idle timeout → Error::IdleTimeout), :975 (peer close → Error::ConnectionClosed).

So a Rust consumer can tell a dead peer from a clean shutdown. A JS consumer of the same protocol cannot — every one of those outcomes arrives as a fulfilled {closeCode, reason}. That's a parity gap between the two implementations, not just a spec footnote.

The sharpest repro: idle timeout is indistinguishable from a clean close

// dead peer — qmux gave up on it
session.ts:749   this.#close(0, "idle timeout");
//               → closed resolves { closeCode: 0, reason: "idle timeout" }

// graceful shutdown — app called close() with no args
session.ts:1435-1436   const code = info?.closeCode ?? 0;
                       const reason = info?.reason ?? "";
//               → closed resolves { closeCode: 0, reason: "" }

Same settled state, same close code. The only thing separating "the peer went away" from "we shut down cleanly" is a free-text reason string. That is why try { await session.closed } catch { reconnect() } can never work here.

There isn't a closeCode workaround either

The obvious escape hatch — "just treat 1002/1003/1006 as abnormal" — is unsound:

• session.ts:500 forwards the raw WebSocket status code straight into WebTransportCloseInfo.closeCode. But WebTransport close codes are application-defined 32-bit values, so an app (or peer) that legitimately closes with code 1006 is indistinguishable from a dropped socket.
• The idle-timeout case above resolves with code 0, which collides with the graceful default.
• It's documented nowhere — js/qmux/README.md has zero mentions of closed, error, or reconnect.

Scope notes

• This is in the published package, not just the repo: I pulled @moq/qmux@0.2.0 from npm and package/session.js is resolve-only (#closedResolve at :224, this.#closedResolve = closed.resolve at :293-295, this.#closedResolve({...}) at :1074). session.d.ts:134 ships readonly closed: Promise<WebTransportCloseInfo> — the type itself advertises a promise that only ever fulfills.
• js/web-transport (the NAPI wrapper) has the same bug independently — js/web-transport/src/session.ts:124 does closed.resolve({ closeCode: 0, reason: String(err) }) inside a .catch(), so an outright error resolves closed with code 0. Out of scope for a qmux fix; flagging it as a likely follow-up.
• A peer-initiated CONNECTION_CLOSE (session.ts:609) should keep resolving — that's a graceful close and I'm not proposing to change it. Only terminations with no CONNECTION_CLOSE, plus locally-detected failures, should reject.

One wrinkle worth surfacing for whoever takes this: the locally-detected protocol violations (session.ts:517, :585, :965, :1044) currently route through the public this.close({ closeCode: 1002, ... }) — i.e. qmux tells itself to close gracefully on detecting a violation. So the fix is a bit more than "add a reject branch to #close()"; those call sites need to go through a distinct failure path. It also means a correct fix necessarily rewrites some currently-green assertions in session.test.ts that lock in the resolve-on-abnormal behavior.

I'll open a PR along these lines shortly — happy to adjust the shape if you'd rather it landed differently.