mirror of
https://github.com/suitenumerique/meet.git
synced 2026-07-26 11:58:53 +00:00
🥅(backend) refine Twirp error handling for participant operations
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.
This commit is contained in:
@@ -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"},
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user