moq-net + js/net: pre-merge API hardening for moq-lite-05#2170
Merged
Conversation
Tighten the moq-net public surface before dev lands on main and becomes
semver-load-bearing. All changes are on dev (breaking allowed).
Future-proofing (avoid an immediate semver bump after merge):
- #[non_exhaustive] on Subscription, group::Fetch, and the Announced enum.
- Move Subscription under track:: (track::Subscription); it only appears in
track signatures.
- Rename the flat Bandwidth{Producer,Consumer} into a bandwidth role module
(bandwidth::{Producer,Consumer}), matching every other Producer/Consumer.
- Rename the kio::Pending payload futures to gerunds for a consistent scheme:
track::{Subscribe->Subscribing, Fetch->Fetching, InfoQuery->Querying},
origin::Requested->Requesting.
- Turn the announce update tuple (PathOwned, Announced) into a named
#[non_exhaustive] struct announce::Update { path, event }.
- Re-export web_transport_trait (it bounds Client/Server entry points).
Misuse resistance / correctness:
- read_frame / poll_read_frame (group + track) now return the full
frame::Frame { timestamp, payload } instead of bare Bytes, so the mandatory
per-frame timestamp is no longer silently dropped.
- Remove the inherent Timestamp::max that panicked on mixed scales and shadowed
the total Ord::max.
- De-panic track::Producer::subscribe after abort(), and make Subscriber::update
/ Subscribing::update return Result instead of panicking on a closed track.
- Session close state is shared across clones: first close wins, and the
transport closes on last-clone drop (was: any clone's drop killed the shared
connection).
- server::Request rejects the handshake on drop (was: silent client hang until
idle timeout).
- Wire the full SUBSCRIBE_UPDATE into the model subscriber so the producer
aggregate reflects it and relays re-forward it upstream (was priority + end
only).
- Counters fields are crate-private (load-bearing atomic orderings must not
leak).
Datagrams:
- Drop the misleading public MAX_DATAGRAM_PAYLOAD cap (1200, but the encoded
body overshot it); enforce a 65535 buffering ceiling as FrameTooLarge and
drop oversize datagrams per hop against max_datagram_size() with a debug log.
Ordered delivery is now a DVR-only opt-in: Subscription::default is
ordered:false, and the aggregate is ordered only when every subscriber asks for
it (AND, was OR). Mirrored in js/net wire defaults.
js/net mirrors the above: subscribe() takes a Subscription options object
(not positional priority), Subscriber.update() replaces updatePriority(),
Datagram/Bandwidth are namespaced (Track.Datagram, Bandwidth.*), detached
public consumer constructors are hidden, dead AnnouncedOptions.ignoreSelf is
removed, datagram age uses performance.now(), plus doc fixes.
moq-ffi: MoqSubscription.ordered defaults to false to match.
Follow-ups filed: #2148-#2155 (FFI/binding parity gaps), #2164
(subscribe_media options record), #2165 (libmoq macOS video build), #2166
(moq-net missing docs).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Sorry @kixelated, your pull request is larger than the review limit of 150000 diff characters
…iew-ccc7ab # Conflicts: # rs/moq-ffi/src/consumer.rs # rs/moq-net/src/model/origin.rs
The session close-once refactor deferred the single transport close to the
last `Session` clone's drop. In `MoqSession`, that clone lives in the `closed`
task and is released after `Drop`'s `RUNTIME.enter()` guard, so the transport
backend's lingering-CLOSE `tokio::spawn` ran with no reactor in context and
panicked ("there is no reactor running"), aborting the process under
panic=abort. This surfaced as a SIGABRT in the Python `test_server` roundtrip.
Close the transport explicitly while the runtime is entered; close-once dedup
makes the trailing clone drop a no-op.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reconcile the API-hardening PR with overlapping work that landed on dev. Where dev and this branch reshaped the same surface, resolved as follows: - read_frame / poll_read_frame (group + track): keep this branch's `frame::Frame` return; drop dev #2174's additive `read_frame_full` / `poll_read_frame_full` split. Redirected the moq-ffi / libmoq / internal callers to the collapsed `read_frame`; kept dev's MoqFrame + timestamp_us FFI plumbing (sourced from the Frame). - Subscriber::update / SubscriberControl::update: keep this branch's `Result` de-panic (dev's #2177 control() still panicked on a closed track); ignore the Result at the FFI/libmoq call sites where a closed-track update is meaningless. - moq_net::Subscription -> moq_net::track::Subscription in dev's newer libmoq. - OriginAnnounce tuple -> announce::Update struct in dev's moq-native lite-06 announce test. - js subscribe options: keep this branch's `Subscription` type name (mirrors Rust track::Subscription) over dev #2167's SubscribeOptions; keep the hooks indirection that hides the detached Request constructor. - lite/subscriber.ts: take dev's announce-id machinery (#2160, incl. AnnouncedOptions/ignoreSelf) wholesale; port its priority poll onto this branch's subscriptionSignal. Validated: cargo check/clippy --workspace clean; moq-net/moq-ffi/libmoq/ moq-native/moq-mux tests pass; js/net tsc+biome+236 tests; py 45 tests pass.
The `moq_net::Subscription` -> `moq_net::track::Subscription` rename in the dev merge pushed two signatures past the width limit; rustfmt wants them wrapped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tightens the moq-net public surface before dev lands on main and its shapes become semver-load-bearing. Targets dev: renames, removals, and signature changes to
pubitems inmoq-net,js/net, andmoq-ffibreak existing contracts.Came out of an extensive review of the moq-net dev delta ahead of the dev→main merge (the "actually start using moq-lite-05" milestone). The goal was the API shape: catch every change that is a one-line fix now but a breaking change forever once it ships on main.
Future-proofing (avoid an immediate semver bump after merge)
#[non_exhaustive]onSubscription,group::Fetch, and theAnnouncedenum (all will gain fields/variants).Subscriptionundertrack::(track::Subscription); it only appears in track signatures.Bandwidth{Producer,Consumer}into abandwidth::{Producer,Consumer}role module, matching every other Producer/Consumer pair.kio::Pendingpayload futures to a consistent gerund scheme:track::{Subscribe→Subscribing, Fetch→Fetching, InfoQuery→Querying},origin::Requested→Requesting.announceupdate tuple(PathOwned, Announced)into a named#[non_exhaustive]structannounce::Update { path, event }.web_transport_trait(it bounds theClient/Serverentry points but was unnameable).Misuse resistance / correctness
read_frame/poll_read_frame(group + track) now return the fullframe::Frame { timestamp, payload }instead of bareBytes, so the mandatory per-frame timestamp is no longer silently dropped.Timestamp::maxthat panicked on mixed scales and shadowed the totalOrd::max.track::Producer::subscribeafterabort();Subscriber::update/Subscribing::updatereturnResultinstead of panicking on a closed track.Sessionclose state is shared across clones: first close wins, transport closes on last-clone drop (was: any clone's drop killed the shared connection).server::Requestrejects the handshake on drop with a warning (was: silent client hang until idle timeout).Countersfields are crate-private (load-bearing atomic orderings must not leak).Ordered delivery is now a DVR-only opt-in
Subscription::defaultisordered: false, and the aggregate is ordered only when every live subscriber asks for it (AND, was OR). Mirrored in thejs/netwire defaults andmoq-ffi'sMoqSubscriptiondefault.Datagrams
Drop the misleading public
MAX_DATAGRAM_PAYLOAD(advertised 1200, but the encoded body overshot it). Enforce a 65535 buffering ceiling asFrameTooLarge, and drop oversize datagrams per hop againstmax_datagram_size()with a debug log, so the limit is enforced where it actually applies (including a future hop with a smaller MTU).js/net (mirrors the above)
subscribe()takes aSubscriptionoptions object (not a positional priority),Subscriber.update()replacesupdatePriority(),Datagram/Bandwidthare namespaced (Track.Datagram,Bandwidth.*), detached public consumer constructors are hidden, deadAnnouncedOptions.ignoreSelfis removed, datagram age usesperformance.now(), plus doc fixes.Test plan
cargo test -p moq-net(444 pass) + moq-mux/json/hang/ffi/relay/transcode/nativecargo check --workspace --all-targetsclean (excluding the pre-existing macOSlibmoqvideo build, libmoq: video.rs fails to compile on macOS (CVBuffer !Send across await) #2165)cargo clippy+RUSTDOCFLAGS=-D warnings cargo docclean on moq-netjs/*packages passcheck(biome + tsc + tests)Follow-ups (filed, out of scope here)
subscribe_mediapositional arg → options recordlibmoqmacOS video!Sendbuild failure (unrelated to this change)🤖 Generated with Claude Code
(Written by Claude Fable 5)
Update — rebased on dev + fixes (Written by Claude Opus 4.8)
Sessionclone's drop, which inMoqSessionruns off the tokio runtime; the backend's lingering-CLOSEtokio::spawnthen panicked underpanic=abort. Now closed under the runtime guard on drop. (Surfaced as the Pythontest_servercrash.)read_frame() -> frame::Frameand dropped dev Preserve raw frame timestamps #2174's additiveread_frame_fullsplit; kept theupdate() -> Resultde-panic (dev Add moq-ffi track info and update #2177 still panicked) and applied it toSubscriberControltoo; kept the jsSubscriptiontype name over dev js/net: model subscription priority as an options object #2167'sSubscribeOptions; took dev's announce-id machinery (feat(moq-net,js/net): moq-lite-06-wip announce ids (typed START/END/RESTART, opt-in) #2160) and datagram FFI (Expose track datagrams through moq-ffi #2175) as-is; moved dev's libmoq refs tomoq_net::track::Subscription.cargo check/clippy --workspaceclean; moq-net/moq-ffi/libmoq/moq-native/moq-mux tests; js/net tsc+biome+236 tests; py 45 tests.