Skip to content

Conversation

@kixelated
Copy link
Collaborator

@kixelated kixelated commented Nov 6, 2025

Now waits for an announce instead of YOLO subscribing. This will work with IETF relays that implement PUBLISH_NAMESPACE.

Summary by CodeRabbit

  • Improvements
    • Enhanced resilience in broadcast state handling with dynamic subscription management.
    • Added diagnostic logging for client-server handshake communication.
    • Improved error handling and state transitions in event-driven workflows.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

This pull request introduces logging enhancements and refactors the clock subscription lifecycle. In the TypeScript connection module, a variable is renamed from msg to client with corresponding debug logs added for client setup transmission and server setup reception. The Rust session module receives similar tracing logs for client and server setup exchanges. The most significant change occurs in the clock module, where the subscription flow transitions from a single-shot pattern to an event-driven loop that dynamically manages clock subscriber lifecycle based on broadcast announcements, with new tracing for state transitions and offline/online changes. No modifications to public APIs or control flow logic are present.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references improving the moq-clock subscriber, which is the primary focus of the changeset. However, it omits the key objective: enabling IETF relay compatibility by waiting for announcements instead of immediate subscription.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch wrong-error

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94b2e54 and 8e7245a.

📒 Files selected for processing (3)
  • js/moq/src/connection/connect.ts (2 hunks)
  • rs/moq-clock/src/main.rs (1 hunks)
  • rs/moq/src/session.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rs/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Write Rust tests integrated within source files (unit tests alongside code).

Files:

  • rs/moq/src/session.rs
  • rs/moq-clock/src/main.rs
🧬 Code graph analysis (3)
rs/moq/src/session.rs (1)
rs/hang-cli/src/client.rs (1)
  • client (8-51)
js/moq/src/connection/connect.ts (2)
js/moq/src/lite/connection.ts (3)
  • stream (133-148)
  • stream (150-166)
  • stream (187-195)
js/moq/src/ietf/connection.ts (1)
  • stream (263-272)
rs/moq-clock/src/main.rs (4)
rs/moq/src/model/origin.rs (8)
  • select (259-283)
  • announce (36-39)
  • announce (69-77)
  • announced (480-482)
  • new (17-19)
  • new (62-67)
  • new (112-118)
  • new (452-471)
rs/moq/src/model/broadcast.rs (4)
  • select (357-365)
  • subscribe_track (191-232)
  • closed (234-240)
  • closed (326-354)
rs/moq/src/session.rs (2)
  • new (21-23)
  • closed (210-215)
rs/moq-clock/src/clock.rs (2)
  • run (15-41)
  • run (82-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Check
🔇 Additional comments (3)
rs/moq/src/session.rs (1)

51-52: Clearer handshake tracing

The added trace logs give us much-needed visibility into both sides of the handshake without touching behavior—nice quality-of-life win.

Also applies to: 69-70

js/moq/src/connection/connect.ts (1)

85-97: Handy client/server setup logs

Mirroring the Rust tracing with these debug statements makes cross-language handshake diagnostics far easier—appreciate the clarity boost.

rs/moq-clock/src/main.rs (1)

78-108: Resilient announce-driven subscriber loop

Really like this refactor: scoping with consume_only, waiting on announces, and canceling the active subscriber via tokio::select! makes the clock follower play nicely with IETF relays while staying responsive to state changes.


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 and usage tips.

@kixelated kixelated merged commit 95421aa into main Nov 7, 2025
1 check passed
@kixelated kixelated deleted the wrong-error branch November 7, 2025 19:04
@moq-bot moq-bot bot mentioned this pull request Nov 7, 2025
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