fix(router): break retry loop on non-retryable errors - #21370
Conversation
The retry loop in async_function_with_retries catches all exceptions blindly and continues retrying even for non-retryable errors like 400 ContextWindowExceeded or 404 NotFoundError. This causes the original retryable error to be raised instead of the actual non-retryable one. Changes: - Update original_exception to latest error on each retry attempt - Add should_retry_this_error() check inside the retry loop to break out immediately on non-retryable errors - Respect _retry_policy_applies precedence Fixes BerriAI#21343
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryFixes the Router retry loop in
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/router.py | Adds original_exception = e to track latest error and a should_retry_this_error() check inside the retry loop to break on non-retryable errors. Logic is correct and consistent with pre-loop behavior. |
| tests/test_litellm/test_router_retry_non_retryable_errors.py | New test file with 5 well-structured mock-only tests covering non-retryable error handling, exception tracking, and normal retry behavior. No real network calls. |
Flowchart
flowchart TD
A[Initial Call] -->|Exception| B[Set original_exception = e]
B --> C{retry_policy applies?}
C -->|Yes| D[Get num_retries from policy]
C -->|No| E[should_retry_this_error]
E -->|Raises: non-retryable| F[Raise error immediately]
E -->|Returns True: retryable| G[Enter Retry Loop]
D --> G
G --> H[make_call attempt]
H -->|Success| I[Return response]
H -->|Exception e| J["original_exception = e (NEW)"]
J --> K[Log retry & get healthy deployments]
K --> L{retry_policy applies?}
L -->|Yes| N[Sleep & continue loop]
L -->|No| M["should_retry_this_error (NEW)"]
M -->|Raises: non-retryable| O[Raise e immediately]
M -->|Returns True: retryable| N
N --> P{More retries?}
P -->|Yes| H
P -->|No| Q[Raise original_exception]
Last reviewed commit: 5992cac
| except Exception: | ||
| raise e |
There was a problem hiding this comment.
Broad except may mask bugs in should_retry_this_error
The except Exception: here catches any exception from should_retry_this_error, including unexpected ones (e.g., AttributeError, TypeError from a bug in the method itself). If should_retry_this_error ever has a bug, it would be silently swallowed and e raised instead, making debugging harder.
Consider catching only the specific exception types that should_retry_this_error raises (which are litellm exception types):
| except Exception: | |
| raise e | |
| except litellm.LITELLM_EXCEPTION_TYPES: | |
| raise e |
That said, this is a minor style concern — the current approach works correctly for all expected scenarios.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
ae613b2
into
BerriAI:litellm_oss_staging_02_18_2026
The retry loop in async_function_with_retries catches all exceptions blindly and continues retrying even for non-retryable errors like 400 ContextWindowExceeded or 404 NotFoundError. This causes the original retryable error to be raised instead of the actual non-retryable one. Changes: - Update original_exception to latest error on each retry attempt - Add should_retry_this_error() check inside the retry loop to break out immediately on non-retryable errors - Respect _retry_policy_applies precedence Fixes BerriAI#21343
Fixes #21343
Summary
The retry loop in
async_function_with_retriesswallows non-retryable errors (400 ContextWindowExceeded, 404 NotFound, etc.) and raises the original retryable error instead. This causes misleading error messages and unnecessary retry attempts.Root Cause
Inside the
for current_attempt in range(num_retries)retry loop inrouter.py, theexcept Exceptionblock has two problems:original_exceptionnever updated - stays stuck on the first error, so when retries exhaust, the stale initial error is raised instead of the latest oneshould_retry_this_error()to break on non-retryable errors, but the retry loop itself never does this check, so it sleeps and retries even on 400/404 errorsExample from the issue: First call hits 429 RateLimitError (retryable), retry hits 400 ContextWindowExceeded (non-retryable). Expected: ContextWindowExceededError raised immediately. Actual: loop continues retrying and eventually raises the stale RateLimitError.
Fix
litellm/router.py- two changes inside the retry loop's except block:original_exception = eto always track the latest errorshould_retry_this_error()check (same function already used in the pre-loop code at line 5106) to break out immediately on non-retryable errors. Guarded byif not _retry_policy_appliesto respect retry policy precedencetests/test_litellm/test_router_retry_non_retryable_errors.py- 5 new tests:test_non_retryable_error_in_retry_loop_raises_immediatelytest_bad_request_error_in_retry_loop_raises_immediatelytest_original_exception_updated_to_latest_errortest_retryable_errors_still_retry_normallytest_not_found_error_in_retry_loop_raises_immediatelyType