mirror of
https://github.com/suitenumerique/meet.git
synced 2026-10-21 03:55:24 +00:00
🐛(backend) raise service errors when LiveKit times out
A timed-out call raised a bare TimeoutError past every service's TwirpError handler, so callers that recover from a failed LiveKit call, the permission checks and the webhook handlers among them, failed instead.
This commit is contained in:
@@ -52,8 +52,8 @@ class ParticipantsManagement:
|
||||
)
|
||||
)
|
||||
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
logger.warning(
|
||||
"Participant %s not found in room %s, skipping muting",
|
||||
identity,
|
||||
@@ -95,8 +95,8 @@ class ParticipantsManagement:
|
||||
await lkapi.room.remove_participant(
|
||||
RoomParticipantIdentity(room=room_name, identity=identity)
|
||||
)
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
logger.warning(
|
||||
"Participant %s not found in room %s, skipping removing",
|
||||
identity,
|
||||
@@ -140,8 +140,8 @@ class ParticipantsManagement:
|
||||
)
|
||||
)
|
||||
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
logger.warning(
|
||||
"Participant %s not found in room %s, skipping update",
|
||||
identity,
|
||||
@@ -203,8 +203,8 @@ class ParticipantsManagement:
|
||||
identity=identity,
|
||||
)
|
||||
)
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
raise ParticipantNotFoundException("Participant does not exist") from e
|
||||
|
||||
logger.exception(
|
||||
|
||||
@@ -80,8 +80,8 @@ class RoomManagement:
|
||||
)
|
||||
)
|
||||
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
raise RoomNotFoundException("Room does not exist") from e
|
||||
|
||||
logger.exception(
|
||||
@@ -108,8 +108,8 @@ class RoomManagement:
|
||||
try:
|
||||
await lkapi.room.delete_room(DeleteRoomRequest(room=room_name))
|
||||
logger.info("Deleted LiveKit room %s", room_name)
|
||||
except TwirpError as e:
|
||||
if e.code == "not_found":
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == "not_found":
|
||||
logger.warning(
|
||||
"Room %s not found in LiveKit, skipping deletion",
|
||||
room_name,
|
||||
|
||||
@@ -54,8 +54,8 @@ class SIPManagement:
|
||||
|
||||
try:
|
||||
await lkapi.sip.create_sip_dispatch_rule(create=request)
|
||||
except TwirpError as e:
|
||||
if e.code == TwirpErrorCode.ALREADY_EXISTS:
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
if isinstance(e, TwirpError) and e.code == TwirpErrorCode.ALREADY_EXISTS:
|
||||
raise DispatchRuleConflictError("Dispatch rule already exists") from e
|
||||
logger.exception(
|
||||
"Unexpected error creating dispatch rule for room %s", room.id
|
||||
@@ -83,7 +83,7 @@ class SIPManagement:
|
||||
existing_rules = await lkapi.sip.list_sip_dispatch_rule(
|
||||
list=ListSIPDispatchRuleRequest()
|
||||
)
|
||||
except TwirpError as e:
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
logger.exception("Failed to list dispatch rules for room %s", room_id)
|
||||
raise SIPException("Could not list dispatch rules") from e
|
||||
finally:
|
||||
@@ -155,7 +155,7 @@ class SIPManagement:
|
||||
|
||||
return True
|
||||
|
||||
except TwirpError as e:
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
logger.exception("Failed to delete dispatch rules for room %s", room_id)
|
||||
raise SIPException("Could not delete dispatch rules") from e
|
||||
|
||||
|
||||
@@ -17,7 +17,12 @@ from asgiref.sync import async_to_sync
|
||||
from livekit.api import TwirpError
|
||||
|
||||
from core.factories import UserFactory
|
||||
from core.services.room_management import RoomManagement
|
||||
from core.services.participants_management import (
|
||||
ParticipantsManagement,
|
||||
ParticipantsManagementException,
|
||||
)
|
||||
from core.services.room_management import RoomManagement, RoomManagementException
|
||||
from core.services.sip_management import SIPException, SIPManagement
|
||||
from core.utils import (
|
||||
NotificationError,
|
||||
create_livekit_client,
|
||||
@@ -152,7 +157,7 @@ def test_create_livekit_client_gives_up_on_a_quiet_livekit(fake_livekit, setting
|
||||
def delete_room():
|
||||
try:
|
||||
RoomManagement.delete_room("room-abc")
|
||||
except TimeoutError as error:
|
||||
except RoomManagementException as error:
|
||||
raised.append(error)
|
||||
|
||||
# A thread, so a call that never gives up fails the test rather than hangs it.
|
||||
@@ -162,6 +167,7 @@ def test_create_livekit_client_gives_up_on_a_quiet_livekit(fake_livekit, setting
|
||||
|
||||
assert not call.is_alive()
|
||||
assert len(raised) == 1
|
||||
assert isinstance(raised[0].__cause__, TimeoutError)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -181,6 +187,92 @@ def test_create_livekit_client_lets_an_answer_through(fake_livekit, settings):
|
||||
RoomManagement.delete_room("room-abc")
|
||||
|
||||
|
||||
class TimingOutClient:
|
||||
"""A LiveKit client whose every call times out."""
|
||||
|
||||
def __getattr__(self, name):
|
||||
return self
|
||||
|
||||
async def __call__(self, *args, **kwargs):
|
||||
raise TimeoutError
|
||||
|
||||
async def aclose(self):
|
||||
"""Nothing to close."""
|
||||
|
||||
|
||||
def delete_dispatch_rule():
|
||||
"""Reach the delete call, past the listing that comes first."""
|
||||
with mock.patch.object(
|
||||
SIPManagement, "_list_dispatch_rules_ids", mock.AsyncMock(return_value=["r"])
|
||||
):
|
||||
SIPManagement().delete_dispatch_rule("room")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("call", "expected"),
|
||||
[
|
||||
pytest.param(
|
||||
lambda: ParticipantsManagement().mute("room", "id", "track"),
|
||||
ParticipantsManagementException,
|
||||
id="mute",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: ParticipantsManagement().remove("room", "id"),
|
||||
ParticipantsManagementException,
|
||||
id="remove",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: ParticipantsManagement().update("room", "id", name="n"),
|
||||
ParticipantsManagementException,
|
||||
id="update",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: ParticipantsManagement().check_if_in_meeting("room", "id"),
|
||||
ParticipantsManagementException,
|
||||
id="check_if_in_meeting",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: RoomManagement.update_metadata("room", {}),
|
||||
RoomManagementException,
|
||||
id="update_metadata",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: RoomManagement.delete_room("room"),
|
||||
RoomManagementException,
|
||||
id="delete_room",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: SIPManagement().create_dispatch_rule(
|
||||
mock.Mock(pk="room", pin_code="1")
|
||||
),
|
||||
SIPException,
|
||||
id="create_dispatch_rule",
|
||||
),
|
||||
pytest.param(
|
||||
lambda: SIPManagement().has_dispatch_rule("room"),
|
||||
SIPException,
|
||||
id="has_dispatch_rule",
|
||||
),
|
||||
pytest.param(delete_dispatch_rule, SIPException, id="delete_dispatch_rule"),
|
||||
pytest.param(
|
||||
lambda: notify_participants("room", {}),
|
||||
NotificationError,
|
||||
id="notify_participants",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_a_livekit_timeout_raises_the_service_exception(call, expected):
|
||||
"""A timed-out call fails the way a LiveKit error does, so callers handle it."""
|
||||
with (
|
||||
mock.patch("core.utils.create_livekit_client", return_value=TimingOutClient()),
|
||||
pytest.raises(expected) as error,
|
||||
):
|
||||
call()
|
||||
|
||||
assert error.type is expected
|
||||
assert isinstance(error.value.__cause__, TimeoutError)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("verify_ssl", [True, False])
|
||||
def test_create_livekit_client_closes_its_session(verify_ssl, settings):
|
||||
"""Closing the client closes its session, whether SSL is verified or not."""
|
||||
|
||||
@@ -280,7 +280,7 @@ async def notify_participants(room_name: str, notification_data: dict):
|
||||
kind="RELIABLE",
|
||||
)
|
||||
)
|
||||
except TwirpError as e:
|
||||
except (TwirpError, TimeoutError) as e:
|
||||
raise NotificationError("Failed to notify room participants") from e
|
||||
finally:
|
||||
await lkapi.aclose()
|
||||
|
||||
Reference in New Issue
Block a user