fix(realtime): add reconnection resilience to send_raw()#3363
Conversation
send_raw() was missing the try/except that send() has — if the WebSocket disconnects mid-send, the data is silently lost instead of being enqueued for retry after reconnection.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad91393087
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw = data if isinstance(data, str) else data.decode("utf-8") | ||
| self._send_queue.enqueue(raw) |
There was a problem hiding this comment.
Preserve byte payloads when queueing failed raw sends
When send_raw() is called with bytes and the socket drops during send, this decodes the payload and stores it in SendQueue, whose flush path later calls send(str). That changes a binary WebSocket frame into a text frame for UTF-8 bytes, and for arbitrary binary bytes (for example audio chunks containing 0xff) raises UnicodeDecodeError before the original connection failure can be re-raised or the message queued. Since send_raw explicitly accepts bytes, the retry path should retain the original frame type instead of forcing UTF-8 text; the same new exception-path conversion appears in the sync and Responses variants as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good point — you are right that the .decode("utf-8") I introduced was incorrect, not just an inherited limitation. Fixed properly:
SendQueuenow storesbytes | strnatively. Byte length is computed aslen(data)for bytes andlen(data.encode("utf-8"))for str.- All four
send_rawpaths (sync + async × realtime + responses) enqueuedatadirectly without any decode, preserving the original frame type. - Added an end-to-end test that drives the real
_reconnect()+ flush path with a non-UTF-8 binary payload (b"\xff\xfe\x00audio"), confirms it is replayed byte-for-byte after reconnect, and verified the test fails (raisingUnicodeDecodeError) against the prior decode-to-UTF-8 implementation.
send_raw() accepts bytes | str, but the reconnect/retry path decoded bytes to UTF-8 before enqueueing. That turned binary WebSocket frames into text frames and raised UnicodeDecodeError for arbitrary binary payloads (e.g. audio chunks containing 0xff) before the original connection failure could surface. SendQueue now stores bytes | str natively and counts byte length per type, so send_raw() can enqueue the original payload unchanged on both the reconnecting and send-failure paths (sync + async, realtime + responses). Addresses Codex review feedback on openai#3363.
Add an end-to-end test that drives the real _reconnect() + flush path: a send_raw() with non-UTF-8 bytes fails on a dropped socket, gets queued, and is replayed byte-for-byte to the new connection after reconnect. Verified this fails (UnicodeDecodeError) against the prior decode-to-UTF-8 implementation.
Summary
send_raw()was missing the try/except thatsend()has for reconnection resiliencerealtime.pyandresponses.py