fix: classify 429 quota-exhaustion errors as non-retryable for fallback - #65633
fix: classify 429 quota-exhaustion errors as non-retryable for fallback#65633AlexFucuson9 wants to merge 1 commit into
Conversation
When Ollama Cloud returns HTTP 429 with "session usage limit" /
"upgrade for higher limits", the error was classified as a transient
rate_limit (retryable=True). This caused the system to retry the same
provider 3 times then give up — never triggering fallback_providers.
Fix: check _USAGE_LIMIT_PATTERNS in the 429 handler (same logic already
present in the 402 and no-status-code paths). Transient signals
("try again", "resets at") → rate_limit; otherwise → billing
(non-retryable, should_fallback=True).
Fixes NousResearch#65563
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Scope
- Single file (agent/error_classifier.py), +28 lines
- Fixes a misclassification bug: quota-exhaustion 429s were being treated as retryable rate limits instead of non-retryable billing failures.
Quality
- Root cause well-identified: explains the failure mode precisely
- Disambiguation logic is sound: transient signals route to retryable, billing-like messages route to non-retryable + should_fallback
- Pattern-matching approach is appropriate for this error class
- No side effects on other code paths
Looks Good
- Clean, focused fix with detailed explanatory comment
- Correct retry/fallback semantics
- PR number referenced in comment
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the missing 429 usage-limit classification; current main still defaults that shape to rate_limit at agent/error_classifier.py:1009-1044.
Problems
agent/error_classifier.py:1000matches_USAGE_LIMIT_PATTERNSbefore checking ordinary rate-limit wording. That list includes"limit exceeded"(agent/error_classifier.py:190-195), so the existing"Rate limit exceeded: too many requests"case (tests/agent/test_error_classifier.py:459-467) would become non-retryablebilling.- The new branch also precedes
_is_openrouter_upstream_error(PR-headagent/error_classifier.py:1018-1033). Existing coverage requires an OpenRouter upstream 429 with metadata.raw"Rate limit exceeded"to remainupstream_rate_limit(tests/agent/test_error_classifier.py:1948-1968); this patch captures it first. - No tests accompany the behavior change. Also, the linked #65563 report was closed after its quoted YAML string was found not to load as a fallback chain (
hermes_cli/fallback_config.py:34-40).
Suggested changes
- Exclude
_RATE_LIMIT_PATTERNSbefore applying permanent usage-limit matching, keep the OpenRouter discriminator ahead of it, and add regression tests for Ollama, normal 429s, and OpenRouter upstream 429s.
Automated hermes-sweeper review.
| # Disambiguate: transient signals ("try again", "resets at") mean | ||
| # it's a periodic quota → rate_limit; otherwise it's billing-like | ||
| # exhaustion → non-retryable + should_fallback. (#65563) | ||
| has_usage_limit = any(p in error_msg for p in _USAGE_LIMIT_PATTERNS) |
There was a problem hiding this comment.
_USAGE_LIMIT_PATTERNS includes the broad phrase "limit exceeded", so this classifies the existing normal 429 text "Rate limit exceeded: too many requests" as non-retryable billing. Exclude _RATE_LIMIT_PATTERNS first, and keep the OpenRouter upstream discriminator ahead of permanent-quota matching; tests/agent/test_error_classifier.py:459-467 and :1948-1968 cover both regressions.
Summary49 PRs reference this fallback/failover complex: their diffs cover ordered chains, custom-endpoint propagation, credential-exhaustion notices, fallback-state recovery, the rate-limit NameError, runtime identity, gateway notifications, transport-recovery state, documentation, and #65633's narrower 429 usage-limit classification. The reported root causes are therefore distributed across distinct call paths rather than represented by one interchangeable patch family. Related pull requests
Duplicates#1761 was salvaged by #3813; #17827 by #18185; #20793 by #27185; #33846 by #54054; and #28159 by #28345. #27359, #27374, #27433, #27468, #27532, #27534, #27608, #27686, #27732, #27734, #27750, #27896, #27903, #27945, #28189, #28254, #28268, #28297, #28304, and #29210 substantially duplicate #28345's NameError correction; #2587, #2705, #3182, #3853, #4784, and #4895 overlap on custom fallback override forwarding. Suggested consolidationKeep #65633 open with a salvage path: narrow its match so ordinary _RATE_LIMIT_PATTERNS and OpenRouter upstream 429 classification retain precedence, then add regression tests for the Ollama phrase, ordinary “Rate limit exceeded,” transient reset signals, and metadata.raw OpenRouter errors; this explicitly follows the contributor keep_open review rather than treating #65563's malformed configuration as proof that classifier work is unnecessary. Also keep #24113 open and port its verified forwarding logic to hermes_cli/cli_agent_setup_mixin.py:53-71 as requested by its keep_open review; all listed duplicate implementation PRs are already closed or merged, so no additional open PR can be closed as a duplicate from this set. Cross-PR triage: Reviewed 49 pull requests and 27 issues in this complex. Each diff was read against this issue; Assessment working set: 264 kB of PR diffs, 171 kB of issue/PR text, 60 kB of discussion (117 comments), 89 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
When Ollama Cloud returns HTTP 429 with body text like
"you (user) have reached your session usage limit, upgrade for higher limits", Hermes misclassifies it as a transient rate limit instead of quota exhaustion. The error hits the default 429 path (rate_limit,retryable=True), which retries the same provider 3 times then gives up — never triggering thefallback_providerschain.Root Cause
The 429 handler in
classify_api_error()checks for:_OVERLOADED_PATTERNS)But it does NOT check for quota/usage-limit exhaustion patterns (
_USAGE_LIMIT_PATTERNS). This logic already exists in:_classify_402, line 1094)The 429 path was missing this check.
Fix
Add
_USAGE_LIMIT_PATTERNScheck in the 429 handler, between the overloaded check and the OpenRouter check. Same disambiguation logic:rate_limit(retryable)billing(non-retryable,should_fallback=True)Changes
agent/error_classifier.py: Add 28 lines — quota-exhaustion detection in 429 handlerTest Plan
Fixes #65563