fix(client): treat empty OPENAI_BASE_URL env var as unset - #3559
Conversation
Filters out content items where `text` is None before joining, preventing a TypeError crash when the API returns null for a text block (issue openai#3063).
result (base64 image string) and status are output-only fields populated by the API response. Marking them Required on an input TypedDict forces callers to provide values they don't have and wouldn't supply when referencing an existing image generation call by id and type. Fixes openai#2648
os.environ.get("OPENAI_BASE_URL") returns "" when the variable is
defined but empty (common in Docker/.env setups). The empty string
passes the subsequent `if base_url is None` check, so the default
https://api.openai.com/v1 fallback is never applied and the client
tries to connect to an empty URL, raising an APIConnectionError.
Using `or None` normalises "" to None, allowing the default to apply.
Fix applied to both the sync OpenAI and async AsyncOpenAI clients.
Fixes openai#2927
![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: d2140bedea
ℹ️ 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".
| result: Optional[str] | ||
| """The generated image encoded in base64.""" | ||
|
|
||
| status: Required[Literal["in_progress", "completed", "generating", "failed"]] | ||
| status: Literal["in_progress", "completed", "generating", "failed"] |
There was a problem hiding this comment.
When callers use the normal client.responses.create(input=[...]) path, the overloads still refer to ResponseInputParam from response_input_param.py, which contains its own duplicate ImageGenerationCall where result and status remain Required. This change fixes only the standalone ResponseInputItemParam, so the documented image-id input still produces type errors for the primary Responses create API; update the duplicate definition or make ResponseInputParam reuse this item type.
Useful? React with 👍 / 👎.
| if output.type == "message": | ||
| for content in output.content: | ||
| if content.type == "output_text": | ||
| if content.type == "output_text" and content.text is not None: |
There was a problem hiding this comment.
In the repository's strict Pyright configuration, ResponseOutputText.text is still annotated as str, so this new content.text is not None check is reported as an always-true comparison and causes scripts/run-pyright to fail. For the null-text scenario this fix is targeting, the model field also needs to be made nullable (or the guard needs a type-safe workaround) so the change can pass CI while representing the API payload accurately.
Useful? React with 👍 / 👎.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
There are three blocking issues on this head:
-
The null-text change does not fix the real response boundary.
ResponseOutputText.textremains typed and validated asstr, so an actual API payload withtext: nullis rejected beforeResponse.output_textruns. The new test hides this by usingmodel_construct()to bypass validation, and pinned Pyright correctly flags the newis not Nonecheck as always true. Please make the generated response field nullable and cover normalmodel_validate/client parsing; also update the structured parsing path so it does not callparse_text()withNone. -
The image-generation typing fix only changes
response_input_item_param.py. The primaryresponses.create(input=...)API uses the duplicateImageGenerationCallinresponse_input_param.py, whereresultandstatusare stillRequired, so the documented image-ID input remains a type error. Please fix the canonical schema/codegen source so both generated surfaces stay consistent and add a type-level regression test. -
This PR is titled/documented as the
OPENAI_BASE_URLfix but also carries the two unrelated changes above and is currently merge-conflicting. Please split or cleanly rebase it, and add focused sync/async tests for the empty environment-variable fallback.
Validation: the base-URL fallback itself works in a local sync/async smoke test, but pinned Pyright fails on response.py, Ruff fails on the new test import ordering, real ResponseOutputText.model_validate(... text=None ...) still raises, and GitHub reports the branch as conflicting.
2 participants
os.environ.get("OPENAI_BASE_URL")returns""when the variable is defined but empty — common in Docker/.envsetups. The empty string passes theif base_url is Noneguard, so the defaulthttps://api.openai.com/v1fallback is never applied and the client raises anAPIConnectionErrortrying to connect to an empty URL.Using
or Nonenormalises""toNone. Fix applied to bothOpenAI(sync) andAsyncOpenAIclients.Fixes #2927