Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
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. |
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2798b3bf65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// How the publisher ended the subscription: cleanly, or with the error its status names. | ||
| pub(crate) fn end(&self, version: Version) -> Result<(), crate::Error> { | ||
| match self.status_code { | ||
| 0x1 => Err(crate::Error::Unauthorized), |
There was a problem hiding this comment.
When an IETF peer sends PUBLISH_DONE status 0x1, this change makes Rust expose Error::Unauthorized, but js/net/src/ietf/subscriber.ts:670-673 still throws a generic Error for the same input. This leaves the Rust and TypeScript implementations with different public terminal semantics; land the shared JS error dependency first or update js/net in this change.
AGENTS.md reference: AGENTS.md:L94-L97
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| self.last_err = Some(Error::Dropped); | ||
| self.last_err = Some(err.clone()); |
There was a problem hiding this comment.
If source A aborts with Unauthorized, the front records that error here; after successfully attaching source B through another route from the same publisher, attach does not clear it. If B's route is later retracted while B remains live, selected(None) therefore ends the front with A's stale Unauthorized instead of the normal route-loss Dropped, so consumers receive an error from a source they already recovered from. Clear last_err after a successful attachment or otherwise scope it to the failed selection attempt.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Problem
An aborted broadcast loses its cause when an origin front closes or a consumer asks for a track afterward. A still-advertised route can re-request an aborted broadcast instead of reporting its error. IETF PUBLISH_DONE Unauthorized is reported as Remote(1).
Approach
Carry source errors through close events and front termination, exclude the dead source's route from that front's failover, and preserve the cause on late track lookup. Decode PUBLISH_DONE Unauthorized as Error::Unauthorized. Add local/routed regressions and a source-close benchmark.
Impact
Alternatives
The branch includes merged #4120; do not duplicate the revocation changes owned by #4179. Keep this PR focused on uncovered Rust paths.
Validation
nix develop --command just check: passed on main including fix(net): end a track with its session's error when the session dies #4120 (4,478 default-feature Rust tests, 496 additional feature tests, JS tests, docs, formatting, and quest validation).cargo clippy --locked -p moq-net --bench origin -- -D warnings: passed in the Nix shell.Follow-ups
The quest remains open with an explicit dependency on #4179. After it lands, complete the combined mock-session/relay proof and map JS PUBLISH_DONE Unauthorized using #4179's shared error type.
(Written by GPT-6)