From 0af30ec3667796ee3902339292c0ead9a7aec8ce Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Tue, 25 Mar 2025 13:08:36 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20notify=20participants=20on?= =?UTF-8?q?ly=20if=20the=20room=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves sendData reliability by preventing execution when the room doesn’t exist. This change addresses errors in staging and production where waiting participants arrive before the room owner creates the room. In remote environments, the LiveKit Python SDK doesn’t return a clean Twirp error when the room is missing; instead of a proper "server unknown" response, it raises a ContentTypeError, as if the LiveKit server weren’t responding with a JSON payload, even though the code specifies otherwise. While the issue cannot be reproduced locally, this should help mitigate production errors. Part of a broader effort to enhance data transmission reliability. Importantly, a participant requesting entry to a room before the owner arrives should not be considered an exception. --- src/backend/core/services/lobby.py | 18 +++++- src/backend/core/tests/services/test_lobby.py | 59 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/backend/core/services/lobby.py b/src/backend/core/services/lobby.py index 36f2fb35..1fe5ddd9 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -12,7 +12,12 @@ from django.conf import settings from django.core.cache import cache from asgiref.sync import async_to_sync -from livekit.api import LiveKitAPI, SendDataRequest, TwirpError # pylint: disable=E0611 +from livekit.api import ( # pylint: disable=E0611 + ListRoomsRequest, + LiveKitAPI, + SendDataRequest, + TwirpError, +) from core import models, utils @@ -343,7 +348,18 @@ class LobbyService: } lkapi = LiveKitAPI(**settings.LIVEKIT_CONFIGURATION) + try: + room_response = await lkapi.room.list_rooms( + ListRoomsRequest( + names=[str(room_id)], + ) + ) + + # Check if the room exists + if not room_response.rooms: + return + await lkapi.room.send_data( SendDataRequest( room=str(room_id), diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index 1382aa05..9f78d89d 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -776,6 +776,43 @@ def test_update_participant_status_success(mock_cache, lobby_service, participan lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) +@mock.patch("core.services.lobby.LiveKitAPI") +def test_notify_participants_success_no_room(mock_livekit_api, lobby_service): + """Test the notify_participants method when the LiveKit room doesn't exist yet.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + + # Set up the mock LiveKitAPI and its behavior + mock_api_instance = mock.Mock() + mock_api_instance.room = mock.Mock() + mock_api_instance.room.send_data = mock.AsyncMock() + + # Create a proper response object with an empty rooms list + class MockResponse: + """LiveKit API response mock with empty rooms list.""" + + rooms = [] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + mock_api_instance.aclose = mock.AsyncMock() + mock_livekit_api.return_value = mock_api_instance + + # Act + lobby_service.notify_participants(room.id) + + # Assert + # Verify the API was initialized with correct configuration + mock_livekit_api.assert_called_once_with(**settings.LIVEKIT_CONFIGURATION) + + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + + # Verify the send_data method was not called since no room exists + mock_api_instance.room.send_data.assert_not_called() + + # Verify the connection was properly closed + mock_api_instance.aclose.assert_called_once() + + @mock.patch("core.services.lobby.LiveKitAPI") def test_notify_participants_success(mock_livekit_api, lobby_service): """Test successful participant notification.""" @@ -784,6 +821,14 @@ def test_notify_participants_success(mock_livekit_api, lobby_service): mock_api_instance = mock.Mock() mock_api_instance.room = mock.Mock() mock_api_instance.room.send_data = mock.AsyncMock() + + class MockResponse: + """LiveKit API response mock with non-empty rooms list.""" + + rooms = ["room-1"] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + mock_api_instance.aclose = mock.AsyncMock() mock_livekit_api.return_value = mock_api_instance @@ -793,6 +838,9 @@ def test_notify_participants_success(mock_livekit_api, lobby_service): # Verify the API was called correctly mock_livekit_api.assert_called_once_with(**settings.LIVEKIT_CONFIGURATION) + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + # Verify the send_data method was called mock_api_instance.room.send_data.assert_called_once() send_data_request = mock_api_instance.room.send_data.call_args[0][0] @@ -817,6 +865,14 @@ def test_notify_participants_error(mock_livekit_api, lobby_service): mock_api_instance.room.send_data = mock.AsyncMock( side_effect=TwirpError(msg="test error", code=123) ) + + class MockResponse: + """LiveKit API response mock with non-empty rooms list.""" + + rooms = ["room-1"] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + mock_api_instance.aclose = mock.AsyncMock() mock_livekit_api.return_value = mock_api_instance @@ -829,6 +885,9 @@ def test_notify_participants_error(mock_livekit_api, lobby_service): # Verify the API was called correctly mock_livekit_api.assert_called_once_with(**settings.LIVEKIT_CONFIGURATION) + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + # Verify send_data was called mock_api_instance.room.send_data.assert_called_once()