(deepgram stt): detect a silently dropped socket instead of hanging - #7206
Conversation
On a half-open connection (no FIN, no RST) the v1 Nova-3 SpeechStream could sit on a dead socket indefinitely: the session stayed open, no transcripts arrived, and no error was ever emitted. Three gaps. 1. `stt.py::_connect_ws` opened the socket without `heartbeat`. aiohttp defaults both `heartbeat` and `receive_timeout` to None, so `recv_task` parks on `ws.receive()` forever. The reconnect loop in `_run` is exception-driven, as is the retry in `RecognizeStream._main_task`, so with nothing raising neither ever runs. stt_v2 already passes `heartbeat=30.0`; v1 was the only Deepgram stream left unbounded. 2. `stt.py::keepalive_task` caught every exception, logged a warning and returned. When no audio is flowing that write is the only thing touching the socket, so it is where a drop surfaces first, and swallowing it left the stream alive on a connection that was gone. Connection errors now re-raise as a retryable APIConnectionError, symmetric with what send_task and recv_task already do after livekit#6429. 3. `recv_task` in both stt.py and stt_v2.py treated WSMsgType.ERROR as an unexpected message type: it logged and continued. That is the shape a heartbeat timeout actually arrives in, so the loop only recovered because aiohttp reports CLOSED on the next iteration, and the reason on `ws.exception()` was discarded in favour of a bare 1006. This one applies to Flux too, which already had the heartbeat. Verified end to end against api.deepgram.com through a proxy that holds both sockets open and stops delivering. Before: one socket, no reconnect, no error, silent for the full 135s window. After: detected in 46s and transcribing again 6s later, reported as deepgram connection lost (caused by ServerTimeoutError: No PONG received after 15.0 seconds) Three regression tests, all of which fail on main.
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| except (aiohttp.ClientError, ConnectionError) as e: | ||
| # when no audio is flowing this write is the only thing touching the | ||
| # socket, so it is where a drop surfaces first. if the close is | ||
| # expected just return; otherwise re-raise as a retryable APIError so | ||
| # _main_task reconnects, symmetric with send_task and recv_task. | ||
| if closing_ws or self._session.closed: | ||
| return | ||
| raise APIConnectionError("deepgram connection closed unexpectedly") from e |
There was a problem hiding this comment.
🟨 Keepalive failures expose raw exception data
Keepalive failures preserve raw aiohttp exceptions in __cause__. Task and retry logs can expose URLs, credentials, or provider data from that cause.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I checked this against the types that can actually reach here, and I don't think it holds for this path, though the concern class is a real one in this repo (#6739, #7031).
send_str() on an established websocket can raise ClientConnectionResetError, ClientOSError or ConnectionResetError. I rendered each through APIError.__str__ and checked both str() and repr() for an API key and for the URL:
| cause | str() |
repr() |
|---|---|---|
ClientConnectionResetError |
clean | clean |
ClientOSError |
clean | clean |
ConnectionResetError |
clean | clean |
ClientResponseError (for contrast) |
clean | leaks the key |
ClientResponseError is the one that carries credentials, because RequestInfo.repr prints the Authorization header. That is the #6739 leak. It is raised during the HTTP request/response cycle, not by a frame write on an already-upgraded socket, so it cannot arise here, and it is already guarded at the site where it can, in _connect_ws with from None.
Worth noting too that APIError.__str__ deliberately walks __cause__, on the reasoning that the root of the chain is the only place the real failure is named. Dropping the cause here would leave _main_task logging a bare "connection closed unexpectedly" with no indication of why.
Happy to add an explicit isinstance guard against ClientResponseError if you would prefer it belt-and-braces, but as written I believe it is unreachable.
| if msg.type == aiohttp.WSMsgType.ERROR: | ||
| if closing_ws or self._session.closed: | ||
| return | ||
|
|
||
| # the heartbeat closes the socket when a ping goes unanswered, | ||
| # and that surfaces here rather than as a close frame. | ||
| # ws.exception() is the only place the reason survives. | ||
| raise APIConnectionError("deepgram connection lost") from ws.exception() |
There was a problem hiding this comment.
🟨 Heartbeat failures expose raw exception data
WSMsgType.ERROR preserves ws.exception() in __cause__. Task and retry logs can expose URLs, credentials, or provider data in both streams.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Same as the keepalive comment above: I don't think this one reaches a credential-bearing exception either.
ws.exception() on a websocket that has already completed its 101 upgrade returns reader and protocol errors: ServerTimeoutError for the heartbeat case this branch exists to catch, plus ClientPayloadError and WebSocketError. None of those carry a URL or headers in str() or repr(). The type that does, ClientResponseError, belongs to the handshake and is already handled with from None in _connect_ws.
Preserving the cause is the whole point of this branch. Without it a heartbeat timeout surfaces as a bare 1006 "closed unexpectedly", which is what happens on main today; with it the session error reads:
deepgram connection lost (caused by ServerTimeoutError: No PONG received after 15.0 seconds)
3 participants
On a half-open connection (no FIN, no RST) the v1 Nova-3
SpeechStreamcan sit on a dead socket indefinitely: the session stays open, no transcripts arrive, and no error is ever emitted.Three gaps
1. No heartbeat.
stt.py::_connect_wsopens the socket withoutheartbeat, and aiohttp defaults bothheartbeatandreceive_timeouttoNone, sorecv_taskparks onws.receive()forever. The reconnect loop in_runis exception-driven, as is the retry inRecognizeStream._main_task, so with nothing raising neither one runs.stt_v2already passesheartbeat=30.0; v1 was the only Deepgram stream left unbounded.2. The keepalive swallowed drops.
keepalive_taskcaught every exception, logged a warning and returned. When no audio is flowing that write is the only thing touching the socket, so it is where a drop surfaces first. Connection errors now re-raise as a retryableAPIConnectionError, symmetric with whatsend_taskandrecv_taskalready do after #6429.3.
WSMsgType.ERRORwas treated as an unexpected message type inrecv_task, logged and stepped over. That is the shape a heartbeat timeout actually arrives in, so the loop only recovered because aiohttp reportsCLOSEDon the next pass, and the reason onws.exception()was discarded in favour of a bare 1006. This one applies tostt_v2.pyas well, which already had the heartbeat.Verification
Run against
api.deepgram.comthrough a proxy that holds both sockets open and stops delivering, so the client's writes keep succeeding and nothing ever comes back:The error now carries its cause:
Three regression tests added, all of which fail on
main.One open question
Detection takes ~46s (30s ping interval plus a 15s pong timeout). That is parity with the
heartbeat=30.0stt_v2already ships, so I kept it rather than changing both, but 46s of dead air is a long time on a live call. Happy to lower it if you would prefer.🤖 Generated with Claude Code