consistent error class ServerError, test suite#744
Conversation
ServerError is now the error base class. we've also kept TwirpError as an alias. also added full api smoke test suite
davidzhao
requested review from
cloudwebrtc,
lukasIO and
xianshijing-lk
as code owners
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| client_timeout = aiohttp.ClientTimeout( | ||
| total=timeout if timeout else DEFAULT_RINGING_TIMEOUT | ||
| ) |
There was a problem hiding this comment.
🚩 accept_whatsapp_call timeout lacks the RINGING_TIMEOUT_MARGIN used by SIP service
The accept_whatsapp_call method uses DEFAULT_RINGING_TIMEOUT (30s) as the request timeout when wait_until_answered is set, without adding RINGING_TIMEOUT_MARGIN (2s). In contrast, the SIP service's create_sip_participant uses ring + RINGING_TIMEOUT_MARGIN to ensure the HTTP request outlasts the ringing window. If the server takes exactly 30s to ring, the connector request could time out at the boundary. This is pre-existing behavior (unchanged by this PR) but the inconsistency is worth noting since the PR cleaned up the DialRequest union that previously included AcceptWhatsAppCallRequest.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| def sip_status_code(self) -> Optional[int]: | ||
| """The SIP response code of the failed call, e.g. 486 (Busy Here).""" | ||
| raw = self.metadata.get("sip_status_code") | ||
| return int(raw) if raw is not None else None |
There was a problem hiding this comment.
nit: should we add a try/except for the int call?
| if not token and not api_key and not api_secret: | ||
| token = os.getenv("LIVEKIT_TOKEN") | ||
| if not token and not api_key and not api_secret: | ||
| api_key = os.getenv("LIVEKIT_API_KEY") | ||
| api_secret = os.getenv("LIVEKIT_API_SECRET") |
There was a problem hiding this comment.
🔍 Environment variable fallback for credentials is now all-or-nothing
The old code resolved api_key and api_secret independently from environment variables (api_key = api_key or os.getenv("LIVEKIT_API_KEY")). The new code at livekit-api/livekit/api/livekit_api.py:56-60 only consults env vars when all three of token, api_key, and api_secret are unset. This means a caller who previously did LiveKitAPI(api_key="mykey") and relied on LIVEKIT_API_SECRET being read from the environment will now get a ValueError. The comment says this is intentional to prevent ambient env vars from silently mixing with explicit args, but it is a breaking change for any user who relied on partial env-var resolution. Worth confirming this is acceptable for the user base.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
ServerError is now the error base class. we've also kept TwirpError as an alias.
also added full api smoke test suite