-
Notifications
You must be signed in to change notification settings - Fork 240
fix: cache participant attrs for BYE attributes_to_headers (#404) - #775
fix: cache participant attrs for BYE attributes_to_headers (#404) #775lixuanqun wants to merge 1 commit into
Conversation
When an agent hangs up by deleting the room, LocalParticipant is gone before SIP sends BYE, so attributes_to_headers mapping was skipped and custom X-* headers were missing. Cache the last-seen participant attributes on join/update and reuse them when building BYE/REFER headers if the room is already nil. Applies to inbound and outbound calls. Fixes livekit#404 Co-authored-by: li xuanqun <793005378@qq.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 1 potential issue.
View 1 additional finding in Devin Review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Saved caller attributes can be wiped out by an empty refresh, so custom hangup headers are lost again
The stored copy of the caller's attributes is unconditionally replaced with whatever the live room reports (c.cachedAttrs = attrs at pkg/sip/inbound.go:1817-1820), even when that live report is empty, so the values kept for the hangup message can be erased and the custom headers go missing again.
Impact: In the exact teardown situation this change is meant to fix, the outgoing hangup can still be sent without the configured custom headers.
How an empty live read overwrites the seeded cache
snapshotParticipantAttrs (inbound pkg/sip/inbound.go:1809-1821, outbound pkg/sip/outbound.go:183-195) writes c.cachedAttrs = attrs with no length check, unlike storeParticipantAttrs which deliberately ignores empty input (pkg/sip/inbound.go:1824). Right after seeding the cache from the join config (pkg/sip/inbound.go:1556-1557 and pkg/sip/outbound.go:508-509) a snapshot is taken immediately; if LocalParticipant.Attributes() has not yet been populated it returns an empty map and the seed is discarded. The same holds for participantAttributes() (pkg/sip/inbound.go:1832-1836), which snapshots first: if the room object still exists during teardown but the local participant's attribute map has already been cleared, the good cache is replaced with an empty one and fillHeaders then returns the headers untouched (pkg/sip/inbound.go:1799-1801).
Guarding the write with if len(attrs) == 0 { return } makes the snapshot strictly additive/refreshing and keeps the fallback usable.
Was this helpful? React with 👍 or 👎 to provide feedback.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #775 +/- ## ========================================== + Coverage 65.25% 66.28% +1.03% ========================================== Files 51 41 -10 Lines 6588 7997 +1409 ========================================== + Hits 4299 5301 +1002 - Misses 1915 2206 +291 - Partials 374 490 +116 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
genseric-ghiro
commented
Aug 31, 2026
Thanks for the fix @lixuanqun. I think it'd be a good idea to implement devin's suggestion for both inbound and outbound calls
When an agent hangs up by deleting the room, LocalParticipant is gone before SIP sends BYE, so attributes_to_headers mapping was skipped and custom X-* headers were missing.
Cache the last-seen participant attributes on join/update and reuse them when building BYE/REFER headers if the room is already nil. Applies to inbound and outbound calls.
Fixes #404