Skip to content

Commit 1dbbf61

Browse files
fix(responses): avoid replaying uncertain typed sends on reconnect (#4011)
A typed Responses WebSocket command could reach the server and then raise locally. Reconnecting replayed that command, potentially creating a duplicate response. The same problem occurred if sending a queued command failed or was cancelled midway through a flush. Queue only commands that have not been attempted. On a failed Responses flush, discard the attempted item and retain the unsent tail in order with its original byte limits. Keep typed sends on their existing transport path so applications that customize `send_raw()` do not unexpectedly intercept typed sends. This covers stable/beta and sync/async; Realtime retains its existing queue policy. Validation on this change: - 294 affected Responses session, auth, preconnect, URL, queue and Realtime recovery tests pass with both Pydantic v1 and v2. - Extended the real WebSocket no-replay regression with a customized raw-send policy: all four typed variants fail before the compatibility fix and pass afterward; all four raw controls pass. - Ruff, formatting and `git diff --check` pass. - Local tests used the committed dependencies (`uv --frozen`): the local uv is newer than the uv 0.12.5 pinned in CI. Hosted checks must still pass on the published commit.
1 parent 4fc2438 commit 1dbbf61

4 files changed

Lines changed: 119 additions & 42 deletions

File tree

‎src/openai/resources/beta/responses/responses.py‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4342,11 +4342,7 @@ async def send(self, event: BetaResponsesClientEvent | BetaResponsesClientEventP
43424342
if self._is_reconnecting:
43434343
self._send_queue.enqueue(data)
43444344
return
4345-
try:
4346-
await self._connection.send(data)
4347-
except Exception:
4348-
self._send_queue.enqueue(data)
4349-
raise
4345+
await self._connection.send(data)
43504346

43514347
async def send_raw(self, data: bytes | str) -> None:
43524348
if self._is_reconnecting:
@@ -4454,7 +4450,7 @@ async def _send(data: str) -> None:
44544450
await self._connection.send(data)
44554451

44564452
try:
4457-
await self._send_queue.flush_async(_send)
4453+
await self._send_queue.flush_async(_send, requeue_failed=False)
44584454
except Exception:
44594455
log.warning("Failed to flush send queue after reconnect")
44604456

@@ -4841,11 +4837,7 @@ def send(self, event: BetaResponsesClientEvent | BetaResponsesClientEventParam)
48414837
if self._is_reconnecting:
48424838
self._send_queue.enqueue(data)
48434839
return
4844-
try:
4845-
self._connection.send(data)
4846-
except Exception:
4847-
self._send_queue.enqueue(data)
4848-
raise
4840+
self._connection.send(data)
48494841

48504842
def send_raw(self, data: bytes | str) -> None:
48514843
if self._is_reconnecting:
@@ -4947,7 +4939,7 @@ def _reconnect(self, exc: Exception) -> bool:
49474939
def _flush_send_queue(self) -> None:
49484940
"""Send all queued messages over the current connection."""
49494941
try:
4950-
self._send_queue.flush_sync(lambda data: self._connection.send(data))
4942+
self._send_queue.flush_sync(lambda data: self._connection.send(data), requeue_failed=False)
49514943
except Exception:
49524944
log.warning("Failed to flush send queue after reconnect")
49534945

‎src/openai/resources/responses/responses.py‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4193,11 +4193,7 @@ async def send(self, event: ResponsesClientEvent | ResponsesClientEventParam) ->
41934193
if self._is_reconnecting:
41944194
self._send_queue.enqueue(data)
41954195
return
4196-
try:
4197-
await self._connection.send(data)
4198-
except Exception:
4199-
self._send_queue.enqueue(data)
4200-
raise
4196+
await self._connection.send(data)
42014197

42024198
async def send_raw(self, data: bytes | str) -> None:
42034199
if self._is_reconnecting:
@@ -4305,7 +4301,7 @@ async def _send(data: str) -> None:
43054301
await self._connection.send(data)
43064302

43074303
try:
4308-
await self._send_queue.flush_async(_send)
4304+
await self._send_queue.flush_async(_send, requeue_failed=False)
43094305
except Exception:
43104306
log.warning("Failed to flush send queue after reconnect")
43114307

@@ -4694,11 +4690,7 @@ def send(self, event: ResponsesClientEvent | ResponsesClientEventParam) -> None:
46944690
if self._is_reconnecting:
46954691
self._send_queue.enqueue(data)
46964692
return
4697-
try:
4698-
self._connection.send(data)
4699-
except Exception:
4700-
self._send_queue.enqueue(data)
4701-
raise
4693+
self._connection.send(data)
47024694

47034695
def send_raw(self, data: bytes | str) -> None:
47044696
if self._is_reconnecting:
@@ -4810,7 +4802,7 @@ def _reconnect(self, exc: Exception) -> bool:
48104802
def _flush_send_queue(self) -> None:
48114803
"""Send all queued messages over the current connection."""
48124804
try:
4813-
self._send_queue.flush_sync(lambda data: self._connection.send(data))
4805+
self._send_queue.flush_sync(lambda data: self._connection.send(data), requeue_failed=False)
48144806
except Exception:
48154807
log.warning("Failed to flush send queue after reconnect")
48164808

‎tests/lib/responses/test_websocket_session.py‎

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,93 @@ async def uncertain_async_send(data: Any) -> None:
799799
await driver.call(lane, "send", {"type": "response.create"})
800800

801801

802+
@pytest.mark.parametrize("mode", ["sync", "async"])
803+
@pytest.mark.parametrize("beta", [False, True], ids=["stable", "beta"])
804+
@pytest.mark.parametrize("raw", [False, True], ids=["typed", "raw"])
805+
async def test_connection_does_not_replay_an_uncertain_create(
806+
mode: str, beta: bool, raw: bool, monkeypatch: pytest.MonkeyPatch
807+
) -> None:
808+
requests: list[dict[str, Any]] = []
809+
810+
def reject_raw(_data: bytes | str) -> None:
811+
raise ValueError("Synthetic application policy: raw sends are disabled")
812+
813+
def reconnect(_event: ReconnectingEvent) -> ReconnectingOverrides:
814+
return {"extra_headers": {"X-Recovery": "fresh"}}
815+
816+
def script(socket: ServerConnection) -> None:
817+
assert socket.request is not None
818+
if socket.request.headers.get("X-Recovery") == "fresh":
819+
socket.send(json.dumps({"type": "response.future", "restored": True}))
820+
while True:
821+
received = json.loads(socket.recv(timeout=5))
822+
requests.append(received)
823+
if received == fresh_request:
824+
break
825+
else:
826+
requests.append(json.loads(socket.recv(timeout=5)))
827+
socket.close(code=1011)
828+
829+
request = {"type": "response.create", "input": "synthetic uncertain request"}
830+
fresh_request = {"type": "response.create", "input": "synthetic fresh work"}
831+
with script_server(script, expected_connections=2) as url:
832+
if mode == "sync":
833+
with OpenAI(api_key="fake-key", base_url=url, http_client=httpx2.Client(trust_env=False)) as client:
834+
resource = client.beta.responses if beta else client.responses
835+
with resource.connect(on_reconnecting=reconnect, initial_delay=0, max_retries=1) as connection:
836+
if not raw:
837+
monkeypatch.setattr(connection, "send_raw", reject_raw)
838+
original_send = connection._connection.send
839+
840+
def uncertain_send(data: Any) -> None:
841+
original_send(data)
842+
raise OSError("Synthetic failure after write")
843+
844+
monkeypatch.setattr(connection._connection, "send", uncertain_send)
845+
with pytest.raises(OSError, match="Synthetic failure after write"):
846+
if raw:
847+
connection.send_raw(json.dumps(request))
848+
else:
849+
connection.send({"type": "response.create", "input": "synthetic uncertain request"})
850+
assert next(iter(connection)).to_dict()["restored"] is True
851+
if raw:
852+
connection.send_raw(json.dumps(fresh_request))
853+
else:
854+
connection.send({"type": "response.create", "input": "synthetic fresh work"})
855+
else:
856+
async with AsyncOpenAI(
857+
api_key="fake-key", base_url=url, http_client=httpx2.AsyncClient(trust_env=False)
858+
) as async_client:
859+
async_resource = async_client.beta.responses if beta else async_client.responses
860+
async with async_resource.connect(
861+
on_reconnecting=reconnect, initial_delay=0, max_retries=1
862+
) as async_connection:
863+
if not raw:
864+
monkeypatch.setattr(async_connection, "send_raw", reject_raw)
865+
original_async_send = async_connection._connection.send
866+
867+
async def uncertain_async_send(data: Any) -> None:
868+
await original_async_send(data)
869+
raise OSError("Synthetic failure after write")
870+
871+
monkeypatch.setattr(async_connection._connection, "send", uncertain_async_send)
872+
with pytest.raises(OSError, match="Synthetic failure after write"):
873+
if raw:
874+
await async_connection.send_raw(json.dumps(request))
875+
else:
876+
await async_connection.send(
877+
{"type": "response.create", "input": "synthetic uncertain request"}
878+
)
879+
event = await asyncio.wait_for(anext(aiter(async_connection)), timeout=5)
880+
assert event.to_dict()["restored"] is True
881+
if raw:
882+
await async_connection.send_raw(json.dumps(fresh_request))
883+
else:
884+
await async_connection.send({"type": "response.create", "input": "synthetic fresh work"})
885+
886+
assert requests == [request, fresh_request]
887+
888+
802889
@pytest.mark.parametrize("mode", ["sync", "async"])
803890
async def test_early_clean_close_is_not_a_completed_response(mode: str) -> None:
804891
def script(socket: ServerConnection) -> None:
@@ -1353,8 +1440,9 @@ def check_then_reconnect(state: Any) -> None:
13531440
assert recovered.wait(5)
13541441
with session._router.condition:
13551442
assert session._router.condition.wait_for(
1356-
lambda: session.connection._connection is not previous
1357-
and not session.connection._is_reconnecting,
1443+
lambda: (
1444+
session.connection._connection is not previous and not session.connection._is_reconnecting
1445+
),
13581446
timeout=5,
13591447
)
13601448
if recover_before_check:

‎tests/test_send_queue_reconnect.py‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,14 @@ def test_reconnect_retries_bounded_send_queue(
108108
q.enqueue("aaa")
109109
ws = MagicMock()
110110
attempts = 0
111-
realtime = connection_type is RealtimeConnection
112111

113112
def failing_send(data: str) -> None:
114113
nonlocal attempts
115-
assert data == ("b" if realtime and attempts == 1 else "aaa")
114+
assert data == ("b" if attempts == 1 else "aaa")
116115
if attempts == 0:
117116
q.enqueue("b")
118117
attempts += 1
119-
if not realtime or attempts == 1:
118+
if attempts == 1:
120119
with pytest.raises(WebSocketQueueFullError):
121120
q.enqueue("c")
122121
raise RuntimeError("fake send failure")
@@ -132,10 +131,10 @@ def failing_send(data: str) -> None:
132131
)
133132
for expected_attempts in range(1, 4):
134133
assert connection._reconnect(RuntimeError("fake disconnect"))
135-
assert q._bytes == ((1 if expected_attempts == 1 else 0) if realtime else 4)
136-
assert attempts == (min(expected_attempts, 2) if realtime else expected_attempts)
134+
assert q._bytes == (1 if expected_attempts == 1 else 0)
135+
assert attempts == min(expected_attempts, 2)
137136
assert not connection._reconnect(RuntimeError("fake disconnect"))
138-
assert attempts == (2 if realtime else 3)
137+
assert attempts == 2
139138

140139
# A healthy application event resets the budget; an upgrade alone does not.
141140
ws.recv.return_value = '{"type": "response.created"}'
@@ -144,7 +143,7 @@ def failing_send(data: str) -> None:
144143
sent: list[str] = []
145144
ws.send.side_effect = sent.append
146145
assert connection._reconnect(RuntimeError("fake disconnect"))
147-
assert sent == ([] if realtime else ["aaa", "b"])
146+
assert sent == []
148147
assert q._bytes == 0
149148
for _ in range(2):
150149
assert connection._reconnect(RuntimeError("fake disconnect"))
@@ -164,15 +163,14 @@ async def test_async_reconnect_retries_bounded_send_queue(
164163
q.enqueue("aaa")
165164
ws = MagicMock()
166165
attempts = 0
167-
realtime = connection_type is AsyncRealtimeConnection
168166

169167
async def failing_send(data: str) -> None:
170168
nonlocal attempts
171-
assert data == ("b" if realtime and attempts == 1 else "aaa")
169+
assert data == ("b" if attempts == 1 else "aaa")
172170
if attempts == 0:
173171
q.enqueue("b")
174172
attempts += 1
175-
if not realtime or attempts == 1:
173+
if attempts == 1:
176174
with pytest.raises(WebSocketQueueFullError):
177175
q.enqueue("c")
178176
raise RuntimeError("fake send failure")
@@ -188,10 +186,10 @@ async def failing_send(data: str) -> None:
188186
)
189187
for expected_attempts in range(1, 4):
190188
assert await connection._reconnect(RuntimeError("fake disconnect"))
191-
assert q._bytes == ((1 if expected_attempts == 1 else 0) if realtime else 4)
192-
assert attempts == (min(expected_attempts, 2) if realtime else expected_attempts)
189+
assert q._bytes == (1 if expected_attempts == 1 else 0)
190+
assert attempts == min(expected_attempts, 2)
193191
assert not await connection._reconnect(RuntimeError("fake disconnect"))
194-
assert attempts == (2 if realtime else 3)
192+
assert attempts == 2
195193

196194
# A healthy application event resets the budget; an upgrade alone does not.
197195
ws.recv = AsyncMock(return_value='{"type": "response.created"}')
@@ -200,15 +198,22 @@ async def failing_send(data: str) -> None:
200198
sent: list[str] = []
201199
ws.send.side_effect = sent.append
202200
assert await connection._reconnect(RuntimeError("fake disconnect"))
203-
assert sent == ([] if realtime else ["aaa", "b"])
201+
assert sent == []
204202
assert q._bytes == 0
205203
for _ in range(2):
206204
assert await connection._reconnect(RuntimeError("fake disconnect"))
207205
assert not await connection._reconnect(RuntimeError("retry budget exhausted"))
208206

209207

208+
@pytest.mark.parametrize(
209+
"connection_type", [AsyncRealtimeConnection, AsyncResponsesConnection, AsyncBetaResponsesConnection]
210+
)
210211
@pytest.mark.asyncio
211-
async def test_cancelled_realtime_flush_does_not_replay_active_send() -> None:
212+
async def test_cancelled_flush_does_not_replay_active_send(
213+
connection_type: type[AsyncRealtimeConnection]
214+
| type[AsyncResponsesConnection]
215+
| type[AsyncBetaResponsesConnection],
216+
) -> None:
212217
q = SendQueue(max_bytes=4)
213218
q.enqueue("é")
214219
q.enqueue("b")
@@ -221,7 +226,7 @@ async def suspended_send(data: str) -> None:
221226
await asyncio.Event().wait()
222227

223228
socket.send = AsyncMock(side_effect=suspended_send)
224-
connection = AsyncRealtimeConnection(socket, send_queue=q)
229+
connection = connection_type(socket, send_queue=q)
225230
task = asyncio.create_task(connection._flush_send_queue())
226231
try:
227232
await asyncio.wait_for(active.wait(), 5)

0 commit comments

Comments
 (0)