feat: add InsufficientQuotaError for insufficient_quota 429 responses#3507
feat: add InsufficientQuotaError for insufficient_quota 429 responses#3507pctablet505 wants to merge 2 commits into
Conversation
Adds InsufficientQuotaError as a subclass of RateLimitError and maps 429 responses with code='insufficient_quota' to it so callers can distinguish quota exhaustion from retryable rate limits. Fixes openai#1671
![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: e77b297e8f
ℹ️ 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".
| if response.status_code == 429: | ||
| if is_mapping(data) and data.get("code") == "insufficient_quota": | ||
| return _exceptions.InsufficientQuotaError(err_msg, response=response, body=data) |
There was a problem hiding this comment.
With the default max_retries, _base_client calls _should_retry() and retries every 429 before _make_status_error() is reached, so this new branch only runs after the retry budget is exhausted; the async path has the same flow. For deterministic code="insufficient_quota" responses this still sends multiple doomed requests and delays surfacing the non-retryable quota error, so the quota code needs to be classified before retrying or handled in _should_retry.
Useful? React with 👍 / 👎.
_should_retry treated every 429 as retryable, so a deterministic insufficient_quota error still burned the whole retry budget before _make_status_error got a chance to raise InsufficientQuotaError, delaying the new error type behind unnecessary retries and backoff sleeps. Check the error code in _should_retry and skip retrying when it's insufficient_quota, mirroring the extraction already done in _make_status_error.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94462847c6
ℹ️ 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".
| # quota; retrying will not help, so don't burn the retry budget | ||
| # on a request that is guaranteed to fail again. | ||
| try: | ||
| body = response.json() |
There was a problem hiding this comment.
Fresh evidence after the retry fix: for stream=True/.with_streaming_response calls, _base_client.request() sends the request with streaming enabled, so the 429 body has not been read when this line calls response.json(); httpx raises ResponseNotRead, the broad except turns the body into None, and the code falls through to retry. Streaming chat/responses requests that receive code="insufficient_quota" will therefore still burn max_retries before raising, unlike the non-streaming path covered by the new tests.
Useful? React with 👍 / 👎.
1 participant
Fixes #1671
Adds
InsufficientQuotaError, a subclass ofRateLimitError, and maps 429 responses withcode="insufficient_quota"to it so callers can distinguish quota exhaustion from retryable rate limits.