Skip to content

Commit f079868

Browse files
feat(websocket): recover queued binary frames after reconnect (#4191)
## Summary During reconnect, `send_raw(bytes)` used to decode queued bytes as UTF-8. A valid UTF-8 payload changed from a binary frame to a text frame. Other bytes raised `UnicodeDecodeError` before the SDK could send them. Preserve each frame and its place in the queue. If recovery stops, `unsent_frames` returns the original text and bytes in order. `unsent_messages` keeps its existing string-list interface. ## Verification The previous transport failed 28 wire cases. The revised transport passed 357 queue, recovery and session tests and 328 URL and security tests. The new sync and async tests use a local WebSocket peer across all seven bindings. Scoped Ruff checks passed. The full local formatter stopped before formatting because the configured registry differed from the checked-in lockfile. Local mypy reported the same eight optional Google import errors on both revisions. Queued text and binary frames remain available through `unsent_frames` in their original order after a reconnect fails. `unsent_messages` keeps its text-only compatibility list, including valid UTF-8 binary as text. Ordinary exception-attribute snapshots now render a fixed redacted holder. Explicit recovery access still returns the data. Release as a minor version. Pyright users whose subclasses redeclare `unsent_messages: list[str]` should remove that redundant declaration and use the inherited writable property. Runtime reads, mutation, assignment, and assignment before `super().__init__` remain supported. This follows README's static-typing minor-release policy. The `feat(websocket):` title uses this repository's standard release-please feature classification; no breaking marker or version bump is included here. Validation: 109 focused queue/reconnect/wire tests pass in both SDK checkouts, including attribute snapshots, recovery list identity and writes, and a subclass that assigns before parent initialization. Pinned Pyright and mypy pass for the changed source and tests; scoped Ruff and git diff checks pass. --------- Co-authored-by: Mark Stuart <323302876+markstuart-oai@users.noreply.github.com>
1 parent 8559299 commit f079868

6 files changed

Lines changed: 300 additions & 50 deletions

File tree

‎src/openai/_exceptions.py‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from __future__ import annotations
22

33
from typing import TYPE_CHECKING, Any, Optional, cast
4-
from typing_extensions import Literal
4+
from typing_extensions import Literal, override
55

66
import httpx2
77

@@ -197,14 +197,65 @@ class InvalidWebhookSignatureError(ValueError):
197197
"""Raised when a webhook signature is invalid, meaning the computed signature does not match the expected signature."""
198198

199199

200+
class _WebSocketRecoveryData:
201+
__slots__ = ("frames", "messages")
202+
203+
def __init__(self, frames: list[str] | list[str | bytes], messages: list[str]) -> None:
204+
self.frames = frames
205+
self.messages = messages
206+
207+
@override
208+
def __repr__(self) -> str:
209+
return "<redacted WebSocket recovery data>"
210+
211+
200212
class WebSocketConnectionClosedError(OpenAIError):
201-
"""Raised when a WebSocket connection closes with unsent messages."""
213+
"""Raised when a WebSocket connection closes with unsent messages.
202214
203-
unsent_messages: list[str]
215+
`unsent_frames` preserves the text and binary frames in their original order.
216+
`unsent_messages` retains the text messages, including UTF-8 binary frames
217+
represented as text for compatibility.
218+
"""
204219

205-
def __init__(self, message: str, *, unsent_messages: list[str]) -> None:
220+
def __init__(self, message: str, *, unsent_messages: list[str] | list[str | bytes]) -> None:
206221
super().__init__(message)
207-
self.unsent_messages = unsent_messages
222+
messages: list[str]
223+
if all(isinstance(data, str) for data in unsent_messages):
224+
messages = cast("list[str]", unsent_messages)
225+
else:
226+
messages = []
227+
for data in unsent_messages:
228+
if isinstance(data, str):
229+
messages.append(data)
230+
else:
231+
try:
232+
messages.append(data.decode("utf-8"))
233+
except UnicodeDecodeError:
234+
# Opaque binary remains available in unsent_frames.
235+
pass
236+
# Keep raw lists out of ordinary exception-attribute snapshots.
237+
# Explicit recovery access remains available through the properties.
238+
self._recovery = _WebSocketRecoveryData(unsent_messages, messages)
239+
240+
@property
241+
def unsent_messages(self) -> list[str]:
242+
return self._recovery.messages
243+
244+
@unsent_messages.setter
245+
def unsent_messages(self, messages: list[str]) -> None:
246+
if hasattr(self, "_recovery"):
247+
self._recovery.messages = messages
248+
else:
249+
# Subclasses could assign the original writable field before super().__init__.
250+
self._recovery = _WebSocketRecoveryData([], messages)
251+
252+
@property
253+
def unsent_frames(self) -> list[str] | list[str | bytes]:
254+
return self._recovery.frames
255+
256+
@unsent_frames.setter
257+
def unsent_frames(self, frames: list[str] | list[str | bytes]) -> None:
258+
self._recovery.frames = frames
208259

209260

210261
class WebSocketQueueFullError(OpenAIError):

‎src/openai/_send_queue.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,32 @@
1212
class SendQueue:
1313
"""Bounded byte-size queue for outgoing WebSocket messages.
1414
15-
Messages are stored as pre-serialized strings. The queue enforces a
15+
Messages retain their text or binary frame type. The queue enforces a
1616
maximum byte budget so that unbounded buffering cannot occur during
1717
reconnection windows.
1818
"""
1919

2020
def __init__(self, max_bytes: int = 1_048_576) -> None:
21-
self._queue: list[tuple[str, int]] = [] # (data, byte_length)
21+
self._queue: list[tuple[str | bytes, int]] = [] # (data, byte_length)
2222
self._bytes: int = 0
2323
self._max_bytes = max_bytes
2424
self._lock = threading.Lock()
2525
self._flush_done: threading.Event | None = None
2626

27-
def enqueue(self, data: str) -> None:
27+
def enqueue(self, data: str | bytes) -> None:
2828
"""Append *data* to the queue.
2929
3030
Raises :class:`WebSocketQueueFullError` if the message would
3131
exceed the byte-size limit.
3232
"""
33-
byte_length = len(data.encode("utf-8"))
33+
byte_length = len(data.encode("utf-8") if isinstance(data, str) else data)
3434
with self._lock:
3535
if self._bytes + byte_length > self._max_bytes:
3636
raise WebSocketQueueFullError("send queue is full, message discarded")
3737
self._queue.append((data, byte_length))
3838
self._bytes += byte_length
3939

40-
def flush_sync(self, send: typing.Callable[[str], object], *, requeue_failed: bool = True) -> None:
40+
def flush_sync(self, send: typing.Callable[[str | bytes], object], *, requeue_failed: bool = True) -> None:
4141
"""Send every queued message via *send*.
4242
4343
If *send* raises, the failing message and all subsequent messages
@@ -63,7 +63,7 @@ def flush_sync(self, send: typing.Callable[[str], object], *, requeue_failed: bo
6363
self._end_flush(pending)
6464

6565
async def flush_async(
66-
self, send: typing.Callable[[str], typing.Awaitable[object]], *, requeue_failed: bool = True
66+
self, send: typing.Callable[[str | bytes], typing.Awaitable[object]], *, requeue_failed: bool = True
6767
) -> None:
6868
"""Async variant of :meth:`flush_sync`."""
6969
while isinstance(pending := self._begin_flush(), threading.Event):
@@ -86,7 +86,7 @@ async def flush_async(
8686
finally:
8787
self._end_flush(pending)
8888

89-
def _begin_flush(self) -> deque[tuple[str, int]] | threading.Event:
89+
def _begin_flush(self) -> deque[tuple[str | bytes, int]] | threading.Event:
9090
with self._lock:
9191
if self._flush_done is not None:
9292
return self._flush_done
@@ -96,14 +96,14 @@ def _begin_flush(self) -> deque[tuple[str, int]] | threading.Event:
9696
# Pending messages remain charged until their sends succeed.
9797
return pending
9898

99-
def _end_flush(self, pending: deque[tuple[str, int]]) -> None:
99+
def _end_flush(self, pending: deque[tuple[str | bytes, int]]) -> None:
100100
with self._lock:
101101
self._queue = list(pending) + self._queue
102102
assert self._flush_done is not None
103103
self._flush_done.set()
104104
self._flush_done = None
105105

106-
def drain(self) -> list[str]:
106+
def drain(self) -> list[str | bytes]:
107107
"""Remove and return all queued messages."""
108108
with self._lock:
109109
items = [data for data, _ in self._queue]

‎src/openai/_websocket_connection.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ def __init__(
5151

5252
def send_raw(self, data: bytes | str) -> None:
5353
if self._is_reconnecting:
54-
raw = data if isinstance(data, str) else data.decode("utf-8")
55-
self._send_queue.enqueue(raw)
54+
self._send_queue.enqueue(data)
5655
return
5756
self._connection.send(data)
5857

@@ -206,8 +205,7 @@ def __init__(
206205

207206
async def send_raw(self, data: bytes | str) -> None:
208207
if self._is_reconnecting:
209-
raw = data if isinstance(data, str) else data.decode("utf-8")
210-
self._send_queue.enqueue(raw)
208+
self._send_queue.enqueue(data)
211209
return
212210
await self._connection.send(data)
213211

@@ -306,7 +304,7 @@ async def _attempt_reconnect(self, close_code: int) -> bool:
306304
async def _flush_send_queue(self) -> None:
307305
"""Send all queued messages over the current connection."""
308306

309-
async def _send(data: str) -> None:
307+
async def _send(data: str | bytes) -> None:
310308
await self._connection.send(data)
311309

312310
try:

‎tests/test_send_queue.py‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def test_flush_sync(self) -> None:
4747
q.enqueue("b")
4848
q.enqueue("c")
4949

50-
sent: list[str] = []
50+
sent: list[str | bytes] = []
5151
q.flush_sync(sent.append)
5252
assert sent == ["a", "b", "c"]
5353
assert len(q) == 0
@@ -58,9 +58,9 @@ def test_flush_sync_requeues_on_failure(self) -> None:
5858
q.enqueue("b")
5959
q.enqueue("c")
6060

61-
sent: list[str] = []
61+
sent: list[str | bytes] = []
6262

63-
def failing_send(data: str) -> None:
63+
def failing_send(data: str | bytes) -> None:
6464
if data == "b":
6565
raise RuntimeError("send failed")
6666
sent.append(data)
@@ -79,9 +79,9 @@ async def test_flush_async(self) -> None:
7979
q.enqueue("a")
8080
q.enqueue("b")
8181

82-
sent: list[str] = []
82+
sent: list[str | bytes] = []
8383

84-
async def async_send(data: str) -> None:
84+
async def async_send(data: str | bytes) -> None:
8585
sent.append(data)
8686

8787
await q.flush_async(async_send)
@@ -95,9 +95,9 @@ async def test_flush_async_requeues_on_failure(self) -> None:
9595
q.enqueue("b")
9696
q.enqueue("c")
9797

98-
sent: list[str] = []
98+
sent: list[str | bytes] = []
9999

100-
async def failing_send(data: str) -> None:
100+
async def failing_send(data: str | bytes) -> None:
101101
if data == "b":
102102
raise RuntimeError("send failed")
103103
sent.append(data)
@@ -116,7 +116,7 @@ def test_flush_sync_preserves_new_items_on_failure(self) -> None:
116116
q.enqueue("a")
117117
q.enqueue("b")
118118

119-
def failing_send(data: str) -> None:
119+
def failing_send(data: str | bytes) -> None:
120120
if data == "b":
121121
# Simulate another thread enqueuing during flush
122122
q.enqueue("new")
@@ -130,10 +130,10 @@ def failing_send(data: str) -> None:
130130
assert remaining == ["b", "new"]
131131

132132

133-
async def flush_in_background(q: SendQueue, send: Callable[[str], None], asynchronous: bool) -> None:
133+
async def flush_in_background(q: SendQueue, send: Callable[[str | bytes], None], asynchronous: bool) -> None:
134134
if asynchronous:
135135

136-
async def async_send(data: str) -> None:
136+
async def async_send(data: str | bytes) -> None:
137137
await asyncio.to_thread(send, data)
138138

139139
await q.flush_async(async_send)
@@ -155,7 +155,7 @@ async def test_repeated_failed_flushes_remain_bounded(asynchronous: bool) -> Non
155155
for attempt in range(3):
156156
entered, release = threading.Event(), threading.Event()
157157

158-
def send(data: str, entered: threading.Event = entered, release: threading.Event = release) -> None:
158+
def send(data: str | bytes, entered: threading.Event = entered, release: threading.Event = release) -> None:
159159
assert data == "éé"
160160
entered.set()
161161
assert release.wait(5)
@@ -174,7 +174,7 @@ def send(data: str, entered: threading.Event = entered, release: threading.Event
174174
await task
175175
assert q._bytes == 6
176176

177-
sent: list[str] = []
177+
sent: list[str | bytes] = []
178178
await flush_in_background(q, sent.append, asynchronous)
179179
assert sent == ["éé", "a", "b"]
180180
assert q._bytes == 0
@@ -189,9 +189,9 @@ async def test_partial_flush_releases_only_successful_bytes(asynchronous: bool,
189189
q.enqueue("é")
190190
q.enqueue("bb")
191191
entered, release = threading.Event(), threading.Event()
192-
sent: list[str] = []
192+
sent: list[str | bytes] = []
193193

194-
def send(data: str) -> None:
194+
def send(data: str | bytes) -> None:
195195
if data == "bb":
196196
entered.set()
197197
assert release.wait(5)
@@ -225,7 +225,7 @@ async def test_drain_during_flush_keeps_pending_bytes_charged(asynchronous: bool
225225
q.enqueue("aaa")
226226
entered, release = threading.Event(), threading.Event()
227227

228-
def send(_data: str) -> None:
228+
def send(_data: str | bytes) -> None:
229229
entered.set()
230230
assert release.wait(5)
231231
raise RuntimeError("fake send failure")
@@ -256,17 +256,17 @@ async def test_overlapping_flushes_preserve_order(first_async: bool, second_asyn
256256
q.enqueue("a")
257257
q.enqueue("b")
258258
entered, release, second_sent = threading.Event(), threading.Event(), threading.Event()
259-
sent: list[str] = []
259+
sent: list[str | bytes] = []
260260

261-
def first_send(data: str) -> None:
261+
def first_send(data: str | bytes) -> None:
262262
if data == "a":
263263
entered.set()
264264
assert release.wait(5)
265265
if fail:
266266
raise RuntimeError("fake send failure")
267267
sent.append(data)
268268

269-
def second_send(data: str) -> None:
269+
def second_send(data: str | bytes) -> None:
270270
second_sent.set()
271271
sent.append(data)
272272

@@ -296,7 +296,7 @@ async def test_cancelled_flush_restores_unsent_messages() -> None:
296296
q.enqueue("b")
297297
entered = asyncio.Event()
298298

299-
async def send(data: str) -> None:
299+
async def send(data: str | bytes) -> None:
300300
if data == "b":
301301
entered.set()
302302
await asyncio.Event().wait()
@@ -309,7 +309,7 @@ async def send(data: str) -> None:
309309
await task
310310
with pytest.raises(WebSocketQueueFullError):
311311
q.enqueue("d")
312-
sent: list[str] = []
312+
sent: list[str | bytes] = []
313313
q.flush_sync(sent.append)
314314
assert sent == ["b", "cc"]
315315
assert q._bytes == 0
@@ -320,14 +320,14 @@ async def test_cancelled_flush_waiter_does_not_strand_ownership() -> None:
320320
q = SendQueue(max_bytes=2)
321321
q.enqueue("a")
322322
entered, release = asyncio.Event(), asyncio.Event()
323-
sent: list[str] = []
323+
sent: list[str | bytes] = []
324324

325-
async def first_send(data: str) -> None:
325+
async def first_send(data: str | bytes) -> None:
326326
entered.set()
327327
await release.wait()
328328
sent.append(data)
329329

330-
async def second_send(data: str) -> None:
330+
async def second_send(data: str | bytes) -> None:
331331
sent.append(data)
332332

333333
first = asyncio.create_task(q.flush_async(first_send))
@@ -350,7 +350,7 @@ def test_interrupted_sync_flush_restores_unsent_messages() -> None:
350350
q.enqueue("a")
351351
q.enqueue("b")
352352

353-
def send(data: str) -> None:
353+
def send(data: str | bytes) -> None:
354354
if data == "b":
355355
raise KeyboardInterrupt
356356

@@ -359,7 +359,7 @@ def send(data: str) -> None:
359359
q.enqueue("c")
360360
with pytest.raises(WebSocketQueueFullError):
361361
q.enqueue("d")
362-
sent: list[str] = []
362+
sent: list[str | bytes] = []
363363
q.flush_sync(sent.append)
364364
assert sent == ["b", "c"]
365365
assert q._bytes == 0

0 commit comments

Comments
 (0)