From 07af7a85ffcd76fcc2605e34a3004cae33b1c35e Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Wed, 8 Apr 2026 18:57:32 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=A5=85(backend)=20refine=20Twirp=20error?= =?UTF-8?q?=20handling=20for=20participant=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid mapping all Twirp errors to generic 500 responses. Explicitly handle the case where a participant is no longer in the room, as this may indicate suspicious behavior or a client state issue. Improve error discrimination to provide more accurate responses. --- src/backend/core/api/viewsets.py | 26 ++++++++++ .../core/services/participants_management.py | 28 ++++++++++ .../test_api_rooms_participants_management.py | 52 +++++++++++++++++++ .../rooms/test_api_rooms_rename_toggle.py | 32 ++++++++++++ 4 files changed, 138 insertions(+) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index e924748c..202695de 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -65,6 +65,7 @@ from core.services.lobby import ( LobbyService, ) from core.services.participants_management import ( + ParticipantNotFoundException, ParticipantsManagement, ParticipantsManagementException, ) @@ -599,6 +600,11 @@ class RoomViewSet( identity=str(serializer.validated_data["participant_identity"]), track_sid=serializer.validated_data["track_sid"], ) + except ParticipantNotFoundException: + return drf_response.Response( + {"error": "Participant not found"}, + status=drf_status.HTTP_404_NOT_FOUND, + ) except ParticipantsManagementException: return drf_response.Response( {"error": "Failed to mute participant"}, @@ -637,6 +643,11 @@ class RoomViewSet( permission=permission.model_dump() if permission else None, name=serializer.validated_data.get("name"), ) + except ParticipantNotFoundException: + return drf_response.Response( + {"error": "Participant not found"}, + status=drf_status.HTTP_404_NOT_FOUND, + ) except ParticipantsManagementException: return drf_response.Response( {"error": "Failed to update participant"}, @@ -669,6 +680,11 @@ class RoomViewSet( room_name=str(room.pk), identity=str(serializer.validated_data["participant_identity"]), ) + except ParticipantNotFoundException: + return drf_response.Response( + {"error": "Participant not found"}, + status=drf_status.HTTP_404_NOT_FOUND, + ) except ParticipantsManagementException: return drf_response.Response( {"error": "Failed to remove participant"}, @@ -710,6 +726,11 @@ class RoomViewSet( identity=identity, attributes={"handRaisedAt": hand_raised_at}, ) + except ParticipantNotFoundException: + return drf_response.Response( + {"error": "Participant not found"}, + status=drf_status.HTTP_404_NOT_FOUND, + ) except ParticipantsManagementException: return drf_response.Response( {"error": "Failed to update participant hand state"}, @@ -744,6 +765,11 @@ class RoomViewSet( identity=identity, name=serializer.validated_data["name"], ) + except ParticipantNotFoundException: + return drf_response.Response( + {"error": "Participant not found"}, + status=drf_status.HTTP_404_NOT_FOUND, + ) except ParticipantsManagementException: return drf_response.Response( {"error": "Failed to rename participant"}, diff --git a/src/backend/core/services/participants_management.py b/src/backend/core/services/participants_management.py index 44b633d8..241d8f57 100644 --- a/src/backend/core/services/participants_management.py +++ b/src/backend/core/services/participants_management.py @@ -27,6 +27,10 @@ class ParticipantsManagementException(Exception): """Exception raised when a participant management operations fail.""" +class ParticipantNotFoundException(ParticipantsManagementException): + """Raised when the target participant does not exist in the room.""" + + class ParticipantsManagement: """Service for managing participants.""" @@ -47,6 +51,14 @@ class ParticipantsManagement: ) except TwirpError as e: + if e.code == "not_found": + logger.warning( + "Participant %s not found in room %s, skipping muting", + identity, + room_name, + ) + raise ParticipantNotFoundException("Participant does not exist") from e + logger.exception( "Unexpected error muting participant %s for room %s", identity, @@ -80,6 +92,14 @@ class ParticipantsManagement: RoomParticipantIdentity(room=room_name, identity=identity) ) except TwirpError as e: + if e.code == "not_found": + logger.warning( + "Participant %s not found in room %s, skipping removing", + identity, + room_name, + ) + raise ParticipantNotFoundException("Participant does not exist") from e + logger.exception( "Unexpected error removing participant %s for room %s", identity, @@ -117,6 +137,14 @@ class ParticipantsManagement: ) except TwirpError as e: + if e.code == "not_found": + logger.warning( + "Participant %s not found in room %s, skipping update", + identity, + room_name, + ) + raise ParticipantNotFoundException("Participant does not exist") from e + logger.exception( "Unexpected error updating participant %s for room %s", identity, diff --git a/src/backend/core/tests/rooms/test_api_rooms_participants_management.py b/src/backend/core/tests/rooms/test_api_rooms_participants_management.py index f6a67da5..c325c96a 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_participants_management.py +++ b/src/backend/core/tests/rooms/test_api_rooms_participants_management.py @@ -545,3 +545,55 @@ def test_remove_participant_unexpected_twirp_error(mock_livekit_client): assert response.data == {"error": "Failed to remove participant"} mock_livekit_client.aclose.assert_called_once() + + +def test_update_participant_not_found(mock_livekit_client): + """Test update participant returns 404 when the participant no longer exists in the room.""" + client = APIClient() + + mock_livekit_client.room.update_participant.side_effect = TwirpError( + msg="participant does not exist", code="not_found", status=404 + ) + + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = {"participant_identity": str(uuid4()), "name": "Test User"} + + url = reverse("rooms-update-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data == {"error": "Participant not found"} + + mock_livekit_client.aclose.assert_called_once() + + +def test_remove_participant_not_found(mock_livekit_client): + """Test remove participant returns 404 when the participant no longer exists in the room.""" + client = APIClient() + + mock_livekit_client.room.remove_participant.side_effect = TwirpError( + msg="participant does not exist", code="not_found", status=404 + ) + + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = {"participant_identity": str(uuid4())} + + url = reverse("rooms-remove-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data == {"error": "Participant not found"} + + mock_livekit_client.aclose.assert_called_once() diff --git a/src/backend/core/tests/rooms/test_api_rooms_rename_toggle.py b/src/backend/core/tests/rooms/test_api_rooms_rename_toggle.py index a4f5db2e..4546711f 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_rename_toggle.py +++ b/src/backend/core/tests/rooms/test_api_rooms_rename_toggle.py @@ -440,6 +440,22 @@ def test_toggle_hand_room_not_found(user): assert response.status_code == status.HTTP_404_NOT_FOUND +def test_toggle_hand_participant_not_found(mock_livekit_client, room, token): + """Test toggle hand returns 404 when the participant no longer exists in the room.""" + mock_livekit_client.room.update_participant.side_effect = TwirpError( + msg="participant does not exist", code="not_found", status=404 + ) + + client = APIClient() + url = reverse("rooms-toggle-hand", kwargs={"pk": room.id}) + response = client.post(url, {"raised": True, "token": token}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data == {"error": "Participant not found"} + + mock_livekit_client.aclose.assert_called_once() + + def test_rename_participant_malformed_token(room): """Test rename returns 403 when the LiveKit token is malformed.""" client = APIClient() @@ -461,3 +477,19 @@ def test_rename_participant_room_not_found(user): response = client.post(url, {"name": "John Doe", "token": token}, format="json") assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_rename_participant_not_found(mock_livekit_client, room, token): + """Test rename returns 404 when the participant no longer exists in the room.""" + mock_livekit_client.room.update_participant.side_effect = TwirpError( + msg="participant does not exist", code="not_found", status=404 + ) + + client = APIClient() + url = reverse("rooms-rename", kwargs={"pk": room.id}) + response = client.post(url, {"name": "John Doe", "token": token}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data == {"error": "Participant not found"} + + mock_livekit_client.aclose.assert_called_once()