Repository navigation
fix(json): treat a publisher-reset group as a lost group - #6204
Conversation
A publisher resets a superseded group with stream code Old, which a reader sees as Error::Stream(StreamError::Old). #5677 narrowed the lost-group match in the moq-json snapshot and window consumers, and the moq-flate snapshot consumer, to the locally raised variants, so that reset became a fatal error and a catalog reader stopped for good. Classify by StreamError instead, so the local and wire spellings are the same loss. 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 49 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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Merge summary: no changes made during
Enabling auto-merge on the head above. (Written by Claude Opus 5.5) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
When a publisher resets a superseded group, it sends stream code
Old. The reader sees that asError::Stream(StreamError::Old).#5677 narrowed the lost-group match in these readers to the locally raised variants (
Error::Old | Evicted | Lagged | GroupTooLarge):So a wire reset now surfaces as a fatal error. A hang catalog is a long-lived snapshot group, which makes this hit catalog readers. For example, moq-hls's
watch_cataloggives up on the broadcast and never resubscribes, so a recording of that broadcast stays empty.It is intermittent: it needs the publisher to supersede the catalog group while a relay-side reader is attached. A relay recorder loop reproduced it about once in 75 broadcasts. Instrumented, the failing read was
Stream(Old).Fix
Classify the error by
StreamError::from(&err), the same mapping the wire uses, so the local and wire spellings count as the same loss:OldEvictedTooFarBehindGroupTooLargeThe JS consumers already classify by
StreamCode, so this brings Rust in line.There is no public API or wire change.
Tests
The existing lost-group tests now run once per spelling: local
Error::Old, then the wireError::Stream(StreamError::Old). The window test also covers the wireTooFarBehind,EvictedandGroupTooLarge. All three tests fail without the fix.just checkpasses exceptnix flake check. That step builds the github-runner image, which needs user namespaces this host's Nix sandbox refuses. The diff touches no Nix. I ran every stage after it by hand, plusjust rs test-changed: 4364, 516 and 224 tests, all passing.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code