mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(router): cooldown 429 errors wrapped as APIConnectionError
OpenAI-like providers registered via providers.json have their HTTP 429 responses incorrectly mapped to APIConnectionError by the catch-all exception handler in exception_mapping_utils.py (because they are not in litellm._openai_like_providers). The cooldown handler in _is_cooldown_required() blanket-ignores all exceptions containing 'APIConnectionError', which means rate-limited deployments are never cooled down and the router keeps retrying the same failing deployment instead of routing to healthy alternatives. This fix adds a _is_rate_limit_error() helper that checks the actual HTTP status code (not string matching) to detect whether an APIConnectionError wraps a 429 response. If so, cooldown proceeds normally so the router can pick a healthy deployment. Fixes #24366
This commit is contained in:
parent
c89496f378
commit
0741e49d79
2 changed files with 92 additions and 0 deletions
|
|
@ -37,6 +37,24 @@ else:
|
|||
Span = Any
|
||||
|
||||
|
||||
def _is_rate_limit_error(
|
||||
exception_status: Union[str, int],
|
||||
) -> bool:
|
||||
"""
|
||||
Check if an exception status code indicates a rate limit error (HTTP 429).
|
||||
|
||||
Some OpenAI-like providers registered via providers.json have their HTTP
|
||||
429 responses incorrectly mapped to APIConnectionError by the catch-all
|
||||
exception handler in exception_mapping_utils.py. This helper checks the
|
||||
actual HTTP status code so the cooldown handler can still detect these.
|
||||
"""
|
||||
try:
|
||||
status = int(exception_status) if isinstance(exception_status, str) and exception_status else exception_status
|
||||
return status == 429
|
||||
except (ValueError, TypeError):
|
||||
return False
|
||||
|
||||
|
||||
def _is_cooldown_required(
|
||||
litellm_router_instance: LitellmRouter,
|
||||
model_id: str,
|
||||
|
|
@ -60,6 +78,13 @@ def _is_cooldown_required(
|
|||
): # don't cooldown on litellm api connection errors errors
|
||||
for ignored_string in ignored_strings:
|
||||
if ignored_string in exception_str:
|
||||
# Don't skip cooldown when the APIConnectionError wraps
|
||||
# a rate limit (429). Some providers (e.g. those registered
|
||||
# via providers.json) have their 429 errors mapped to
|
||||
# APIConnectionError instead of RateLimitError because they
|
||||
# fall through to the catch-all exception handler.
|
||||
if _is_rate_limit_error(exception_status):
|
||||
return True
|
||||
return False
|
||||
|
||||
if isinstance(exception_status, str):
|
||||
|
|
|
|||
|
|
@ -469,3 +469,70 @@ def test_should_cooldown_deployment_minimum_request_threshold(testing_litellm_ro
|
|||
assert (
|
||||
should_cooldown is True
|
||||
), f"Should cooldown when we have {DEFAULT_FAILURE_THRESHOLD_MINIMUM_REQUESTS} failed requests (100% failure rate)"
|
||||
|
||||
|
||||
def test_is_cooldown_required_429_wrapped_as_apiconnectionerror():
|
||||
"""
|
||||
Test that _is_cooldown_required returns True when a 429 rate limit error
|
||||
is wrapped as an APIConnectionError.
|
||||
|
||||
Regression test for: https://github.com/BerriAI/litellm/issues/24366
|
||||
|
||||
Some OpenAI-like providers (registered via providers.json) have their 429
|
||||
responses mapped to APIConnectionError by the catch-all exception handler.
|
||||
The cooldown handler should still recognize these as rate limit errors and
|
||||
trigger a cooldown.
|
||||
"""
|
||||
mock_router = MagicMock()
|
||||
mock_router.allowed_fails = 0
|
||||
mock_router.disable_cooldowns = False
|
||||
mock_router.failed_calls = MagicMock()
|
||||
mock_router.failed_calls.get_cache.return_value = None
|
||||
|
||||
# This is the actual exception string produced when a providers.json
|
||||
# provider returns HTTP 429
|
||||
exception_str = (
|
||||
"litellm.APIConnectionError: ProviderException - "
|
||||
"Error code: 429 - {'error': {'message': 'Rate limit exceeded. "
|
||||
"Retry in 6s.', 'type': 'rate_limit'}}"
|
||||
)
|
||||
|
||||
result = _is_cooldown_required(
|
||||
litellm_router_instance=mock_router,
|
||||
model_id="test-deployment-id",
|
||||
exception_status="429",
|
||||
exception_str=exception_str,
|
||||
)
|
||||
|
||||
assert result is True, (
|
||||
"_is_cooldown_required should return True for 429 errors "
|
||||
"wrapped as APIConnectionError"
|
||||
)
|
||||
|
||||
|
||||
def test_is_cooldown_required_genuine_apiconnectionerror():
|
||||
"""
|
||||
Test that _is_cooldown_required still returns False for genuine
|
||||
APIConnectionError (no 429 / rate limit indicators).
|
||||
"""
|
||||
mock_router = MagicMock()
|
||||
mock_router.allowed_fails = 0
|
||||
mock_router.disable_cooldowns = False
|
||||
mock_router.failed_calls = MagicMock()
|
||||
mock_router.failed_calls.get_cache.return_value = None
|
||||
|
||||
exception_str = (
|
||||
"litellm.APIConnectionError: ProviderException - Connection refused"
|
||||
)
|
||||
|
||||
result = _is_cooldown_required(
|
||||
litellm_router_instance=mock_router,
|
||||
model_id="test-deployment-id",
|
||||
exception_status="",
|
||||
exception_str=exception_str,
|
||||
)
|
||||
|
||||
assert result is False, (
|
||||
"_is_cooldown_required should return False for genuine "
|
||||
"connection errors (no rate limit)"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue