mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(router): add NotFoundErrorRetries so a retry policy can pin 404 retries
RetryPolicy had no field for 404s, so any policy that set DefaultRetries made the router retry every 404 across the pool, including OpenAI's 404 on a missing response id, which arrives as a BadRequestError whose status_code is 404 NotFoundErrorRetries now governs every answer whose status code is 404 whatever exception class the mapping picked, ahead of the class walk and DefaultRetries. A 404 without it still falls back to BadRequestErrorRetries for the BadRequestError shape and then to DefaultRetries, so existing policies keep their behavior until the new field is set. The Admin UI retry settings tab gains a NotFoundError (404) row above the catch-all row Fixes #36896
This commit is contained in:
parent
0058e1554b
commit
a9ad3eaf95
7 changed files with 134 additions and 3 deletions
|
|
@ -1,6 +1,7 @@
|
|||
"""Resolve how many retries a RetryPolicy grants for a given exception."""
|
||||
|
||||
from collections.abc import Callable, Mapping
|
||||
from itertools import chain
|
||||
from types import MappingProxyType
|
||||
from typing import Final
|
||||
|
||||
|
|
@ -28,6 +29,11 @@ _RETRIES_BY_EXCEPTION_TYPE: Final[Mapping[type, Callable[[RetryPolicy], int | No
|
|||
)
|
||||
|
||||
|
||||
def _retries_for_a_404_answer(exception: Exception, policy: RetryPolicy) -> int | None:
|
||||
status_code: Final = getattr(exception, "status_code", None)
|
||||
return policy.NotFoundErrorRetries if status_code == 404 else None
|
||||
|
||||
|
||||
def _resolve_policy(
|
||||
retry_policy: RetryPolicy | Mapping[str, int | None] | None,
|
||||
model_group: str | None,
|
||||
|
|
@ -49,13 +55,14 @@ def get_num_retries_from_retry_policy(
|
|||
model_group: str | None = 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."""
|
||||
"""Prefer NotFoundErrorRetries for any 404 answer, then walk the exception's MRO most specific class first."""
|
||||
policy: Final = _resolve_policy(retry_policy, model_group, model_group_retry_policy)
|
||||
if policy is None:
|
||||
return None
|
||||
configured: Final = (
|
||||
by_class: Final = (
|
||||
_RETRIES_BY_EXCEPTION_TYPE[cls](policy) for cls in type(exception).__mro__ if cls in _RETRIES_BY_EXCEPTION_TYPE
|
||||
)
|
||||
configured: Final = chain((_retries_for_a_404_answer(exception, policy),), by_class)
|
||||
return next((retries for retries in configured if retries is not None), policy.DefaultRetries)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -109,6 +109,7 @@ class RetryPolicy(BaseModel):
|
|||
ContentPolicyViolationErrorRetries: int | None = None
|
||||
InternalServerErrorRetries: int | None = None
|
||||
ServiceUnavailableErrorRetries: int | None = None
|
||||
NotFoundErrorRetries: int | None = None
|
||||
DefaultRetries: int | None = None
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
from types import MappingProxyType
|
||||
from typing import Final
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
import litellm
|
||||
|
|
@ -16,6 +17,7 @@ _EXCEPTION_FOR_FIELD: Final = MappingProxyType(
|
|||
"ContentPolicyViolationErrorRetries": litellm.ContentPolicyViolationError,
|
||||
"InternalServerErrorRetries": litellm.InternalServerError,
|
||||
"ServiceUnavailableErrorRetries": litellm.ServiceUnavailableError,
|
||||
"NotFoundErrorRetries": litellm.NotFoundError,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -26,6 +28,17 @@ def _error(exception_type: type[Exception]) -> Exception:
|
|||
return exception_type(message="boom", llm_provider="openai", model="gpt-5.6")
|
||||
|
||||
|
||||
def _bad_request_answered_with_404() -> litellm.BadRequestError:
|
||||
upstream: Final = httpx.Response(
|
||||
404, request=httpx.Request("GET", "https://api.openai.com/v1/responses/resp_missing")
|
||||
)
|
||||
exception: Final = litellm.BadRequestError(
|
||||
message="Response with id 'resp_missing' not found.", llm_provider="openai", model="gpt-5.6", response=upstream
|
||||
)
|
||||
assert exception.status_code == 404
|
||||
return exception
|
||||
|
||||
|
||||
@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])
|
||||
|
|
@ -66,7 +79,7 @@ def test_subclass_falls_back_to_the_parent_field():
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("exception_type", (litellm.BadGatewayError, litellm.NotFoundError))
|
||||
@pytest.mark.parametrize("exception_type", (litellm.BadGatewayError,))
|
||||
def test_default_retries_covers_exceptions_without_a_specific_field(exception_type: type[Exception]):
|
||||
exception: Final = _error(exception_type)
|
||||
|
||||
|
|
@ -86,6 +99,46 @@ def test_specific_field_wins_over_default_retries():
|
|||
assert get_num_retries_from_retry_policy(exception=_error(litellm.BadGatewayError), retry_policy=policy) == 0
|
||||
|
||||
|
||||
def test_not_found_retries_governs_a_bad_request_error_answered_with_404():
|
||||
exception: Final = _bad_request_answered_with_404()
|
||||
|
||||
assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(NotFoundErrorRetries=0)) == 0
|
||||
assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(NotFoundErrorRetries=4)) == 4
|
||||
|
||||
|
||||
def test_not_found_retries_wins_over_bad_request_and_default_retries_for_a_404():
|
||||
policy: Final = RetryPolicy(NotFoundErrorRetries=0, BadRequestErrorRetries=5, DefaultRetries=3)
|
||||
|
||||
assert get_num_retries_from_retry_policy(exception=_bad_request_answered_with_404(), retry_policy=policy) == 0
|
||||
assert get_num_retries_from_retry_policy(exception=_error(litellm.NotFoundError), retry_policy=policy) == 0
|
||||
|
||||
|
||||
def test_a_404_without_not_found_retries_falls_back_to_bad_request_then_default_retries():
|
||||
exception: Final = _bad_request_answered_with_404()
|
||||
|
||||
assert (
|
||||
get_num_retries_from_retry_policy(
|
||||
exception=exception, retry_policy=RetryPolicy(BadRequestErrorRetries=0, DefaultRetries=3)
|
||||
)
|
||||
== 0
|
||||
)
|
||||
assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(DefaultRetries=3)) == 3
|
||||
assert get_num_retries_from_retry_policy(exception=_error(litellm.NotFoundError), retry_policy=RetryPolicy(DefaultRetries=3)) == 3
|
||||
|
||||
|
||||
def test_not_found_retries_leaves_a_plain_400_alone():
|
||||
exception: Final = _error(litellm.BadRequestError)
|
||||
assert exception.status_code == 400
|
||||
|
||||
assert get_num_retries_from_retry_policy(exception=exception, retry_policy=RetryPolicy(NotFoundErrorRetries=0)) is None
|
||||
assert (
|
||||
get_num_retries_from_retry_policy(
|
||||
exception=exception, retry_policy=RetryPolicy(NotFoundErrorRetries=0, BadRequestErrorRetries=2)
|
||||
)
|
||||
== 2
|
||||
)
|
||||
|
||||
|
||||
def test_default_retries_applies_when_the_specific_field_is_unset():
|
||||
policy: Final = RetryPolicy(DefaultRetries=2)
|
||||
|
||||
|
|
|
|||
|
|
@ -15391,6 +15391,53 @@ async def test_router_retry_policy_controls_upstream_attempt_count(
|
|||
assert upstream.call_count == expected_upstream_calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"error_body,error_type",
|
||||
[
|
||||
({"message": "model is down", "type": "server_error"}, litellm.NotFoundError),
|
||||
({"message": "Response with id 'resp_x' not found.", "type": "invalid_request_error"}, litellm.BadRequestError),
|
||||
],
|
||||
)
|
||||
@pytest.mark.parametrize(
|
||||
"retry_policy,expected_upstream_calls",
|
||||
[
|
||||
({"DefaultRetries": 3}, 4),
|
||||
({"DefaultRetries": 3, "NotFoundErrorRetries": 0}, 1),
|
||||
({"NotFoundErrorRetries": 2}, 3),
|
||||
],
|
||||
)
|
||||
async def test_router_not_found_retries_governs_every_404_shape(
|
||||
monkeypatch: pytest.MonkeyPatch, retry_policy, expected_upstream_calls, error_body, error_type
|
||||
):
|
||||
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": "sk-fake",
|
||||
"api_base": "https://retry-policy.local/v1",
|
||||
},
|
||||
}
|
||||
],
|
||||
num_retries=2,
|
||||
retry_policy=retry_policy,
|
||||
disable_cooldowns=True,
|
||||
)
|
||||
|
||||
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(404, headers={"retry-after": "0"}, json={"error": error_body})
|
||||
)
|
||||
with pytest.raises(error_type) as raised:
|
||||
await router.acompletion(model="gpt-5.6", messages=[{"role": "user", "content": "hi"}])
|
||||
|
||||
assert raised.value.status_code == 404
|
||||
assert upstream.call_count == expected_upstream_calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_generic_call_keeps_the_deployment_name_of_an_azure_ai_model_on_an_azure_openai_host(monkeypatch):
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
|
|
|
|||
|
|
@ -43,6 +43,26 @@ describe("ModelRetrySettingsTab", () => {
|
|||
expect(screen.getByText(/RateLimitError \(429\)/)).toBeInTheDocument();
|
||||
expect(screen.getByText(/ContentPolicyViolationError \(400\)/)).toBeInTheDocument();
|
||||
expect(screen.getByText(/InternalServerError \(500\)/)).toBeInTheDocument();
|
||||
expect(screen.getByText(/NotFoundError \(404\)/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should write the NotFoundError row to NotFoundErrorRetries ahead of the catch-all row", () => {
|
||||
const setGlobalRetryPolicy = vi.fn();
|
||||
render(
|
||||
<ModelRetrySettingsTab
|
||||
{...buildProps({
|
||||
selectedModelGroup: "global",
|
||||
globalRetryPolicy: { DefaultRetries: 3 },
|
||||
setGlobalRetryPolicy,
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
|
||||
const notFoundInput = screen.getByRole("spinbutton", { name: /NotFoundError \(404\) retry count$/ });
|
||||
fireEvent.change(notFoundInput, { target: { value: "2" } });
|
||||
|
||||
const updater = setGlobalRetryPolicy.mock.calls.at(-1)![0];
|
||||
expect(updater({ DefaultRetries: 3 })).toMatchObject({ DefaultRetries: 3, NotFoundErrorRetries: 2 });
|
||||
});
|
||||
|
||||
it("should use defaultRetry when globalRetryPolicy is null (global scope)", () => {
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ const retryPolicyMap: Record<string, string> = {
|
|||
"ContentPolicyViolationError (400)": "ContentPolicyViolationErrorRetries",
|
||||
"InternalServerError (500)": "InternalServerErrorRetries",
|
||||
"ServiceUnavailableError (503)": "ServiceUnavailableErrorRetries",
|
||||
"NotFoundError (404)": "NotFoundErrorRetries",
|
||||
"All other errors": "DefaultRetries",
|
||||
};
|
||||
|
||||
|
|
|
|||
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -36811,6 +36811,8 @@ export interface components {
|
|||
DefaultRetries?: number | null;
|
||||
/** Internalservererrorretries */
|
||||
InternalServerErrorRetries?: number | null;
|
||||
/** Notfounderrorretries */
|
||||
NotFoundErrorRetries?: number | null;
|
||||
/** Ratelimiterrorretries */
|
||||
RateLimitErrorRetries?: number | null;
|
||||
/** Serviceunavailableerrorretries */
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue