diff --git a/litellm/router_utils/cooldown_handlers.py b/litellm/router_utils/cooldown_handlers.py index 81bfac2ad19..39e81e89c01 100644 --- a/litellm/router_utils/cooldown_handlers.py +++ b/litellm/router_utils/cooldown_handlers.py @@ -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): diff --git a/tests/router_unit_tests/test_router_cooldown_utils.py b/tests/router_unit_tests/test_router_cooldown_utils.py index 33640ad8581..8a3351e82f2 100644 --- a/tests/router_unit_tests/test_router_cooldown_utils.py +++ b/tests/router_unit_tests/test_router_cooldown_utils.py @@ -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)" + )