Skip to content

Commit 1db942a

Browse files
davidzhaoclaude
andcommitted
rpc interceptors: record deadline expiry independently of the unwinding error
wait_for reported whatever the chain raised while being cancelled; an interceptor that catches the cancellation and raises during cleanup could turn a deadline expiry into APPLICATION_ERROR (or, via the TimeoutError tag, back into a mislabelled application timeout). The chain now runs as its own future under a call_later deadline that sets a flag before cancelling it. Once the flag is set, anything the chain raises while unwinding is reported as RESPONSE_TIMEOUT; a CancelledError without the flag is external cancellation and stays RECIPIENT_DISCONNECTED; any exception before the deadline, TimeoutError included, propagates as an application error. The _ChainTimeoutError tag is no longer needed. Tests: cleanup raising TimeoutError and RuntimeError after the deadline both map to RESPONSE_TIMEOUT; the existing deadline, in-chain timeout and outside cancellation cases still hold. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 6c6540e commit 1db942a

2 files changed

Lines changed: 61 additions & 25 deletions

File tree

‎livekit-rtc/livekit/rtc/participant.py‎

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,6 @@ def disconnect_reason(
238238
RpcHandler = Callable[["RpcInvocationData"], Union[Awaitable[Optional[str]], Optional[str]]]
239239

240240

241-
class _ChainTimeoutError(Exception):
242-
"""A ``TimeoutError`` raised *inside* the incoming RPC chain (by the handler or an
243-
interceptor), wrapped so it is distinguishable from the response deadline expiring."""
244-
245-
def __init__(self, error: asyncio.TimeoutError) -> None:
246-
super().__init__(str(error))
247-
self.error = error
248-
249-
250241
F = TypeVar(
251242
"F", bound=Callable[[RpcInvocationData], Union[Awaitable[Optional[str]], Optional[str]]]
252243
)
@@ -642,30 +633,46 @@ async def _run_incoming_chain(self, invocation: RpcInvocationData) -> Optional[s
642633
643634
The deadline covers the whole chain, so time an interceptor spends before or after
644635
``next`` counts against it; when it passes, the chain is cancelled and the caller
645-
gets ``RESPONSE_TIMEOUT``. Cancellation from outside (the room disconnecting) maps to
646-
``RECIPIENT_DISCONNECTED``, as before.
636+
gets ``RESPONSE_TIMEOUT``, whatever the chain raises while unwinding. Cancellation
637+
from outside (the room disconnecting) maps to ``RECIPIENT_DISCONNECTED``, as before.
647638
648-
A ``TimeoutError`` raised by the handler or an interceptor itself (an HTTP client
649-
timing out, say) is not the response deadline: it propagates as an application
650-
error rather than being reported to the caller as ``RESPONSE_TIMEOUT``.
639+
A ``TimeoutError`` raised by the handler or an interceptor itself before the
640+
deadline (an HTTP client timing out, say) is not the response deadline: it
641+
propagates as an application error rather than being reported as
642+
``RESPONSE_TIMEOUT``.
651643
"""
652644
handle = _chain_incoming(list(self._rpc_interceptors), self._invoke_rpc_handler)
645+
loop = asyncio.get_running_loop()
646+
# ensure_future: `next` continuations are typed as Awaitable, not Coroutine
647+
chain_task: asyncio.Future[Optional[str]] = asyncio.ensure_future(handle(invocation))
653648

654-
async def _guarded() -> Optional[str]:
655-
try:
656-
return await handle(invocation)
657-
except asyncio.TimeoutError as e:
658-
# tag it so it cannot be mistaken for wait_for's own deadline expiry below
659-
raise _ChainTimeoutError(e) from e
649+
# the deadline is recorded independently of whatever the chain raises while it
650+
# unwinds, so a TimeoutError (or anything else) from cancellation cleanup cannot be
651+
# mistaken for an application failure, nor a genuine in-chain timeout for the deadline
652+
deadline_fired = False
660653

654+
def _on_deadline() -> None:
655+
nonlocal deadline_fired
656+
deadline_fired = True
657+
chain_task.cancel()
658+
659+
deadline = loop.call_later(invocation.response_timeout, _on_deadline)
661660
try:
662-
return await asyncio.wait_for(_guarded(), timeout=invocation.response_timeout)
663-
except _ChainTimeoutError as e:
664-
raise e.error from e.error.__cause__
665-
except asyncio.TimeoutError:
666-
raise RpcError._built_in(RpcError.ErrorCode.RESPONSE_TIMEOUT) from None
661+
return await chain_task
667662
except asyncio.CancelledError:
663+
if deadline_fired:
664+
raise RpcError._built_in(RpcError.ErrorCode.RESPONSE_TIMEOUT) from None
665+
# cancelled from outside: awaiting propagated the cancel into the chain; let it
666+
# finish unwinding before answering the caller
667+
if not chain_task.done():
668+
await asyncio.wait([chain_task])
668669
raise RpcError._built_in(RpcError.ErrorCode.RECIPIENT_DISCONNECTED) from None
670+
except Exception:
671+
if deadline_fired:
672+
raise RpcError._built_in(RpcError.ErrorCode.RESPONSE_TIMEOUT) from None
673+
raise
674+
finally:
675+
deadline.cancel()
669676

670677
async def _invoke_rpc_handler(self, invocation: RpcInvocationData) -> Optional[str]:
671678
"""Run the registered handler for ``invocation`` (the innermost step of the chain).

‎livekit-rtc/tests/test_rpc_interceptors.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,35 @@ async def handler_times_out(data: RpcInvocationData) -> str:
272272
await lp2._run_incoming_chain(RpcInvocationData("r2", "alice", "{}", 5.0, method="m"))
273273

274274

275+
@pytest.mark.parametrize("cleanup_error", [asyncio.TimeoutError, RuntimeError])
276+
async def test_cleanup_errors_after_the_deadline_are_still_a_timeout(
277+
cleanup_error: type[BaseException],
278+
) -> None:
279+
"""Once the deadline fired, whatever the chain raises while unwinding is not the
280+
caller's business: they get RESPONSE_TIMEOUT."""
281+
lp = _participant()
282+
283+
class RaisesOnCancel(rtc.RpcInterceptor):
284+
async def intercept_incoming(
285+
self, invocation: RpcInvocationData, next: IncomingRpcNext
286+
) -> Optional[str]:
287+
try:
288+
return await next(invocation)
289+
except asyncio.CancelledError:
290+
raise cleanup_error("cleanup failed while cancelling") from None
291+
292+
async def slow(data: RpcInvocationData) -> str:
293+
await asyncio.sleep(10)
294+
return "never"
295+
296+
lp.add_rpc_interceptor(RaisesOnCancel())
297+
lp._rpc_handlers["slow"] = slow
298+
299+
with pytest.raises(rtc.RpcError) as info:
300+
await lp._run_incoming_chain(RpcInvocationData("r1", "alice", "{}", 0.02, method="slow"))
301+
assert info.value.code == rtc.RpcError.ErrorCode.RESPONSE_TIMEOUT
302+
303+
275304
async def test_outside_cancellation_maps_to_recipient_disconnected() -> None:
276305
lp = _participant()
277306
started = asyncio.Event()

0 commit comments

Comments
 (0)