fix: generalize rate-limit retry to all model providers#3974
fix: generalize rate-limit retry to all model providers#39744444J99 wants to merge 2 commits intocamel-ai:masterfrom
Conversation
Replace the OpenAI-specific `RateLimitError` catch in `ChatAgent._get_model_response` and `_aget_model_response` with a provider-agnostic `is_rate_limit_error()` utility that detects 429 errors from OpenAI, Anthropic, Google GenAI, Mistral, and any SDK exposing a `status_code` attribute or a `RateLimitError` class name. Closes camel-ai#3882
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@claude review |
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, comment @claude review on this pull request to trigger a review.
| status = getattr(error, "code", None) or getattr( | ||
| error, "status_code", None | ||
| ) | ||
| if status == 429: |
There was a problem hiding this comment.
i think 429 doesn't always mean rate limit, the check here is not robust
There was a problem hiding this comment.
Agreed — status-code-only checks for Google GenAI, Mistral, and the generic fallback now require corroborating evidence (Retry-After header, X-RateLimit-Remaining: 0, or rate-limit keywords in the error message) before returning True; typed SDK exceptions remain as-is since their semantics are guaranteed.
A bare HTTP 429 status code is not always a rate limit — some providers reuse it for quota exhaustion, concurrent-request caps, or other throttling conditions. The status-code-based fallback paths (Google GenAI, Mistral, generic) now require at least one corroborating signal: a Retry-After header, X-RateLimit-Remaining at zero, or rate-limit keywords in the error message. Typed SDK exceptions (openai.RateLimitError, anthropic.RateLimitError) remain unchanged — their semantics are guaranteed by the SDK. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Description
Resolves #3882.
ChatAgent._get_model_responseand_aget_model_responsecatch onlyopenai.RateLimitError, so rate-limit responses from Anthropic, Google GenAI, Mistral, and other providers bypass the retry logic entirely — the error falls through to the genericexcept Exceptionblock and raises immediately instead of backing off and retrying.Changes
camel/utils/commons.pyAdded
is_rate_limit_error(error)— a provider-agnostic utility that detects HTTP 429 rate-limit errors from any supported model backend:openai.RateLimitErroranthropic.RateLimitErrorgoogle.genai.errors.ClientErrorwithcode == 429mistralai.models.sdkerror.SDKErrorwithstatus_code == 429RateLimitError(forward-compatible with new SDKs)status_code == 429Each provider check uses a lazy
try/except ImportErrorso only installed SDKs are inspected, with zero import-time overhead when a provider is not in use.camel/agents/chat_agent.pyfrom openai import RateLimitErrorimportExceptionand dispatch viais_rate_limit_error(e)— rate-limit errors trigger exponential backoff with jitter; all other errors re-raise immediately (preserving existing behavior)camel/utils/__init__.pyis_rate_limit_errorin the public APItest/utils/test_commons.pyAdded 6 unit tests covering:
RateLimitErrordetectionstatus_code=429attribute (positive)status_code=500attribute (negative)RuntimeError(negative)Testing
All new tests pass locally. No existing tests were modified (only a new import added).