Skip to content

feat(moq-net): request_broadcast returns Unroutable when no route exists#1779

Merged
kixelated merged 1 commit into
devfrom
claude/request-broadcast-errors-pobryq
Jun 18, 2026
Merged

feat(moq-net): request_broadcast returns Unroutable when no route exists#1779
kixelated merged 1 commit into
devfrom
claude/request-broadcast-errors-pobryq

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #1772 (the new OriginConsumer::request_broadcast). Two asks:

  1. Return Unroutable instead of NotFound. request_broadcast previously failed with Error::NotFound when a path was neither announced nor served by an OriginDynamic handler. That's indistinguishable from a track/broadcast that genuinely doesn't exist, so this adds a dedicated Error::Unroutable (wire code 30) meaning "no announcement and no dynamic router, so there is no route to this broadcast." It is returned in both failure modes:

    • synchronously, when no handler is live (dynamic == 0);
    • asynchronously, when a request was registered while a handler was live but then loses every handler before being served (the channel closes without a result).

    An explicit BroadcastRequest::reject(err) still surfaces the handler's own error unchanged.

  2. Checked the "accept before dynamic" race. Conclusion: the model is race-free. OriginDynamic::new/clone/drop and the dynamic == 0 check in request_broadcast all mutate the count under the same lock, so a handler that is live when request_broadcast acquires the lock always registers the request rather than rejecting it. Crucially, accept is decoupled from the count: once a handler has picked a request up via requested_broadcast, the BroadcastRequest owns the result channel, so it can still accept and resolve the requester even after every handler (itself included) drops and flips the count to zero. Added dynamic_request_accept_after_handler_dropped to lock that in.

    The only remaining race is caller-ordering: if request_broadcast runs before a handler is created, it fails fast with Unroutable. That's the same race the track-level code already addresses by creating the dynamic handler before announcing (see lite/subscriber.rs:414), and the synchronous increment in OriginDynamic::new makes the same "create before expose" discipline sufficient here. No relay site wires OriginDynamic in yet, so there's no caller to fix today.

Public API changes

  • moq-net: new enum variant Error::Unroutable (the enum is #[non_exhaustive], so additive). New wire code 30. libmoq already maps every moq_net::Error to a single -2 status, so no FFI change is needed.

Test plan

  • cargo test -p moq-net --lib (386 passed)
  • cargo clippy -p moq-net clean
  • dynamic_request_unroutable_without_handler (renamed) asserts Unroutable
  • dynamic_request_handler_dropped asserts Unroutable for both the pending future and a fresh request
  • dynamic_request_accept_after_handler_dropped (new) covers accept-after-drop

(Written by Claude)

🤖 Generated with Claude Code


Generated by Claude Code

`OriginConsumer::request_broadcast` previously failed with `NotFound` when a
path was neither announced nor served by an `OriginDynamic` handler, which is
indistinguishable from a track/broadcast genuinely not existing. Add a dedicated
`Error::Unroutable` (wire code 30) and return it both synchronously (no handler
live) and when a registered request loses every handler before being served.

Also adds a regression test for the accept/dynamic race: once a handler picks a
request up, `accept` still resolves the requester even if every handler drops
first and flips the dynamic count to zero. The in-flight request is decoupled
from the count, so it is not spuriously rejected as Unroutable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VhPahy17b2FWxDdz1ugFqm
@kixelated kixelated merged commit eeec303 into dev Jun 18, 2026
1 check passed
@kixelated kixelated deleted the claude/request-broadcast-errors-pobryq branch June 18, 2026 20:21
@kixelated kixelated mentioned this pull request Jun 30, 2026
4 tasks
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.

2 participants