fix(rtmp): treat non-Start onStatus as rejection during PublishPending#517
fix(rtmp): treat non-Start onStatus as rejection during PublishPending#517staging-devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Any onStatus code that is not NetStream.Publish.Start is now treated as a terminal rejection when the connection is in PublishPending state. Previously, codes like NetStream.Publish.BadName fell through to the catch-all arm, leaving the connection stuck in PublishPending until the 10s handshake timeout. Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| _ if self.state == RtmpConnectionState::PublishPending => { | ||
| let desc = | ||
| extract_info_description(payload).unwrap_or_else(|| code_str.to_string()); | ||
| self.events.push_back(RtmpConnectionEvent::DisconnectedByPeer { reason: desc }); | ||
| self.set_state(RtmpConnectionState::Disconnecting); |
There was a problem hiding this comment.
📝 Info: PublishPending now fails fast for any non-start publish status
The changed arm makes PublishPending terminal for all status codes other than the exact NetStream.Publish.Start. In the handshake driver, Disconnecting is converted into an immediate handshake error rather than waiting for the 10s timeout (crates/nodes/src/transport/rtmp.rs:493-498), and during the media loop DisconnectedByPeer causes an exit through drain_events (crates/nodes/src/transport/rtmp.rs:607-620). This is an intentional behavior change, not a bug, but reviewers should note it broadens rejection handling beyond codes containing Error, Failed, or Rejected.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #517 +/- ##
==========================================
+ Coverage 79.39% 79.43% +0.04%
==========================================
Files 232 232
Lines 66904 67006 +102
Branches 1909 1909
==========================================
+ Hits 53117 53229 +112
+ Misses 13781 13771 -10
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
2 participants
Summary
handle_on_status()now treats any onStatus code that is notNetStream.Publish.Startas a terminal rejection when the connection is inPublishPendingstate, emittingDisconnectedByPeerand transitioning toDisconnecting.NetStream.Publish.BadNamefell through to the catch-all arm, leaving the connection stuck inPublishPendinguntil the 10s handshake timeout inrtmp.rs.PublishPending(e.g.NetStream.Play.Startwhile alreadyPublishing).Closes #507
Review & Validation
PublishPending— codes received in other states still fall through to the no-op_ => {}.handle_on_status_bad_name_rejects_publish— assertsBadNametransitions toDisconnectingwith aDisconnectedByPeerevent containing the description.handle_on_status_unknown_code_rejects_while_publish_pending— assertsDenied(arbitrary non-Start code) also rejects.handle_on_status_non_start_ignored_when_already_publishing— asserts unrelated codes are harmlessly ignored when alreadyPublishing.Link to Devin session: https://staging.itsdev.in/sessions/6a120b5ffe5a4acdae5dd10b3e629d4f
Requested by: @streamer45
Devin Review
a45ca91