From 902a93d10d2159f66b9f8b5cacbde527d50e88b0 Mon Sep 17 00:00:00 2001 From: Shivam Rawat Date: Tue, 4 Aug 2026 16:51:21 -0700 Subject: [PATCH 1/3] fix(router): honor ServiceUnavailableErrorRetries and InternalServerErrorRetries in retry policy --- litellm/router_utils/get_retry_from_policy.py | 8 ++ litellm/types/router.py | 1 + .../test_get_retry_from_policy.py | 102 ++++++++++++++++++ tests/test_litellm/test_router.py | 31 ++++++ .../components/ModelRetrySettingsTab.tsx | 1 + ui/litellm-dashboard/src/lib/http/schema.d.ts | 2 + 6 files changed, 145 insertions(+) create mode 100644 tests/test_litellm/router_utils/test_get_retry_from_policy.py diff --git a/litellm/router_utils/get_retry_from_policy.py b/litellm/router_utils/get_retry_from_policy.py index 1645e6776fc..fbcc3de83c0 100644 --- a/litellm/router_utils/get_retry_from_policy.py +++ b/litellm/router_utils/get_retry_from_policy.py @@ -8,7 +8,9 @@ from litellm.exceptions import ( AuthenticationError, BadRequestError, ContentPolicyViolationError, + InternalServerError, RateLimitError, + ServiceUnavailableError, Timeout, ) from litellm.types.router import RetryPolicy @@ -26,6 +28,8 @@ def get_num_retries_from_retry_policy( TimeoutErrorRetries: Optional[int] = None RateLimitErrorRetries: Optional[int] = None ContentPolicyViolationErrorRetries: Optional[int] = None + InternalServerErrorRetries: Optional[int] = None + ServiceUnavailableErrorRetries: Optional[int] = None """ # if we can find the exception then in the retry policy -> return the number of retries @@ -48,6 +52,10 @@ def get_num_retries_from_retry_policy( and retry_policy.ContentPolicyViolationErrorRetries is not None ): return retry_policy.ContentPolicyViolationErrorRetries + if isinstance(exception, ServiceUnavailableError) and retry_policy.ServiceUnavailableErrorRetries is not None: + return retry_policy.ServiceUnavailableErrorRetries + if isinstance(exception, InternalServerError) and retry_policy.InternalServerErrorRetries is not None: + return retry_policy.InternalServerErrorRetries if isinstance(exception, BadRequestError) and retry_policy.BadRequestErrorRetries is not None: return retry_policy.BadRequestErrorRetries diff --git a/litellm/types/router.py b/litellm/types/router.py index 21bed84a3a1..e0952d9dd02 100644 --- a/litellm/types/router.py +++ b/litellm/types/router.py @@ -95,6 +95,7 @@ class RetryPolicy(BaseModel): RateLimitErrorRetries: Optional[int] = None ContentPolicyViolationErrorRetries: Optional[int] = None InternalServerErrorRetries: Optional[int] = None + ServiceUnavailableErrorRetries: Optional[int] = None class UpdateRouterConfig(BaseModel): diff --git a/tests/test_litellm/router_utils/test_get_retry_from_policy.py b/tests/test_litellm/router_utils/test_get_retry_from_policy.py new file mode 100644 index 00000000000..a5e239b8595 --- /dev/null +++ b/tests/test_litellm/router_utils/test_get_retry_from_policy.py @@ -0,0 +1,102 @@ +import litellm +from litellm.router_utils.get_retry_from_policy import ( + get_num_retries_from_retry_policy, +) +from litellm.types.router import RetryPolicy + + +def _service_unavailable_error() -> litellm.ServiceUnavailableError: + return litellm.ServiceUnavailableError( + message="model is down", + llm_provider="openai", + model="gpt-5.6", + ) + + +def _internal_server_error() -> litellm.InternalServerError: + return litellm.InternalServerError( + message="upstream 500", + llm_provider="openai", + model="gpt-5.6", + ) + + +def test_service_unavailable_error_retries_honored(): + policy = RetryPolicy(ServiceUnavailableErrorRetries=0) + + assert ( + get_num_retries_from_retry_policy( + exception=_service_unavailable_error(), + retry_policy=policy, + ) + == 0 + ) + + +def test_service_unavailable_error_retries_nonzero(): + policy = RetryPolicy(ServiceUnavailableErrorRetries=4) + + assert ( + get_num_retries_from_retry_policy( + exception=_service_unavailable_error(), + retry_policy=policy, + ) + == 4 + ) + + +def test_internal_server_error_retries_honored(): + policy = RetryPolicy(InternalServerErrorRetries=0) + + assert ( + get_num_retries_from_retry_policy( + exception=_internal_server_error(), + retry_policy=policy, + ) + == 0 + ) + + +def test_service_unavailable_not_covered_by_internal_server_error_retries(): + policy = RetryPolicy(InternalServerErrorRetries=0) + + assert ( + get_num_retries_from_retry_policy( + exception=_service_unavailable_error(), + retry_policy=policy, + ) + is None + ) + + +def test_internal_server_error_not_covered_by_service_unavailable_retries(): + policy = RetryPolicy(ServiceUnavailableErrorRetries=0) + + assert ( + get_num_retries_from_retry_policy( + exception=_internal_server_error(), + retry_policy=policy, + ) + is None + ) + + +def test_service_unavailable_error_retries_from_dict_policy(): + assert ( + get_num_retries_from_retry_policy( + exception=_service_unavailable_error(), + retry_policy={"ServiceUnavailableErrorRetries": 0}, + ) + == 0 + ) + + +def test_service_unavailable_error_retries_from_model_group_policy(): + assert ( + get_num_retries_from_retry_policy( + exception=_service_unavailable_error(), + model_group="gpt-5.6", + model_group_retry_policy={"gpt-5.6": RetryPolicy(ServiceUnavailableErrorRetries=1)}, + ) + == 1 + ) diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 46b5ce65c3f..c98be9c7d80 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -6574,3 +6574,34 @@ def test_model_info_is_active_for_environment_matrix(monkeypatch): monkeypatch.delenv("LITELLM_ENVIRONMENT") with pytest.raises(ValueError, match="LITELLM_ENVIRONMENT"): model_info_is_active_for_environment(model_info={"supported_environments": ["production"]}) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("policy_retries,expected_calls", [(0, 1), (1, 2)]) +async def test_router_retry_policy_service_unavailable_retries(policy_retries, expected_calls): + from litellm.types.router import RetryPolicy + + router = litellm.Router( + model_list=[ + { + "model_name": "gpt-5.6", + "litellm_params": {"model": "openai/gpt-5.6", "api_key": "fake-key"}, + } + ], + retry_policy=RetryPolicy(ServiceUnavailableErrorRetries=policy_retries), + disable_cooldowns=True, + ) + + error = litellm.ServiceUnavailableError( + message="model is down", + llm_provider="openai", + model="gpt-5.6", + ) + with patch.object(litellm, "acompletion", AsyncMock(side_effect=error)) as mock_acompletion: + with pytest.raises(litellm.ServiceUnavailableError): + await router.acompletion( + model="gpt-5.6", + messages=[{"role": "user", "content": "hi"}], + ) + + assert mock_acompletion.call_count == expected_calls diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx index a4e3c4b958c..a9e0b8eb051 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx @@ -30,6 +30,7 @@ const retryPolicyMap: Record = { "RateLimitError (429)": "RateLimitErrorRetries", "ContentPolicyViolationError (400)": "ContentPolicyViolationErrorRetries", "InternalServerError (500)": "InternalServerErrorRetries", + "ServiceUnavailableError (503)": "ServiceUnavailableErrorRetries", }; const ModelRetrySettingsTab = ({ diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 9133bfb5cf4..cf20f86a28d 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -30947,6 +30947,8 @@ export interface components { InternalServerErrorRetries?: number | null; /** Ratelimiterrorretries */ RateLimitErrorRetries?: number | null; + /** Serviceunavailableerrorretries */ + ServiceUnavailableErrorRetries?: number | null; /** Timeouterrorretries */ TimeoutErrorRetries?: number | null; }; From b29f9a94bccd10406fb3a78610041fc397a141c1 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 4 Sep 2026 16:09:01 -0700 Subject: [PATCH 2/3] refactor(router): resolve retry policy by exception MRO and add DefaultRetries Replace the hand-ordered isinstance ladder in get_num_retries_from_retry_policy with a class-to-field mapping walked along the exception's MRO, most specific class first. A RetryPolicy field can no longer go silently dead the way InternalServerErrorRetries did, and subclasses such as ContentPolicyViolationError or MidStreamFallbackError pick up their parent's field when they have none of their own. Add a DefaultRetries catch-all so errors without a dedicated field (BadGatewayError, APIConnectionError, NotFoundError, ...) can be governed by the policy too. Specific fields still win over DefaultRetries. Wiring the previously dead InternalServerErrorRetries changes one test expectation: a policy of 2 now overrides a per-deployment num_retries of 5, so the amplification test sees 3 upstream requests instead of 6. Expose DefaultRetries as "All other errors" in the Admin UI retry settings tab and ratchet the lint budgets down by the violations this branch fixed. --- basedpyright-code-budget.json | 8 +- litellm/router_utils/get_retry_from_policy.py | 83 +++++---- litellm/types/router.py | 1 + ruff-strict-budget.json | 2 +- .../test_get_retry_from_policy.py | 169 +++++++++++------- tests/test_litellm/test_router.py | 31 ++-- .../test_router_per_deployment_num_retries.py | 7 +- type-discipline-budget.json | 4 +- .../components/ModelRetrySettingsTab.tsx | 1 + ui/litellm-dashboard/src/lib/http/schema.d.ts | 2 + 10 files changed, 179 insertions(+), 129 deletions(-) diff --git a/basedpyright-code-budget.json b/basedpyright-code-budget.json index 9b59480a0dc..669107bb5b1 100644 --- a/basedpyright-code-budget.json +++ b/basedpyright-code-budget.json @@ -57,7 +57,7 @@ "limit": 5601 }, "reportMissingTypeArgument": { - "limit": 15285 + "limit": 15284 }, "reportMissingTypeStubs": { "limit": 40 @@ -99,7 +99,7 @@ "limit": 0 }, "reportUnknownArgumentType": { - "limit": 44360 + "limit": 44358 }, "reportUnknownLambdaType": { "limit": 109 @@ -108,10 +108,10 @@ "limit": 38309 }, "reportUnknownParameterType": { - "limit": 19622 + "limit": 19621 }, "reportUnknownVariableType": { - "limit": 29846 + "limit": 29844 }, "reportUnnecessaryCast": { "limit": 111 diff --git a/litellm/router_utils/get_retry_from_policy.py b/litellm/router_utils/get_retry_from_policy.py index 051cde127bf..ad4a6b0be99 100644 --- a/litellm/router_utils/get_retry_from_policy.py +++ b/litellm/router_utils/get_retry_from_policy.py @@ -1,8 +1,8 @@ -""" -Get num retries for an exception. +"""Resolve how many retries a RetryPolicy grants for a given exception.""" -- Account for retry policy by exception type. -""" +from collections.abc import Callable, Mapping +from types import MappingProxyType +from typing import Final from litellm.exceptions import ( AuthenticationError, @@ -15,49 +15,48 @@ from litellm.exceptions import ( ) from litellm.types.router import RetryPolicy +_RETRIES_BY_EXCEPTION_TYPE: Final[Mapping[type, Callable[[RetryPolicy], int | None]]] = MappingProxyType( + { + AuthenticationError: lambda policy: policy.AuthenticationErrorRetries, + Timeout: lambda policy: policy.TimeoutErrorRetries, + RateLimitError: lambda policy: policy.RateLimitErrorRetries, + ContentPolicyViolationError: lambda policy: policy.ContentPolicyViolationErrorRetries, + BadRequestError: lambda policy: policy.BadRequestErrorRetries, + ServiceUnavailableError: lambda policy: policy.ServiceUnavailableErrorRetries, + InternalServerError: lambda policy: policy.InternalServerErrorRetries, + } +) + + +def _resolve_policy( + retry_policy: RetryPolicy | Mapping[str, int | None] | None, + model_group: str | None, + model_group_retry_policy: Mapping[str, RetryPolicy | Mapping[str, int | None]] | None, +) -> RetryPolicy | None: + selected: Final = ( + model_group_retry_policy[model_group] + if model_group_retry_policy is not None and model_group is not None and model_group in model_group_retry_policy + else retry_policy + ) + if isinstance(selected, Mapping): + return RetryPolicy(**selected) + return selected + def get_num_retries_from_retry_policy( exception: Exception, - retry_policy: RetryPolicy | dict | None = None, + retry_policy: RetryPolicy | Mapping[str, int | None] | None = None, model_group: str | None = None, - model_group_retry_policy: dict[str, RetryPolicy] | None = None, -): - """ - BadRequestErrorRetries: Optional[int] = None - AuthenticationErrorRetries: Optional[int] = None - TimeoutErrorRetries: Optional[int] = None - RateLimitErrorRetries: Optional[int] = None - ContentPolicyViolationErrorRetries: Optional[int] = None - InternalServerErrorRetries: Optional[int] = None - ServiceUnavailableErrorRetries: Optional[int] = None - """ - # if we can find the exception then in the retry policy -> return the number of retries - - if model_group_retry_policy is not None and model_group is not None and model_group in model_group_retry_policy: - retry_policy = model_group_retry_policy.get(model_group, None) - - if retry_policy is None: + model_group_retry_policy: Mapping[str, RetryPolicy | Mapping[str, int | None]] | None = None, +) -> int | None: + """Walk the exception's MRO, most specific class first, and return the first configured retry count.""" + policy: Final = _resolve_policy(retry_policy, model_group, model_group_retry_policy) + if policy is None: return None - if isinstance(retry_policy, dict): - retry_policy = RetryPolicy(**retry_policy) - - if isinstance(exception, AuthenticationError) and retry_policy.AuthenticationErrorRetries is not None: - return retry_policy.AuthenticationErrorRetries - if isinstance(exception, Timeout) and retry_policy.TimeoutErrorRetries is not None: - return retry_policy.TimeoutErrorRetries - if isinstance(exception, RateLimitError) and retry_policy.RateLimitErrorRetries is not None: - return retry_policy.RateLimitErrorRetries - if ( - isinstance(exception, ContentPolicyViolationError) - and retry_policy.ContentPolicyViolationErrorRetries is not None - ): - return retry_policy.ContentPolicyViolationErrorRetries - if isinstance(exception, ServiceUnavailableError) and retry_policy.ServiceUnavailableErrorRetries is not None: - return retry_policy.ServiceUnavailableErrorRetries - if isinstance(exception, InternalServerError) and retry_policy.InternalServerErrorRetries is not None: - return retry_policy.InternalServerErrorRetries - if isinstance(exception, BadRequestError) and retry_policy.BadRequestErrorRetries is not None: - return retry_policy.BadRequestErrorRetries + configured: Final = ( + _RETRIES_BY_EXCEPTION_TYPE[cls](policy) for cls in type(exception).__mro__ if cls in _RETRIES_BY_EXCEPTION_TYPE + ) + return next((retries for retries in configured if retries is not None), policy.DefaultRetries) def reset_retry_policy() -> RetryPolicy: diff --git a/litellm/types/router.py b/litellm/types/router.py index 6ed9b3efd03..267e8853db1 100644 --- a/litellm/types/router.py +++ b/litellm/types/router.py @@ -105,6 +105,7 @@ class RetryPolicy(BaseModel): ContentPolicyViolationErrorRetries: int | None = None InternalServerErrorRetries: int | None = None ServiceUnavailableErrorRetries: int | None = None + DefaultRetries: int | None = None OptionalPreCallChecks = list[ diff --git a/ruff-strict-budget.json b/ruff-strict-budget.json index 4aac1756af4..70408ea022b 100644 --- a/ruff-strict-budget.json +++ b/ruff-strict-budget.json @@ -9,7 +9,7 @@ "limit": 809 }, "ANN201": { - "limit": 1999 + "limit": 1998 }, "ANN202": { "limit": 835 diff --git a/tests/test_litellm/router_utils/test_get_retry_from_policy.py b/tests/test_litellm/router_utils/test_get_retry_from_policy.py index a5e239b8595..df157ea5ff7 100644 --- a/tests/test_litellm/router_utils/test_get_retry_from_policy.py +++ b/tests/test_litellm/router_utils/test_get_retry_from_policy.py @@ -1,102 +1,147 @@ +from types import MappingProxyType +from typing import Final + +import pytest + import litellm -from litellm.router_utils.get_retry_from_policy import ( - get_num_retries_from_retry_policy, -) +from litellm.router_utils.get_retry_from_policy import get_num_retries_from_retry_policy from litellm.types.router import RetryPolicy +_EXCEPTION_FOR_FIELD: Final = MappingProxyType( + { + "BadRequestErrorRetries": litellm.BadRequestError, + "AuthenticationErrorRetries": litellm.AuthenticationError, + "TimeoutErrorRetries": litellm.Timeout, + "RateLimitErrorRetries": litellm.RateLimitError, + "ContentPolicyViolationErrorRetries": litellm.ContentPolicyViolationError, + "InternalServerErrorRetries": litellm.InternalServerError, + "ServiceUnavailableErrorRetries": litellm.ServiceUnavailableError, + } +) -def _service_unavailable_error() -> litellm.ServiceUnavailableError: - return litellm.ServiceUnavailableError( - message="model is down", - llm_provider="openai", - model="gpt-5.6", +_SPECIFIC_FIELDS: Final = tuple(name for name in RetryPolicy.model_fields if name != "DefaultRetries") + + +def _error(exception_type: type[Exception]) -> Exception: + return exception_type(message="boom", llm_provider="openai", model="gpt-5.6") + + +@pytest.mark.parametrize("field", _SPECIFIC_FIELDS) +def test_every_specific_field_controls_retries_for_its_exception(field: str): + exception: Final = _error(_EXCEPTION_FOR_FIELD[field]) + + assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(**{field: 0})) == 0 + assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(**{field: 4})) == 4 + + +@pytest.mark.parametrize("field", _SPECIFIC_FIELDS) +def test_specific_field_does_not_apply_to_unrelated_exceptions(field: str): + policy: Final = RetryPolicy(**{field: 0}) + unrelated: Final = tuple( + exception_type + for name, exception_type in _EXCEPTION_FOR_FIELD.items() + if name != field and not issubclass(exception_type, _EXCEPTION_FOR_FIELD[field]) ) - -def _internal_server_error() -> litellm.InternalServerError: - return litellm.InternalServerError( - message="upstream 500", - llm_provider="openai", - model="gpt-5.6", - ) + for exception_type in unrelated: + assert get_num_retries_from_retry_policy(exception=_error(exception_type), retry_policy=policy) is None -def test_service_unavailable_error_retries_honored(): - policy = RetryPolicy(ServiceUnavailableErrorRetries=0) +def test_subclass_prefers_its_own_field_over_the_parent_field(): + policy: Final = RetryPolicy(BadRequestErrorRetries=5, ContentPolicyViolationErrorRetries=1) assert ( - get_num_retries_from_retry_policy( - exception=_service_unavailable_error(), - retry_policy=policy, - ) - == 0 + get_num_retries_from_retry_policy(exception=_error(litellm.ContentPolicyViolationError), retry_policy=policy) + == 1 + ) + assert get_num_retries_from_retry_policy(exception=_error(litellm.BadRequestError), retry_policy=policy) == 5 + + +def test_subclass_falls_back_to_the_parent_field(): + policy: Final = RetryPolicy(BadRequestErrorRetries=5) + + assert ( + get_num_retries_from_retry_policy(exception=_error(litellm.ContentPolicyViolationError), retry_policy=policy) + == 5 ) -def test_service_unavailable_error_retries_nonzero(): - policy = RetryPolicy(ServiceUnavailableErrorRetries=4) +@pytest.mark.parametrize("exception_type", (litellm.BadGatewayError, litellm.NotFoundError)) +def test_default_retries_covers_exceptions_without_a_specific_field(exception_type: type[Exception]): + exception: Final = _error(exception_type) + assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(DefaultRetries=0)) == 0 assert ( get_num_retries_from_retry_policy( - exception=_service_unavailable_error(), - retry_policy=policy, - ) - == 4 - ) - - -def test_internal_server_error_retries_honored(): - policy = RetryPolicy(InternalServerErrorRetries=0) - - assert ( - get_num_retries_from_retry_policy( - exception=_internal_server_error(), - retry_policy=policy, - ) - == 0 - ) - - -def test_service_unavailable_not_covered_by_internal_server_error_retries(): - policy = RetryPolicy(InternalServerErrorRetries=0) - - assert ( - get_num_retries_from_retry_policy( - exception=_service_unavailable_error(), - retry_policy=policy, + exception=exception, retry_policy=RetryPolicy(ServiceUnavailableErrorRetries=0) ) is None ) -def test_internal_server_error_not_covered_by_service_unavailable_retries(): - policy = RetryPolicy(ServiceUnavailableErrorRetries=0) +def test_specific_field_wins_over_default_retries(): + policy: Final = RetryPolicy(DefaultRetries=0, RateLimitErrorRetries=3) + + assert get_num_retries_from_retry_policy(exception=_error(litellm.RateLimitError), retry_policy=policy) == 3 + assert get_num_retries_from_retry_policy(exception=_error(litellm.BadGatewayError), retry_policy=policy) == 0 + + +def test_default_retries_applies_when_the_specific_field_is_unset(): + policy: Final = RetryPolicy(DefaultRetries=2) assert ( - get_num_retries_from_retry_policy( - exception=_internal_server_error(), - retry_policy=policy, - ) - is None + get_num_retries_from_retry_policy(exception=_error(litellm.ServiceUnavailableError), retry_policy=policy) == 2 ) -def test_service_unavailable_error_retries_from_dict_policy(): +def test_empty_policy_matches_nothing(): + assert ( + get_num_retries_from_retry_policy(exception=_error(litellm.ServiceUnavailableError), retry_policy=RetryPolicy()) + is None + ) + assert ( + get_num_retries_from_retry_policy(exception=_error(litellm.ServiceUnavailableError), retry_policy=None) is None + ) + + +def test_dict_policy_is_accepted(): assert ( get_num_retries_from_retry_policy( - exception=_service_unavailable_error(), + exception=_error(litellm.ServiceUnavailableError), retry_policy={"ServiceUnavailableErrorRetries": 0}, ) == 0 ) -def test_service_unavailable_error_retries_from_model_group_policy(): +def test_model_group_policy_replaces_the_global_policy(): + exception: Final = _error(litellm.ServiceUnavailableError) + global_policy: Final = RetryPolicy(ServiceUnavailableErrorRetries=5) + assert ( get_num_retries_from_retry_policy( - exception=_service_unavailable_error(), + exception=exception, + retry_policy=global_policy, model_group="gpt-5.6", - model_group_retry_policy={"gpt-5.6": RetryPolicy(ServiceUnavailableErrorRetries=1)}, + model_group_retry_policy={"gpt-5.6": {"ServiceUnavailableErrorRetries": 1}}, ) == 1 ) + assert ( + get_num_retries_from_retry_policy( + exception=exception, + retry_policy=global_policy, + model_group="gpt-5.6", + model_group_retry_policy={"gpt-5.6": RetryPolicy(RateLimitErrorRetries=1)}, + ) + is None + ) + assert ( + get_num_retries_from_retry_policy( + exception=exception, + retry_policy=global_policy, + model_group="other-group", + model_group_retry_policy={"gpt-5.6": RetryPolicy(ServiceUnavailableErrorRetries=1)}, + ) + == 5 + ) diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index cb3baf042dd..5d83d0f8877 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -12896,10 +12896,17 @@ async def test_prompt_management_factory_marks_injection_for_every_deployment(mo @pytest.mark.asyncio -@pytest.mark.parametrize("policy_retries,expected_calls", [(0, 1), (1, 2)]) -async def test_router_retry_policy_service_unavailable_retries(policy_retries, expected_calls): - from litellm.types.router import RetryPolicy - +@pytest.mark.parametrize( + "retry_policy,error_type,expected_calls", + [ + ({"ServiceUnavailableErrorRetries": 0}, litellm.ServiceUnavailableError, 1), + ({"ServiceUnavailableErrorRetries": 1}, litellm.ServiceUnavailableError, 2), + ({"InternalServerErrorRetries": 0}, litellm.InternalServerError, 1), + ({"DefaultRetries": 0}, litellm.BadGatewayError, 1), + ({"DefaultRetries": 0, "ServiceUnavailableErrorRetries": 1}, litellm.ServiceUnavailableError, 2), + ], +) +async def test_router_retry_policy_controls_attempt_count(retry_policy, error_type, expected_calls): router = litellm.Router( model_list=[ { @@ -12907,20 +12914,14 @@ async def test_router_retry_policy_service_unavailable_retries(policy_retries, e "litellm_params": {"model": "openai/gpt-5.6", "api_key": "fake-key"}, } ], - retry_policy=RetryPolicy(ServiceUnavailableErrorRetries=policy_retries), + num_retries=2, + retry_policy=retry_policy, disable_cooldowns=True, ) + error = error_type(message="model is down", llm_provider="openai", model="gpt-5.6") - error = litellm.ServiceUnavailableError( - message="model is down", - llm_provider="openai", - model="gpt-5.6", - ) with patch.object(litellm, "acompletion", AsyncMock(side_effect=error)) as mock_acompletion: - with pytest.raises(litellm.ServiceUnavailableError): - await router.acompletion( - model="gpt-5.6", - messages=[{"role": "user", "content": "hi"}], - ) + with pytest.raises(error_type): + await router.acompletion(model="gpt-5.6", messages=[{"role": "user", "content": "hi"}]) assert mock_acompletion.call_count == expected_calls diff --git a/tests/test_litellm/test_router_per_deployment_num_retries.py b/tests/test_litellm/test_router_per_deployment_num_retries.py index d75e32a1821..99ad7c224f8 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -415,8 +415,9 @@ class TestNoProviderRetryAmplification: @pytest.mark.asyncio async def test_retry_policy_configured_does_not_reintroduce_amplification(self): """ - With a retry policy configured alongside a per-deployment ``num_retries=5``, the - provider SDK still must not retry: exactly ``6`` upstream requests, not 36. + ``InternalServerErrorRetries=2`` overrides the per-deployment ``num_retries=5`` for the + 500s this upstream returns, and the provider SDK still must not retry on top: exactly + ``3`` upstream requests, not 18. """ router = self._router( "https://policy.local/v1", @@ -424,7 +425,7 @@ class TestNoProviderRetryAmplification: num_retries=1, retry_policy=RetryPolicy(InternalServerErrorRetries=2), ) - assert await self._call_and_count(router) == 6 + assert await self._call_and_count(router) == 3 @pytest.mark.asyncio async def test_global_num_retries_not_amplified(self): diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 8589a9451cf..3d01c08e8eb 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -1,6 +1,6 @@ { "LIT001": { - "limit": 22328 + "limit": 22326 }, "LIT002": { "limit": 26748 @@ -30,7 +30,7 @@ "limit": 16468 }, "LIT011": { - "limit": 5514 + "limit": 5512 }, "LIT012": { "limit": 4487 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx index 9d6501c97ba..069a3f27beb 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelRetrySettingsTab.tsx @@ -35,6 +35,7 @@ const retryPolicyMap: Record = { "ContentPolicyViolationError (400)": "ContentPolicyViolationErrorRetries", "InternalServerError (500)": "InternalServerErrorRetries", "ServiceUnavailableError (503)": "ServiceUnavailableErrorRetries", + "All other errors": "DefaultRetries", }; const isValidRetryCount = (value: number) => Number.isFinite(value) && Number.isInteger(value) && value >= 0; diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 549b9c0d01d..7d7fa8d7fe6 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -35014,6 +35014,8 @@ export interface components { BadRequestErrorRetries?: number | null; /** Contentpolicyviolationerrorretries */ ContentPolicyViolationErrorRetries?: number | null; + /** Defaultretries */ + DefaultRetries?: number | null; /** Internalservererrorretries */ InternalServerErrorRetries?: number | null; /** Ratelimiterrorretries */ From 541ab50c043be762fb73d73cf2ae648235e06112 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 4 Sep 2026 16:23:08 -0700 Subject: [PATCH 3/3] test(router): fake the upstream with respx in the retry policy attempt test The test-quality gate rejects patching litellm.acompletion, and faking the HTTP boundary is the stronger test anyway: the 503, 500 and 502 responses now travel through the real OpenAI SDK and exception mapping before the router decides how many times to retry. Adds a case showing that a 503 key does not govern a 502. --- tests/test_litellm/test_router.py | 39 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 5d83d0f8877..31eb46f1458 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -12,6 +12,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import httpx import openai import pytest +import respx @@ -567,7 +568,6 @@ async def test_async_router_acancel_batch_does_not_fall_back_across_model_groups model string, and the fallback provider is then asked to cancel a batch it never issued, which can only answer not-found. The router re-raises the owner's error after that wasted round trip, so the pin's observable is the foreign call never happening.""" - import respx monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) router = litellm.Router( @@ -716,7 +716,6 @@ async def test_async_router_acreate_file_litellm_proxy_sends_target_model_names_ from io import BytesIO import httpx - import respx jsonl_file = BytesIO( json.dumps({"body": {"model": "chained-batch", "messages": [{"role": "user", "content": "hi"}]}}).encode( @@ -12897,31 +12896,45 @@ async def test_prompt_management_factory_marks_injection_for_every_deployment(mo @pytest.mark.asyncio @pytest.mark.parametrize( - "retry_policy,error_type,expected_calls", + "retry_policy,upstream_status,error_type,expected_upstream_calls", [ - ({"ServiceUnavailableErrorRetries": 0}, litellm.ServiceUnavailableError, 1), - ({"ServiceUnavailableErrorRetries": 1}, litellm.ServiceUnavailableError, 2), - ({"InternalServerErrorRetries": 0}, litellm.InternalServerError, 1), - ({"DefaultRetries": 0}, litellm.BadGatewayError, 1), - ({"DefaultRetries": 0, "ServiceUnavailableErrorRetries": 1}, litellm.ServiceUnavailableError, 2), + ({"ServiceUnavailableErrorRetries": 0}, 503, litellm.ServiceUnavailableError, 1), + ({"ServiceUnavailableErrorRetries": 1}, 503, litellm.ServiceUnavailableError, 2), + ({"InternalServerErrorRetries": 0}, 500, litellm.InternalServerError, 1), + ({"DefaultRetries": 0}, 502, litellm.BadGatewayError, 1), + ({"DefaultRetries": 0, "ServiceUnavailableErrorRetries": 1}, 503, litellm.ServiceUnavailableError, 2), + ({"ServiceUnavailableErrorRetries": 0}, 502, litellm.BadGatewayError, 3), ], ) -async def test_router_retry_policy_controls_attempt_count(retry_policy, error_type, expected_calls): +async def test_router_retry_policy_controls_upstream_attempt_count( + monkeypatch: pytest.MonkeyPatch, retry_policy, upstream_status, error_type, expected_upstream_calls +): + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) router = litellm.Router( model_list=[ { "model_name": "gpt-5.6", - "litellm_params": {"model": "openai/gpt-5.6", "api_key": "fake-key"}, + "litellm_params": { + "model": "openai/gpt-5.6", + "api_key": "sk-fake", + "api_base": "https://retry-policy.local/v1", + }, } ], num_retries=2, retry_policy=retry_policy, disable_cooldowns=True, ) - error = error_type(message="model is down", llm_provider="openai", model="gpt-5.6") - with patch.object(litellm, "acompletion", AsyncMock(side_effect=error)) as mock_acompletion: + with respx.mock(assert_all_called=True) as respx_mock: + upstream = respx_mock.post("https://retry-policy.local/v1/chat/completions").mock( + return_value=httpx.Response( + upstream_status, + headers={"retry-after": "0"}, + json={"error": {"message": "model is down", "type": "server_error"}}, + ) + ) with pytest.raises(error_type): await router.acompletion(model="gpt-5.6", messages=[{"role": "user", "content": "hi"}]) - assert mock_acompletion.call_count == expected_calls + assert upstream.call_count == expected_upstream_calls