From 835e66817fb35d03c48c0626efe4b2117754cf76 Mon Sep 17 00:00:00 2001 From: Genseric Ghiro Date: Tue, 8 Sep 2026 12:41:51 -0400 Subject: [PATCH 1/2] Expose the SIP transfer failure reason on SipCallError Surface sip_transfer_reason / sip_transfer_id from the error metadata, and wrap transfer failures that carry no SIP status (ringing timeout, terminated subscription) as SipCallError too. --- livekit-api/livekit/api/sip_service.py | 7 ++-- livekit-api/livekit/api/twirp_client.py | 47 ++++++++++++++++++------- tests/api/test_livekitapi.py | 45 +++++++++++++++++++++++ 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/livekit-api/livekit/api/sip_service.py b/livekit-api/livekit/api/sip_service.py index 7cf67d23..c222cdf0 100644 --- a/livekit-api/livekit/api/sip_service.py +++ b/livekit-api/livekit/api/sip_service.py @@ -49,9 +49,10 @@ def _as_sip_error(err: ServerError) -> ServerError: - """Surface a SIP dialing failure as a SipCallError so callers can branch on - the SIP status; other failures (auth, validation) are returned unchanged.""" - if "sip_status_code" in err.metadata: + """Surface a SIP dialing or transfer failure as a SipCallError so callers can + branch on the SIP status or the transfer reason; other failures (auth, + validation) are returned unchanged.""" + if "sip_status_code" in err.metadata or "sip_transfer_reason" in err.metadata: return SipCallError.from_server_error(err) return err diff --git a/livekit-api/livekit/api/twirp_client.py b/livekit-api/livekit/api/twirp_client.py index 228c6d1d..efd5ffde 100644 --- a/livekit-api/livekit/api/twirp_client.py +++ b/livekit-api/livekit/api/twirp_client.py @@ -87,10 +87,20 @@ def __str__(self) -> str: return result +_SIP_META_KEYS = ( + "sip_status_code", + "sip_status", + "sip_transfer_reason", + "sip_transfer_id", + "error_details", +) + + class SipCallError(ServerError): """A :class:`ServerError` from a SIP dialing call (``create_sip_participant`` / - ``transfer_sip_participant``) that failed with a SIP response status. The SIP - code and reason are exposed as properties; any other error metadata remains + ``transfer_sip_participant``) that failed with a SIP response status, or a + transfer that failed for any other reason. The SIP code and reason, and the + transfer reason, are exposed as properties; any other error metadata remains available via :attr:`metadata`.""" @property @@ -109,25 +119,36 @@ def sip_status(self) -> Optional[str]: """The SIP reason phrase of the failed call, e.g. "Busy Here".""" return self.metadata.get("sip_status") + @property + def sip_transfer_reason(self) -> Optional[str]: + """Why a transfer failed, e.g. "STR_RINGING_TIMEOUT". Only set for + ``transfer_sip_participant``.""" + return self.metadata.get("sip_transfer_reason") + + @property + def sip_transfer_id(self) -> Optional[str]: + """The id of the failed transfer, for matching against SIP transfer logs.""" + return self.metadata.get("sip_transfer_id") + @classmethod def from_server_error(cls, err: ServerError) -> "SipCallError": return cls(err.code, err.message, status=err.status, metadata=err.metadata) def __str__(self) -> str: code = self.metadata.get("sip_status_code") - if code is None: + transfer_reason = self.metadata.get("sip_transfer_reason") + if code is None and transfer_reason is None: return super().__str__() # A clear, SIP-specific representation, including any extra metadata. - reason = self.metadata.get("sip_status") - result = f"SIP call failed: {code}" - if reason: - result += f" {reason}" - result += f" ({self.code})" - extra = { - k: v - for k, v in self.metadata.items() - if k not in ("sip_status_code", "sip_status", "error_details") - } + parts = [] + if transfer_reason is not None: + parts.append(transfer_reason) + if code is not None: + reason = self.metadata.get("sip_status") + parts.append(f"{code} {reason}" if reason else str(code)) + what = "SIP transfer failed" if transfer_reason is not None else "SIP call failed" + result = f"{what}: {', '.join(parts)} ({self.code})" + extra = {k: v for k, v in self.metadata.items() if k not in _SIP_META_KEYS} if extra: result += " [" + ", ".join(f"{k}={v}" for k, v in extra.items()) + "]" return result diff --git a/tests/api/test_livekitapi.py b/tests/api/test_livekitapi.py index f84bfa48..bf2e5a7e 100644 --- a/tests/api/test_livekitapi.py +++ b/tests/api/test_livekitapi.py @@ -38,6 +38,7 @@ import livekit.api as api from livekit.api import SipCallError, ServerError +from livekit.api.sip_service import _as_sip_error from livekit.protocol.rtc import SessionDescription BASE = os.getenv("LK_TEST_SERVER_URL", "http://127.0.0.1:9999") @@ -564,6 +565,50 @@ def test_sip_no_answer(): assert err.sip_status_code == 408 +# -- transfer failures surface the reason ------------------------------------- + +# These build the error directly: the reason depends on how the far end +# behaves, which the mock cannot stage. + + +def _transfer_error(**metadata: str) -> ServerError: + return _as_sip_error( + ServerError( + "deadline_exceeded", "call transfer failed", status=408, metadata=metadata + ) + ) + + +def test_sip_transfer_reason(): + err = _transfer_error( + sip_transfer_reason="STR_RINGING_TIMEOUT", sip_transfer_id="STR_abc" + ) + assert isinstance(err, SipCallError) + assert err.sip_transfer_reason == "STR_RINGING_TIMEOUT" + assert err.sip_transfer_id == "STR_abc" + # No SIP response was involved in this failure. + assert err.sip_status_code is None + assert "STR_RINGING_TIMEOUT" in str(err) + + +def test_sip_transfer_rejected_reports_reason_and_status(): + err = _transfer_error( + sip_transfer_reason="STR_REJECTED", + sip_status_code="486", + sip_status="Busy Here", + ) + assert err.sip_transfer_reason == "STR_REJECTED" + assert err.sip_status_code == 486 + assert err.sip_status == "Busy Here" + assert "STR_REJECTED" in str(err) + assert "486" in str(err) and "Busy Here" in str(err) + + +def test_non_sip_error_is_unchanged(): + err = ServerError("unauthenticated", "bad token", status=401) + assert _as_sip_error(err) is err + + # -- cross-cutting: client-side dial timeout ---------------------------------- From 08722724c6077a6b3badf37786d8a3b9b082ca52 Mon Sep 17 00:00:00 2001 From: Genseric Ghiro Date: Tue, 8 Sep 2026 13:57:48 -0400 Subject: [PATCH 2/2] Removing confusing comment --- tests/api/test_livekitapi.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/api/test_livekitapi.py b/tests/api/test_livekitapi.py index 0d6e4070..59dac849 100644 --- a/tests/api/test_livekitapi.py +++ b/tests/api/test_livekitapi.py @@ -567,9 +567,6 @@ def test_sip_no_answer(): # -- transfer failures surface the reason ------------------------------------- -# These build the error directly: the reason depends on how the far end -# behaves, which the mock cannot stage. - def _transfer_error(**metadata: str) -> ServerError: return _as_sip_error(