Skip to content

Commit ef68e5b

Browse files
committed
Retry httpx.RemoteProtocolError raised mid-stream, not just at connect time
parse_sse_lines wrapped every next(it) failure in the non-retryable ChatError, including httpx.RemoteProtocolError -- so _is_retryable_exc's RemoteProtocolError branch only ever fired for a disconnect at connect time, never for the mid-stream case CodeRabbit flagged and the one actually seen in production. Raise TransientChatError instead when the wrapped exception is a RemoteProtocolError.
1 parent 09ebe48 commit ef68e5b

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,14 @@ def parse_sse_lines(lines: Iterable[str]) -> ChatResult:
229229
# Only a transport-level failure (e.g. connection drop mid-stream) is rescued
230230
# here -- a bug in the processing below must propagate uncaught, not get
231231
# mislabeled as a network error.
232-
raise ChatError(f"SSE stream error: {exc}", partial_result=_build_chat_result(acc)) from exc
232+
partial = _build_chat_result(acc)
233+
if isinstance(exc, httpx.RemoteProtocolError):
234+
# Same mid-stream disconnect _is_retryable_exc already retries when it happens
235+
# at connect time -- here it surfaces from `next(it)` instead, so it must be
236+
# raised as TransientChatError or the wrapping below would mask it as
237+
# non-retryable and defeat the retry this class exists for.
238+
raise TransientChatError(f"SSE stream error: {exc}", partial_result=partial) from exc
239+
raise ChatError(f"SSE stream error: {exc}", partial_result=partial) from exc
233240
line = raw_line.decode("utf-8") if isinstance(raw_line, bytes) else raw_line
234241
if not line:
235242
current_event = "message" # blank line ends one event block per the SSE spec

packages/gooddata-eval/tests/test_sse_client.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,28 @@ def _lines():
8585
assert partial.tool_call_events[0].function_name == "create_key_driver_analysis"
8686

8787

88+
def test_parse_sse_lines_remote_protocol_error_mid_stream_is_retryable():
89+
# httpx.RemoteProtocolError raised from `next(it)` (mid-stream, not at connect time) must
90+
# come out as TransientChatError -- otherwise _is_retryable_exc never sees the raw
91+
# RemoteProtocolError (only the ChatError parse_sse_lines wraps it in) and the retry this
92+
# class exists for never fires. Same partial_result guarantee as any other transport error.
93+
def _lines():
94+
yield (
95+
'data: {"item": {"role": "assistant", "content": '
96+
+ json.dumps({"type": "toolCall", "callId": "c1", "name": "create_key_driver_analysis"})
97+
+ "}}"
98+
)
99+
yield ""
100+
raise httpx.RemoteProtocolError("peer closed connection without sending complete message body")
101+
102+
with pytest.raises(TransientChatError) as ei:
103+
parse_sse_lines(_lines())
104+
partial = ei.value.partial_result
105+
assert partial is not None
106+
assert len(partial.tool_call_events) == 1
107+
assert partial.tool_call_events[0].function_name == "create_key_driver_analysis"
108+
109+
88110
def test_parse_sse_lines_a_real_parsing_bug_propagates_uncaught_not_as_a_chat_error():
89111
# A malformed payload (here: "item" is a string, not a dict) crashes the processing
90112
# code itself with a plain AttributeError -- must surface loudly as that bug, not get

0 commit comments

Comments
 (0)