-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(room_io): disconnect locally before server delete to suppress spurious ERROR logs #6252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -630,6 +630,16 @@ def delete_room(self, room_name: str | None = None) -> asyncio.Future[api.Delete | |
| return fut | ||
|
|
||
| async def _delete_room() -> None: | ||
| # Disconnect locally before issuing the server-side delete so the | ||
| # rust-sdk's connection-closed flag is set before the server closes | ||
| # the publisher data channels. Without this the channels are torn | ||
| # down from the remote side while the local session is still | ||
| # "connected", which triggers spurious ERROR-level logs: | ||
| # "publisher data channel '_reliable' closed unexpectedly" | ||
| try: | ||
| await self._room.disconnect() | ||
| except Exception: | ||
| logger.exception("error disconnecting room before delete; proceeding with delete") | ||
| try: | ||
| await self.api.room.delete_room( | ||
| api.DeleteRoomRequest(room=room_name or self._room.name) | ||
|
|
@@ -739,7 +749,7 @@ def transfer_sip_participant( | |
| self._track_pending_task(task, name="transfer_sip_participant") | ||
| return task | ||
|
|
||
| def shutdown(self, reason: str = "user requested") -> None: | ||
| def shutdown(self, reason: str = "") -> None: | ||
| self._on_shutdown(reason) | ||
|
Comment on lines
+752
to
753
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Default shutdown reason changed from descriptive string to empty string The default Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| def add_participant_entrypoint( | ||
|
|
||
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.
🔴 Room deletion uses the room name read after disconnecting, which may be empty
The room name is read (
self._room.nameatlivekit-agents/livekit/agents/job.py:647) after the local disconnect has already been issued, so if the SDK clears the name on disconnect the server-side delete request is sent with an empty room identifier.Impact: Callers that invoke
delete_room()without an explicitroom_name(e.g. the end-call tool) silently fail to delete the room, leaving it open.Mechanism: room name captured after disconnect instead of before
The new code at lines 639-644 calls
await self._room.disconnect()before the delete-room API call at line 646-647. Whenroom_nameisNone(the default), the fallback expressionroom_name or self._room.namereadsself._room.nameafter the room has been disconnected.In the LiveKit Python SDK,
Room.nameis backed by the Rust FFI handle's internal state. Afterdisconnect(), this state is typically cleared, causingnameto return an empty string. SinceNone or ""evaluates to"", theDeleteRoomRequestis issued withroom="", which will either fail or be a no-op on the server.Callers that hit this path:
livekit-agents/livekit/agents/beta/tools/end_call.py:127:await job_ctx.delete_room()— noroom_nameargumentexamples/voice_agents/error_callback.py:82:ctx.delete_room()Callers that pass
room_nameexplicitly (e.g.room_io.py:482,warm_transfer.py:252) are unaffected.The fix is to capture the room name before disconnecting:
Was this helpful? React with 👍 or 👎 to provide feedback.