fix: handle empty OPENAI_BASE_URL and top-level Responses API error fields#3052
fix: handle empty OPENAI_BASE_URL and top-level Responses API error fields#3052maryamfaizan53 wants to merge 3 commits intoopenai:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d31330621a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| else: | ||
| data = sse.json() | ||
| if is_mapping(data) and data.get("error"): | ||
| if is_mapping(data) and (data.get("error") or data.get("message")): |
There was a problem hiding this comment.
error exists for top-level message events
This branch now triggers when a stream payload has only a top-level message, but the exception construction still reads data["error"]; for spec-compliant error events without an error object, this raises KeyError and bypasses the intended APIError path. In practice, streaming failures can still crash with an internal key lookup error instead of returning a usable API exception (the same pattern is duplicated in the async path).
Useful? React with 👍 / 👎.
1 participant
Fixes #2927 and #2487.
Fix 1 — Empty OPENAI_BASE_URL breaks the client (
_client.py)If
OPENAI_BASE_URL=""is set,os.environ.get("OPENAI_BASE_URL")returns""notNone, so the fallback to the default endpoint never triggers. Changed toos.environ.get("OPENAI_BASE_URL") or Noneso empty strings are treated as unset.Fix 2 — Streaming error events silently ignored (
_streaming.py)Per the Responses API spec, streaming error events send
messageat the top level — not inside a nested"error"key. The old conditiondata.get("error")never matched spec-compliant errors. Updated to(data.get("error") or data.get("message"))so both formats are handled correctly.