mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
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.
This commit is contained in:
parent
2de21d0695
commit
b29f9a94bc
10 changed files with 179 additions and 129 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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[
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@
|
|||
"limit": 809
|
||||
},
|
||||
"ANN201": {
|
||||
"limit": 1999
|
||||
"limit": 1998
|
||||
},
|
||||
"ANN202": {
|
||||
"limit": 835
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ const retryPolicyMap: Record<string, string> = {
|
|||
"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;
|
||||
|
|
|
|||
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -35014,6 +35014,8 @@ export interface components {
|
|||
BadRequestErrorRetries?: number | null;
|
||||
/** Contentpolicyviolationerrorretries */
|
||||
ContentPolicyViolationErrorRetries?: number | null;
|
||||
/** Defaultretries */
|
||||
DefaultRetries?: number | null;
|
||||
/** Internalservererrorretries */
|
||||
InternalServerErrorRetries?: number | null;
|
||||
/** Ratelimiterrorretries */
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue