feat(transaction): RFC 6026 Accepted state + Timer L/M for INVITE 200 OK retransmission absorption#128
Conversation
|
20 commits is too granular for a single feature. Please squash into ~3-5 logical commits (e.g. feat, docs, test). The style: cargo fmt cleanup commits and (N/N) suffix noise can be folded into their parent commits. The PR body mentions auto-ACK retention as a known RFC deviation. Consider adding an EndpointBuilder option (e.g. .auto_ack_invite_2xx(bool), default true) so users can opt into strict RFC 3261 §17.1.1.3 behavior without a breaking change. This can be a follow-up, not a blocker for this PR. The on_timer → Accepted → Terminated path has no integration test (deferred as R1). Consider adding a t1_override test knob so this can be tested without waiting 32s. |
5dd5ea1 to
bbdc4a6
Compare
|
Squashed to 4 commits per your spec. Split the breaking
(N/N) suffixes + style fmt commits folded into parents. Final state matches the previous PR HEAD: 247 lib tests pass, fmt clean. On On the |
Implements RFC 6026 §7.1–§7.2 to fix the long-standing issue where rsipstack's INVITE state machine terminates immediately on 2xx and cannot absorb retransmitted 200 OKs from the UAS, breaking glare resolution and forking scenarios.
Closes #127. Per maintainer @shenjinti's confirmation on the issue thread: "you're completely right ... A PR would be very welcome."
Branch is rebased onto current
upstream/main(8928174).Standards
Changes
Acceptedstate variantsrc/transaction/mod.rsTimerL+TimerMvariantssrc/transaction/mod.rscan_transitionmatrixsrc/transaction/mod.rstimer_l+timer_mfieldssrc/transaction/transaction.rssrc/transaction/transaction.rson_timerAccepted dispatchsrc/transaction/transaction.rssrc/transaction/transaction.rs::respondsrc/transaction/transaction.rs::on_received_responsesrc/transaction/tests/test_transaction_states.rsBoth
TransactionStateandTransactionTimerare now#[non_exhaustive]to allow future RFC additions without breakage. This is itself a breaking change — recommend targeting 0.6.0.Backward compatibility
The pre-existing convenience that auto-ACKs client INVITE 2xx from
on_received_responseis retained for 0.5.x consumers (technically a deviation from RFC 3261 §17.1.1.3, which mandates the TU drive the 2xx ACK). Documented honestly inlib.rs::Standards Compliance. Removal is a candidate for a follow-up PR that migrates 2xx-ACK responsibility to the dialog layer.Validation
cargo test --lib— 247 passed, 0 failedcargo test --doc— 65 passed, 0 failedcargo fmt --check— cleanPre-existing baseline: 66 clippy errors on
main(not introduced by this PR; touched files contribute zero new clippy regressions).Pre-submit review
Two read-only architecture review passes were run before submission. Each finding was either fixed in an atomic follow-up commit or explicitly deferred with rationale.
First pass — 3 MUST-FIX + 4 RECOMMEND + 3 NIT:
lib.rsauto-ACK retention documented honestly#[non_exhaustive]on both public enums.ok())respond/send_ack/receive(transaction_type, timer)dispatch in Accepted entry handlerwarn!on mismatched pairingSecond pass — 4 SHOULD-FIX (run after rebase onto current main, evaluated against an internal Rust style guide):
respondcancel-safety rustdoc had inverted mutation order — fixed to describe actualstore last_response → await send → transitionordersend_ackrustdoc claimed a nonexistentself.connectionfallback — rewritten to describe actual two-source resolution and silent no-send fallback when neither input arg nor 2xx-Contact lookup yields a connectionlib.rsdisclaimersend_ackpost-send dispatch replaced withif/else+debug_assert_eq!invariant (catches future#[non_exhaustive]additions)Follow-up out of scope for this PR
Commit list (atomic, 20 commits)
feat(transaction): add Accepted state for RFC 6026 §7.1/§7.2feat(transaction): add TimerL + TimerM variants for RFC 6026feat(transaction): allow RFC 6026 edges in can_transition matrixfeat(transaction): add timer_l + timer_m fields on Transactionfeat(transaction): Accepted-state entry handler per RFC 6026 §7.1/§7.2feat(transaction): on_timer Accepted dispatch + Completed 2xx guardfeat(transaction): server-side RFC 6026 2xx routing + ACK handlingfeat(transaction): client-side RFC 6026 2xx routing + send_ack splittest(transaction): RFC 6026 conformance tests for Accepted statedocs(lib): expand RFC 6026 standards-compliance claimstyle: cargo fmt --all cleanupfix(transaction): pass every 2xx in Accepted to TU per RFC 6026 §7.2 [F1]docs(lib): clarify RFC 6026 client-side auto-ACK retention [F2]feat(transaction): mark TransactionState + TransactionTimer #[non_exhaustive] [F3]refactor(transaction): tighten Accepted timer dispatch [R4 + N1 + N2]fix(transaction): log auto-ACK failures instead of silent .ok() [R2]docs(transaction): cancel-safety rustdoc on async public APIs [R3]docs(transaction): correct rustdoc and comment accuracy on touched APIs [D1 + D2 + D3]refactor(transaction): drop dead wildcard arm in send_ack post-send dispatch [T1]style: cargo fmt cleanup on rebased F1 hunk