fix(exceptions): coerce APIStatusError.code to str to match Optional[str] annotation - #3784
fix(exceptions): coerce APIStatusError.code to str to match Optional[str] annotation#3784dajiaohuang wants to merge 2 commits into
Conversation
…nnotation Replace construct_type call with explicit str coercion to properly handle numeric code values returned by the API. Fixes openai#3781
Validate expires_in to reject negative values, zero, NaN, infinity, and overflow values (e.g. 10**400). Convert bool to int first since bool is a subclass of int. Fixes openai#3735
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 4eee6b354e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(expires_in, bool): | ||
| expires_in = int(expires_in) | ||
| if not isinstance(expires_in, (int, float)) or math.isnan(expires_in) or math.isinf(expires_in) or expires_in <= 0: | ||
| raise ValueError("Token exchange response did not include a valid expires_in") |
There was a problem hiding this comment.
When the token endpoint returns a newly rejected value such as 0, -1, NaN, or infinity, this now raises ValueError; both sync and async request loops only propagate OpenAIError directly, so they instead retry the token exchange and eventually wrap the validation failure as APIConnectionError. Raise OpenAIError here as the previous validation did, and cover both client paths.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
| raise OpenAIError("Token exchange response did not include a valid expires_in") | ||
| if isinstance(expires_in, bool): | ||
| expires_in = int(expires_in) | ||
| if not isinstance(expires_in, (int, float)) or math.isnan(expires_in) or math.isinf(expires_in) or expires_in <= 0: |
There was a problem hiding this comment.
If a token response contains an oversized integer such as 10**400, math.isnan(expires_in) raises OverflowError while converting it to a float, so the intended finite-value rejection is never reached and the public request path retries and misreports it as a connection failure. Convert inside an OverflowError guard, as _as_finite_float already does for X.509 authentication, and test both sync and async flows.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
| if isinstance(expires_in, bool): | ||
| expires_in = int(expires_in) |
There was a problem hiding this comment.
For a malformed JSON response containing "expires_in": true, these lines turn the boolean into 1.0, cache the token, and proceed with the API request even though booleans are not numeric expiration values; the X.509 validator explicitly rejects the same input. Return a validation error for both booleans and add focused sync/async coverage.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
1 participant
Summary
APIStatusError.code is typed Optional[str] but could silently hold an int at runtime when the API returns a numeric error code.
Root cause:
construct_type returns the raw value unchanged when the runtime type doesn't match the target type. When the API sends {"code": 404}, exc.code becomes an int.
Fix:
Replace construct_type call with explicit str() coercion:
This guarantees the annotation is always honoured regardless of what the API sends.
Issue: fixes #3531