fix(net): send registered PUBLISH_DONE statuses - #3232
Conversation
Co-Authored-By: GPT-5.6 <noreply@openai.com>
|
Warning Review limit reachedNext included review available in 13 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: Pro Plus Run ID: 📒 Files selected for processing (5)
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. |
![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: 6afda0f37c
ℹ️ 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".
| // PUBLISH_DONE is required for every supported draft. The peer may have already | ||
| // closed its side to unsubscribe, in which case there is nowhere to send it. | ||
| try { | ||
| await stream.writer.u53(PublishDone.id); |
There was a problem hiding this comment.
When a TypeScript publisher ends a draft-17 through draft-19 track, this new write leaves a PUBLISH_DONE message unread by both existing subscriber implementations. The TypeScript subscriber only waits on stream.reader.closed in js/net/src/ietf/subscriber.ts:502-505; a ReadableStream with queued message bytes does not become closed until those bytes are consumed, so the remote track remains open indefinitely. The Rust subscriber similarly calls Reader::closed() at rs/moq-net/src/ietf/subscriber.rs:1361, which deliberately returns an error when it finds additional bytes, causing normal completion to abort the track. Decode PUBLISH_DONE on the request stream before treating it as closed, including handling its status code.
Useful? React with 👍 / 👎.
1 participant
Closes #3207.
Root cause
The Rust IETF publisher treated PUBLISH_DONE like an HTTP response and emitted unregistered
200/500status codes. The TypeScript publisher had kept a draft-14-through-16 guard after PUBLISH_DONE moved onto the request stream, so it emitted no terminal message for drafts 17 through 19. A rejected TypeScript track also reset the request instead of reporting a PUBLISH_DONE failure.Change
TRACK_ENDED(0x2) andINTERNAL_ERROR(0x0) codes.This follows draft-19 sections 10.11 and 15.11.3. No repository draft changes are needed because this corrects our implementation of the external IETF draft without changing a moq-dev wire format.
Checks
nix develop --command just fix origin/mainnix develop --command just check origin/mainnix develop --command just test default origin/main(all JavaScript suites and 2,933 Rust tests passed)nix develop --command just test smoke-full(all 21 Rust/Python/JS to Rust/Python/JS/native/C/GStreamer paths passed)(Written by GPT-5.6)