Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add regression tests for top-level, nested, and missing-message streaming error events.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes Responses API streaming errors by surfacing top-level error messages while preserving nested error compatibility.
Changes:
- Handles top-level and nested streaming error payloads.
- Raises
APIErrorwith appropriate fallback messages. - Applies behavior to synchronous and asynchronous streams.
| File | Summary |
|---|---|
src/openai/_streaming.py |
Updates sync/async SSE error handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if sse.event == "error" and is_mapping(data): | ||
| message = data.get("message") | ||
| error = data.get("error") | ||
| if is_mapping(error): | ||
| message = error.get("message") | ||
| if not message or not isinstance(message, str): | ||
| if is_mapping(error): | ||
| message = error.get("message") | ||
| if not message or not isinstance(message, str): | ||
| message = "An error occurred during streaming" | ||
|
|
||
| raise APIError( | ||
| message=message, | ||
| request=self.response.request, | ||
| body=data["error"], | ||
| body=error if is_mapping(error) else data, |
There was a problem hiding this comment.
Fixed in f50cb1c: tests/test_streaming.py adds sync+async regression cases for all three error-event shapes — top-level message (Responses-style), nested error.message fallback, and missing-message fallback (generic An error occurred during streaming), parametrized over both openai and azure clients.
The Responses API delivers error events with the message at the top level
(e.g. {"type": "error", "message": "..."}), but the streaming code only
looked inside a nested error object, so such events were silently dropped or
reported the default message. Read the top-level message first, fall back to
the nested error.message for backward compatibility, and raise on error
events regardless of shape.
Add sync and async regression tests asserting that an SSE error event raises APIError with the correct message for the Responses-style top-level message shape, the nested error-object shape, and a message-less fallback, so the silent-swallow behavior cannot regress.
f50cb1c to
a49d79b
Compare
|
AI-assisted independent offline check at Eight error/data envelope controls across Stream and AsyncStream: 10/16 on pinned main, 16/16 on this head. The added controls include absent/non-string top-level messages, top-level versus nested message priority, nested-error backward compatibility, and ordinary event emission. Error bodies are compared as well as messages. Scope: Actual stream classes and SSE decoder over in-memory HTTP responses; model-data processing is stubbed. No endpoint request, network or provider compatibility claim. Python 3.14.7, Pydantic 2.13.5 and shared local dependencies; this is a bounded regression matrix, not the full SDK suite or a historical lockfile matrix. Network was denied by the test sandbox. Standalone reproducer (run separately with each source tree on PYTHONPATH)import asyncio,importlib,json
from types import SimpleNamespace
source=importlib.import_module('openai._streaming');http=importlib.import_module('httpx2')
fallback='An error occurred during streaming'
nested={'message':'fictional nested'}
cases=[('named_top','error',{'type':'error','message':'fictional top'},'fictional top',None),('named_nested','error',{'error':nested},'fictional nested',nested),('named_empty','error',{},fallback,None),('named_bad_top_nested','error',{'message':7,'error':nested},'fictional nested',nested),('named_both','error',{'message':'fictional top','error':nested},'fictional top',nested),('unnamed_nested',None,{'error':nested},'fictional nested',nested),('ordinary',None,{'fictional':'data'},None,None),('named_ordinary','response.output_text.delta',{'type':'response.output_text.delta','delta':'fictional'},None,None)]
async def main():
rows=[]
for is_async in [False,True]:
for name,event,data,expected,body in cases:
payload=((f'event: {event}\n' if event else '')+'data: '+json.dumps(data)+'\n\n').encode()
response=http.Response(200,request=http.Request('GET','https://example.invalid/fictional'),content=payload)
client=SimpleNamespace(_make_sse_decoder=source.SSEDecoder,_process_response_data=lambda **kwargs:kwargs['data'])
stream=(source.AsyncStream if is_async else source.Stream)(cast_to=object,response=response,client=client)
actual=None;error_body=None;emitted=[];kind=None
try:
if is_async:emitted=[x async for x in stream]
else:emitted=list(stream)
except source.APIError as exc:kind=type(exc).__name__;actual=exc.message;error_body=exc.body
except Exception as exc:kind=type(exc).__name__
expected_body=body if body is not None else data
ok=(kind=='APIError' and actual==expected and error_body==expected_body) if expected else kind is None and len(emitted)==1
rows.append({'case':name,'async':is_async,'error_type':kind,'message':actual,'expected_message':expected,'body_matches':error_body==expected_body if expected else None,'emitted':len(emitted),'pass':ok})
print(json.dumps({'source_module':source.__file__,'cases':rows,'passed':sum(r['pass'] for r in rows),'failed':sum(not r['pass'] for r in rows),'scope':'Actual Stream/AsyncStream and SSE decoder over an in-memory HTTP response; stubbed model-data processor, no client endpoint/network/provider proof.'}))
asyncio.run(main()) |

Fixes #2487
Problem
The SDK's own generated type
ResponseErrorEvent(from the OpenAPI spec) carriesmessageat the top level and has no nestederrorobject:But
_streaming.pyonly raises an error when the payload has a nestederrorkey and readsdata["error"]["message"]. For a real Responses API error event ({"type": "error", "code": ..., "message": ..., ...}), the guard fails and the error is silently swallowed instead of being surfaced asAPIError.Fix
In
_streaming.py, when the SSE event type is"error":messagefirst, falling back to the nestederror.messageshape used by chat-completions-style payloadsAPIErrorwith the correct message instead of yielding the event as dataVerification
{"error": {"message": ...}}payload → still raises (backward compatible)python -m py_compilepasses on the changed module