Repository navigation
fix(tokio,uring)!: a WebTransport stream reset by a closed session reads its close reason - #8527
Conversation
Quest-Claim: 55aca416798ca2c2
…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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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. View limit details
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. |
There was a problem hiding this comment.
ℹ️ No defects found — one test-coverage gap worth a look.
Reviewed changes
- Sender encodes
WT_SESSION_GONEat teardown time:encode_stream_codenow returns the draft's session-gone code once a close reason is recorded, so explicit resets, stops, and drops all carry it;Stream::newno longer fixes the drop code, aDropimpl chooses it instead. - Reader waits instead of reporting
SessionGone:map_errorbecamepoll_error/error()in all four web stream modules, returningPoll::Pendingfor aWT_SESSION_GONEreset until the reason lands. - tokio's
CloseReasongrows a waiter list:OnceLock<Error>becameMutex<Reason { err, waiters }>, withsetwaking 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, sinceweb.closedis always written beforeclose_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::SessionGoneremoved 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.deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

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-tokioandmoq-uringalike:SessionGone. The wait ends when the capsule arrives or the connection closes. In tokio the sharedCloseReasonkeeps 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 with21 "version"and then drops its streams. The client's read and write both fail withClosed(21, "version"). Before the sender change, this test failed withReset(0).just checkpasses.just test interop --all: every cell passed exceptCLI FETCH js-native-bun -> rust(group 1 of tail not found), which passed when the--taillane was rerun, so it looks flaky.Impact
!):moq_tokio::quic::web::Error::SessionGoneandmoq_uring::quic::web::Error::SessionGoneare removed, because they can no longer surface. Migration: delete any match arm on them. The peer's close now arrives asError::Closed, or as the connection error.source: "session"already resolves throughquic.closed(sessionCause).Choices made without asking
SessionGoneinstead of keeping a variant that can never be produced.source: "session". If it reports it as a"stream"error with no code instead, the JS client still learns the close throughtransport.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 reviewafter each round.01a1cd28-b6d2-7bb1-b64a-7145cff359a3, round 2The driver changes preserve resets across repeated polls in both adapters, resolving the earlier finding without introducing new issues in the reviewed changes.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.