Skip to content

feat(hang): add typed catalog consumers and cap renditions - #3935

Open
kixelated wants to merge 5 commits into
mainfrom
quest/main/hang-catalog-consumer
Open

kixelated wants to merge 5 commits into
mainfrom
quest/main/hang-catalog-consumer

Conversation

@kixelated

@kixelated kixelated commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

Reading a Hang catalog requires manual track wiring in Rust and JS. moqsrc can open an unbounded number of pending rendition subscriptions from one catalog update. JS also lacks a timeline consumer and typed JSON failure signals.

Approach

Add one-call typed catalog readers in Rust and JS. Reject catalog updates above 64 combined video, audio, and text renditions before subscribers reconcile media tracks. Add a JS timeline reader, expose the media retention age, and align JSON schemas and errors with Rust. Preserve the mux consumer's existing error variants.

Impact

  • Rust: additive Catalog::<E>::subscribe, catalog::Consumer<E>, Catalog::check_renditions, catalog::MAX_RENDITIONS, Error::{TooManyRenditions, EscapingBroadcast}, and container::MAX_AGE.
  • JS Hang: additive Catalog.watch, Catalog.checkRenditions, Catalog.TooManyRenditions, Catalog.MAX_RENDITIONS, and Timeline.{Consumer, Entry, Event}.
  • JS JSON: additive Desync and MissingSnapshot errors and optional Stream.Config.schema. Unexpected snapshot frame read failures now propagate; only explicit retention gap codes resync.
  • Wire: none.

Tests

just check passed locally. Focused Rust Hang/mux and JS Hang/JSON regression tests passed. All PR CI jobs passed, including Test and WASM (after rerunning a Nix cache download failure). The local full just test compiled the workspace but nextest could not list the unrelated moq-uring::bench/echo_noq benchmark because worker setup hit Cannot allocate memory (os error 12) on the shared host.

Alternatives

A cap in moqsrc alone would leave other catalog readers with the same unbounded policy. The shared catalog validation rejects the complete update before media subscriptions are opened.

Follow-ups

None.

Closes #3137.

(written by GPT-6 Astra)

codex and others added 5 commits September 22, 2026 16:43
Co-Authored-By: GPT-6 Astra <codex@openai.com>
Co-Authored-By: GPT-6 Astra <codex@openai.com>
Co-Authored-By: GPT-6 Astra <codex@openai.com>
Co-Authored-By: GPT-6 Astra <codex@openai.com>
Co-Authored-By: GPT-6 Astra <codex@openai.com>
@kixelated
kixelated marked this pull request as ready for review September 23, 2026 00:48
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 26 minutes.

Check out review usage here.

View limit details

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

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 6d9bd511-ba2c-4c9a-8a66-77a503dfdab1

📥 Commits

Reviewing files that changed from the base of the PR and between 248054c and e9623fe.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (33)
  • doc/concept/hang.md
  • doc/lib/js/hang.md
  • doc/lib/js/json.md
  • doc/lib/rs/hang.md
  • js/hang/src/catalog/consumer.test.ts
  • js/hang/src/catalog/root.ts
  • js/hang/src/timeline.test.ts
  • js/hang/src/timeline.ts
  • js/json/src/error.ts
  • js/json/src/index.ts
  • js/json/src/snapshot/codec.test.ts
  • js/json/src/snapshot/consumer.ts
  • js/json/src/snapshot/decoder.ts
  • js/json/src/snapshot/index.ts
  • js/json/src/snapshot/snapshot.test.ts
  • js/json/src/stream/codec.test.ts
  • js/json/src/stream/consumer.ts
  • js/json/src/stream/decoder.ts
  • js/json/src/stream/encoder.ts
  • js/json/src/stream/index.ts
  • js/json/src/stream/producer.ts
  • js/watch/src/broadcast.ts
  • quest/next/README.md
  • quest/next/hang-catalog-consumer.md
  • rs/hang/Cargo.toml
  • rs/hang/src/catalog/consumer.rs
  • rs/hang/src/catalog/mod.rs
  • rs/hang/src/catalog/root.rs
  • rs/hang/src/container/frame.rs
  • rs/hang/src/container/mod.rs
  • rs/hang/src/error.rs
  • rs/moq-mux/src/catalog/hang/consumer.rs
  • rs/moq-mux/src/catalog/msf/consumer.rs

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

Comment @coderabbitai help to get the list of available commands.

@moq-bot

moq-bot Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor
  1. JS containment skips data tracks Rust rejects, so an escaping json/binary broadcast passes watch but fails Rust (js/watch/src/broadcast.ts:21, AGENTS.md#Cross-Package Sync). Chain catalog.json.tracks and catalog.binary.tracks through findEscaping and filterCatalog like rs/moq-mux/src/catalog/hang/consumer.rs:87.
  2. JS Catalog.watch enforces only the rendition cap while Rust Consumer::poll_next also rejects escaping broadcasts (js/hang/src/catalog/root.ts:74, rs/moq-mux/CLAUDE.md#Cross-language). Add the same escaping check to watch or make it share assertResolvable.

Verdict: request changes
(Written by Muse Spark)

New%20session%20-%202026-09-23T00%3A50%3A07.519Z
opencode session  |  github run

@kixelated

Copy link
Copy Markdown
Collaborator Author

Automated review

What this does

Adds one-call typed catalog readers (Catalog::subscribe / Catalog.watch), a shared MAX_RENDITIONS (64) refusal before media subscriptions open, a JS timeline Consumer, public container::MAX_AGE, and JS JSON error alignment (Desync, MissingSnapshot, optional Stream.Config.schema). Closes the hang-catalog-consumer quest and #3137.

Strengths

  • Cap lives in the shared catalog path, not only in moqsrc — every reader inherits the same DoS/pending-subscription bound.
  • Additive APIs in Rust and JS mirror each other; docs updated; focused regression tests for the cap, watch, and timeline conversion (including fractional timescale flooring).
  • Escaping-broadcast checks on the Rust consumer and JS watch path keep validation in one place with the rendition limit.
  • JSON consumers no longer swallow unexpected frame-read failures; that is the right default for callers that were wrapping next() just to distinguish dead tracks from bad frames.

Concerns

  • Hard refusal of the whole oversized update (vs. truncating) is the safer product choice, but publishers that accidentally emit huge catalogs will look “offline” rather than degraded — call that out in any operator notes if not already.
  • Local full just test hit OOM on an unrelated bench; CI green is the real gate here — fine as long as Test/WASM stay green.

Recommendation rationale

Directly closes a known unbounded-subscription issue with a shared policy, reduces boilerplate for catalog readers, and keeps wire format unchanged. Complexity is mostly the consumer glue plus the cap — worth it.

MERGE

This is an automated review, not the maintainer's decision
(Written by Grok)

This branch has not been deployed

No deployments
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.

moqsrc: bound the pending rendition subscriptions a catalog can open

2 participants