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>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| attrs := r.LocalParticipant.Attributes() // clones | ||
| c.attrsMu.Lock() | ||
| c.cachedAttrs = attrs | ||
| c.attrsMu.Unlock() |
There was a problem hiding this comment.
🟡 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.
| attrs := r.LocalParticipant.Attributes() // clones | |
| c.attrsMu.Lock() | |
| c.cachedAttrs = attrs | |
| c.attrsMu.Unlock() | |
| attrs := r.LocalParticipant.Attributes() // clones | |
| if len(attrs) == 0 { | |
| return // do not drop a previously cached snapshot | |
| } | |
| c.attrsMu.Lock() | |
| c.cachedAttrs = attrs | |
| c.attrsMu.Unlock() |
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:
|
2 participants
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