Skip to content

fix(tokio,uring)!: a WebTransport stream reset by a closed session reads its close reason - #8527

Merged
kixelated merged 3 commits into
mainfrom
quest/m1/webtransport-close-reason
Nov 12, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m1/webtransport-close-reason

Conversation

@kixelated

@kixelated kixelated commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A relay refusing a handshake (version mismatch, refused token) closed the session with its code, but the SETUP stream's reset (code 0) usually reached the client before the CLOSE_WEBTRANSPORT_SESSION capsule, so the client reported cancelled. This follows moq-dev/moq#8478.

Both ends now follow draft-ietf-webtrans-http3 §4.4, in moq-tokio and moq-uring alike:

  • Sender: once a session closes, streams it resets, stops, or drops use WT_SESSION_GONE instead of their application code. The drop code is now picked at drop time, not when the stream opens.
  • Reader: a reset or stop with WT_SESSION_GONE that arrives before the session's close reason now waits for it. Before, it reported SessionGone. The wait ends when the capsule arrives or the connection closes. In tokio the shared CloseReason keeps a waiter list, and the capsule task always records a reason. In uring the reader waits on the connection close, which always follows reading the capsule.

Closes the quest quest/m1/webtransport-close-reason.md, which this PR deletes.

Tests

  • session_gone_waits_for_the_close (tokio unit test): covers the reset arriving first. The reader stays pending and is woken by the capsule. If the capsule arrives first, the reader resolves at once. A closed session encodes WT_SESSION_GONE.
  • streams_after_close_carry_the_close_reason (tokio end to end): the server closes with 21 "version" and then drops its streams. The client's read and write both fail with Closed(21, "version"). Before the sender change, this test failed with Reset(0).
  • just check passes. just test interop --all: every cell passed except CLI FETCH js-native-bun -> rust (group 1 of tail not found), which passed when the --tail lane was rerun, so it looks flaky.

Impact

  • Public API (breaking, hence the !): moq_tokio::quic::web::Error::SessionGone and moq_uring::quic::web::Error::SessionGone are removed, because they can no longer surface. Migration: delete any match arm on them. The peer's close now arrives as Error::Closed, or as the connection error.
  • Wire: streams torn down by a closed WebTransport session carry WT_SESSION_GONE (0x170d7b68), as the draft specifies. This does not apply to raw QUIC.
  • js/net: no change. A stream that fails with source: "session" already resolves through quic.closed (sessionCause).

Choices made without asking

  • I removed SessionGone instead of keeping a variant that can never be produced.
  • I did not verify in a real browser that Chrome reports a WT_SESSION_GONE reset as source: "session". If it reports it as a "stream" error with no code instead, the JS client still learns the close through transport.closed, as before.

Follow-ups

None.

Models

Size: S
Implementer: Claude Code, claude-opus-5-5

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1cd28-b6d2-7bb1-b64a-7145cff359a3, round 2
  • Head: ec66ff3
  • Base: origin/main at 4262a06
  • Score: 10/10, The driver changes preserve resets across repeated polls in both adapters, resolving the earlier finding without introducing new issues in the reviewed changes.
  • Findings: none
  • The last round's findings, at 006e3d8:
    • Fixed: medium, rs/moq-tokio/src/quic/web/recv.rs:105: Preserve the pending reset across read polls; Both drivers now require self.reset.is_none() before returning clean EOF from poll_read_generic, so subsequent polls return the cached reset and resume resolving the session close reason; the updated test covers repeated reads after a reset.

kixelated and others added 2 commits November 11, 2026 14:04
…ads its close reason

A closing session now resets and stops its released streams with WT_SESSION_GONE
(draft-ietf-webtrans-http3 4.4), and a reader that sees WT_SESSION_GONE before the
CLOSE_WEBTRANSPORT_SESSION capsule waits for the capsule or the connection close,
so a handshake refusal reports the peer's code and reason instead of a reset of 0.

Error::SessionGone is removed: it can no longer surface.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A WebTransport client always reads the peer's close reason fix(tokio,uring)!: a WebTransport stream reset by a closed session reads its close reason Nov 12, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 12, 2026 00:50
@coderabbitai

coderabbitai Bot commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 03fd0992-69f5-4f31-abaa-afe06828593e
Commits

Reviewing files that changed from the base of the PR and between f190d90 and ec66ff3.

Files selected for processing: 12

  • quest/m1/webtransport-close-reason.md
  • rs/moq-tokio/src/quic/driver/recv.rs
  • rs/moq-tokio/src/quic/driver/tests.rs
  • rs/moq-tokio/src/quic/web/error.rs
  • rs/moq-tokio/src/quic/web/recv.rs
  • rs/moq-tokio/src/quic/web/send.rs
  • rs/moq-tokio/src/quic/web/session.rs
  • rs/moq-uring/src/quic/driver/recv.rs
  • rs/moq-uring/src/quic/web.rs
  • rs/moq-uring/src/quic/web/error.rs
  • rs/moq-uring/src/quic/web/recv.rs
  • rs/moq-uring/src/quic/web/send.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No defects found — one test-coverage gap worth a look.

Reviewed changes

  • Sender encodes WT_SESSION_GONE at teardown time: encode_stream_code now returns the draft's session-gone code once a close reason is recorded, so explicit resets, stops, and drops all carry it; Stream::new no longer fixes the drop code, a Drop impl chooses it instead.
  • Reader waits instead of reporting SessionGone: map_error became poll_error/error() in all four web stream modules, returning Poll::Pending for a WT_SESSION_GONE reset until the reason lands.
  • tokio's CloseReason grows a waiter list: OnceLock<Error> became Mutex<Reason { err, waiters }>, with set waking outside the lock; the capsule task still records a reason on every exit path.
  • uring waits on the connection close and maps it through Handles::map, since web.closed is always written before close_code.
  • Drivers keep a reset sticky: all_data_read && reset.is_none() makes a recorded reset the answer to every later read, which the new pending-then-retry loop depends on.
  • Breaking API: Error::SessionGone removed in both crates, with a migration note in the Impact section.

ℹ️ The uring half of the fix has no test

Every new test for this change is in moq-tokio: the unit test on CloseReason and streams_after_close_carry_the_close_reason. The moq-uring side changes the same two behaviors but is covered by nothing, and its reader is the structurally different one — it waits on the connection close rather than a per-session reason and waiter list — so a regression there would only surface in an interop cell.

Technical details
# Cover uring's WT_SESSION_GONE encode and wait

## Affected sites
- `rs/moq-uring/src/quic/web.rs:912` — `Handles::encode_stream_code`: the closed-session
  branch is new and untested.
- `rs/moq-uring/src/quic/web.rs:929` — `Handles::poll_session_gone`: waits on
  `conn.poll_closed`, then `Handles::map`; no test ever has a reader park here.
- `rs/moq-uring/src/quic/web/recv.rs:196` and `rs/moq-uring/src/quic/web/send.rs:262` — the
  new `Drop` impls that pick the code at drop time.
- `rs/moq-uring/tests/integration/web.rs` — the harness that should gain the test:
  `tokio_client`, `drain`, `write_finish`, and `within`, plus the existing reset tests
  `a_dropped_stream_carries_a_webtransport_code` and `a_reset_stream_keeps_its_header`.

## Required outcome
- At least one integration test fails if either uring branch is reverted: streams released
  after a uring session closes are reset and stopped with `WT_SESSION_GONE` (the uring
  sender), and a uring reader that sees `WT_SESSION_GONE` reports the session's close code
  and reason once it is known (the uring reader).
- Mirror the tokio coverage rather than a new harness: `streams_after_close_carry_the_close_reason`
  has a uring-shaped twin in the existing file.

## Suggested approach
- Sender direction: the tokio client closes nothing; the uring server calls
  `session.close(21, "version")` and drops a stream, and the tokio client asserts
  `recv::Error::Session(Error::Closed(21, ..))` and `send::Error::Session(..)`, exactly as
  the tokio e2e test does with the roles swapped.
- Reader direction: the tokio client closes with a code, then the uring server reads a stream
  the client reset and expects `quic::web::recv::Error::Session(quic::web::Error::Closed(..))`
  through `within(&handle, ..)` so the pending wait cannot hang the suite.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit 01f50c1 Nov 12, 2026
7 checks passed
@kixelated
kixelated deleted the quest/m1/webtransport-close-reason branch November 12, 2026 04:32
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.

1 participant