mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(router): keep the provider's error when the retry skip empties the group
Before excluding the deployment that just refused, the retry-skip guard asked whether another one could still answer. It asked by re-running a single routing filter, the order filter, while deployment selection also applies cooldowns, the context-window pre-call check, tag routing, and routing plugins. Any filter the guard did not replicate made it answer yes while the real pick was left with nothing. A group narrowed to one deployment by tag routing turned the provider's own 400 into a no-deployments 429. The skip now runs where every filter has already been applied, and it keeps the deployments untouched when skipping would leave none. The caller gets the provider's error either way, and a group with one eligible deployment retries in place as it did before.
This commit is contained in:
parent
cb1ec76e46
commit
ecf7e4e766
3 changed files with 150 additions and 67 deletions
|
|
@ -401,8 +401,7 @@ def _stream_chunks_have_generated_content(chunks: Sequence[ModelResponseStream])
|
|||
|
||||
_NO_SESSION_KWARGS: Final[Mapping[str, Mapping[str, object]]] = MappingProxyType({})
|
||||
_SESSION_ADAPTER: Final = TypeAdapter(Mapping[str, object])
|
||||
_EXCLUDED_DEPLOYMENT_IDS_ADAPTER: Final = TypeAdapter(tuple[str, ...])
|
||||
_TARGET_ORDER_ADAPTER: Final = TypeAdapter(int | None)
|
||||
_SKIPPED_DEPLOYMENT_IDS_ADAPTER: Final = TypeAdapter(tuple[str, ...])
|
||||
|
||||
|
||||
def _with_router_resolved_session_model(session: object, model_name: str) -> Mapping[str, Mapping[str, object]]:
|
||||
|
|
@ -7461,29 +7460,19 @@ class Router:
|
|||
)
|
||||
|
||||
@staticmethod
|
||||
def _deployment_ids_to_skip_on_retry(
|
||||
exception: Exception,
|
||||
already_skipped: object,
|
||||
healthy_deployments: list[dict], # mutable-ok: matches the routing filters' list contract
|
||||
target_order: object = None,
|
||||
) -> tuple[str, ...]:
|
||||
def _deployment_ids_to_skip_on_retry(exception: Exception, already_skipped: object) -> tuple[str, ...]:
|
||||
failed_deployment_id: Final[str | None] = getattr(exception, "failed_deployment_id", None)
|
||||
status_code: Final = getattr(exception, "status_code", None)
|
||||
if not failed_deployment_id or not isinstance(status_code, int):
|
||||
return ()
|
||||
if litellm._should_retry(status_code): # pyright: ignore[reportPrivateUsage] # as in should_retry_this_error
|
||||
return ()
|
||||
already_skipped_ids: Final = _EXCLUDED_DEPLOYMENT_IDS_ADAPTER.validate_python(already_skipped or ())
|
||||
skipped: Final = frozenset((*already_skipped_ids, failed_deployment_id))
|
||||
same_order_candidates: Final = litellm.utils.get_order_filtered_deployments(
|
||||
healthy_deployments, target_order=_TARGET_ORDER_ADAPTER.validate_python(target_order)
|
||||
)
|
||||
if not litellm.utils.get_excluded_filtered_deployments(same_order_candidates, excluded_deployment_ids=skipped):
|
||||
return ()
|
||||
already_skipped_ids: Final = _SKIPPED_DEPLOYMENT_IDS_ADAPTER.validate_python(already_skipped or ())
|
||||
skipped: Final = tuple(sorted(frozenset((*already_skipped_ids, failed_deployment_id))))
|
||||
verbose_router_logger.debug(
|
||||
"Retry skips deployments that already answered %s to this request: %s", status_code, sorted(skipped)
|
||||
"Retry skips deployments that already answered %s to this request: %s", status_code, skipped
|
||||
)
|
||||
return tuple(sorted(skipped))
|
||||
return skipped
|
||||
|
||||
@tracer.wrap()
|
||||
async def async_function_with_retries(self, *args, **kwargs):
|
||||
|
|
@ -7582,12 +7571,10 @@ class Router:
|
|||
kwargs = self.log_retry(kwargs=kwargs, e=original_exception)
|
||||
skipped_deployment_ids: Final = self._deployment_ids_to_skip_on_retry(
|
||||
exception=original_exception,
|
||||
already_skipped=kwargs.get("_excluded_deployment_ids"),
|
||||
healthy_deployments=_healthy_deployments,
|
||||
target_order=kwargs.get("_target_order"),
|
||||
already_skipped=kwargs.get("_retry_skipped_deployment_ids"),
|
||||
)
|
||||
if skipped_deployment_ids:
|
||||
kwargs["_excluded_deployment_ids"] = skipped_deployment_ids
|
||||
kwargs["_retry_skipped_deployment_ids"] = skipped_deployment_ids
|
||||
else:
|
||||
raise
|
||||
|
||||
|
|
@ -7659,12 +7646,10 @@ class Router:
|
|||
|
||||
retry_skipped_deployment_ids = self._deployment_ids_to_skip_on_retry(
|
||||
exception=e,
|
||||
already_skipped=kwargs.get("_excluded_deployment_ids"),
|
||||
healthy_deployments=_healthy_deployments,
|
||||
target_order=kwargs.get("_target_order"),
|
||||
already_skipped=kwargs.get("_retry_skipped_deployment_ids"),
|
||||
)
|
||||
if retry_skipped_deployment_ids:
|
||||
kwargs["_excluded_deployment_ids"] = retry_skipped_deployment_ids
|
||||
kwargs["_retry_skipped_deployment_ids"] = retry_skipped_deployment_ids
|
||||
_timeout = self._time_to_sleep_before_retry(
|
||||
e=e,
|
||||
remaining_retries=remaining_retries,
|
||||
|
|
@ -12508,6 +12493,19 @@ class Router:
|
|||
excluded_deployment_ids=_excluded_deployment_ids,
|
||||
)
|
||||
|
||||
## RETRY SKIP ## -> drop deployments that already refused this request with a
|
||||
## non-retryable status, unless that leaves nothing, so the caller still gets
|
||||
## the provider's own error instead of a no-deployments error.
|
||||
_retry_skipped_deployment_ids: Final = (
|
||||
request_kwargs.pop("_retry_skipped_deployment_ids", None) if request_kwargs else None
|
||||
)
|
||||
healthy_deployments = (
|
||||
litellm.utils.get_excluded_filtered_deployments(
|
||||
healthy_deployments, excluded_deployment_ids=_retry_skipped_deployment_ids
|
||||
)
|
||||
or healthy_deployments
|
||||
)
|
||||
|
||||
if len(healthy_deployments) == 0:
|
||||
exception: Final = await async_raise_no_deployment_exception(
|
||||
litellm_router_instance=self,
|
||||
|
|
@ -13413,6 +13411,17 @@ class Router:
|
|||
excluded_deployment_ids=_excluded_deployment_ids,
|
||||
)
|
||||
|
||||
## RETRY SKIP ## -> see async counterpart in async_get_healthy_deployments.
|
||||
_retry_skipped_deployment_ids: Final = (
|
||||
request_kwargs.pop("_retry_skipped_deployment_ids", None) if request_kwargs else None
|
||||
)
|
||||
healthy_deployments = (
|
||||
litellm.utils.get_excluded_filtered_deployments(
|
||||
healthy_deployments, excluded_deployment_ids=_retry_skipped_deployment_ids
|
||||
)
|
||||
or healthy_deployments
|
||||
)
|
||||
|
||||
if len(healthy_deployments) == 0:
|
||||
model_ids = self.get_model_ids(model_name=model)
|
||||
_cooldown_time = self.cooldown_cache.get_min_cooldown(
|
||||
|
|
|
|||
|
|
@ -2832,8 +2832,9 @@ class ComplexityRouter(CustomLogger):
|
|||
where the prompt never arrives as messages.
|
||||
|
||||
Probed on a COPY of request_kwargs because the owner pops routing bookkeeping off the
|
||||
dict it is handed (`_target_order`, `_excluded_deployment_ids`), and this is a
|
||||
speculative question about a model that may never be picked.
|
||||
dict it is handed (`_target_order`, `_excluded_deployment_ids`,
|
||||
`_retry_skipped_deployment_ids`), and this is a speculative question about a model
|
||||
that may never be picked.
|
||||
|
||||
Every way the owner says "nothing here can serve this" is a negative verdict: no healthy
|
||||
deployment for the group at all (BadRequestError, which ContextWindowExceededError
|
||||
|
|
|
|||
|
|
@ -13234,61 +13234,134 @@ async def test_router_retry_policy_400_retries_on_sibling_deployment(
|
|||
assert response._hidden_params["additional_headers"]["x-litellm-attempted-retries"] == 1
|
||||
|
||||
|
||||
_UPSTREAM_400 = {"message": "upstream refused this request", "type": "invalid_request_error", "code": "bad_request"}
|
||||
|
||||
|
||||
def _retry_skip_deployment(deployment_id, host, litellm_params=None, model_info=None):
|
||||
return {
|
||||
"model_name": "gpt-5.6",
|
||||
"litellm_params": {
|
||||
"model": "openai/gpt-5.6",
|
||||
"api_key": "sk-fake",
|
||||
"api_base": f"https://{host}.local/v1",
|
||||
**(litellm_params or {}),
|
||||
},
|
||||
"model_info": {"id": deployment_id, **(model_info or {})},
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"status_code,failed_deployment_id,already_skipped,healthy_deployment_ids,expected",
|
||||
"status_code,failed_deployment_id,already_skipped,expected",
|
||||
[
|
||||
(400, "rejecting", None, ["rejecting", "accepting"], ("rejecting",)),
|
||||
(403, "rejecting", None, ["rejecting", "accepting"], ("rejecting",)),
|
||||
(400, "second", ("first",), ["first", "second", "third"], ("first", "second")),
|
||||
(429, "rejecting", None, ["rejecting", "accepting"], ()),
|
||||
(503, "rejecting", None, ["rejecting", "accepting"], ()),
|
||||
(400, "rejecting", None, ["rejecting"], ()),
|
||||
(400, None, None, ["rejecting", "accepting"], ()),
|
||||
(None, "rejecting", None, ["rejecting", "accepting"], ()),
|
||||
("400", "rejecting", None, ["rejecting", "accepting"], ()),
|
||||
(400, "rejecting", None, ("rejecting",)),
|
||||
(403, "rejecting", None, ("rejecting",)),
|
||||
(400, "second", ("first",), ("first", "second")),
|
||||
(400, "first", ("first",), ("first",)),
|
||||
(429, "rejecting", None, ()),
|
||||
(503, "rejecting", None, ()),
|
||||
(408, "rejecting", None, ()),
|
||||
(400, None, None, ()),
|
||||
(None, "rejecting", None, ()),
|
||||
("400", "rejecting", None, ()),
|
||||
],
|
||||
)
|
||||
def test_router_deployment_ids_to_skip_on_retry(
|
||||
status_code, failed_deployment_id, already_skipped, healthy_deployment_ids, expected
|
||||
):
|
||||
def test_router_deployment_ids_to_skip_on_retry(status_code, failed_deployment_id, already_skipped, expected):
|
||||
exception = Exception("upstream refused this request")
|
||||
exception.status_code = status_code
|
||||
exception.failed_deployment_id = failed_deployment_id
|
||||
healthy_deployments = [{"model_info": {"id": deployment_id}} for deployment_id in healthy_deployment_ids]
|
||||
|
||||
assert (
|
||||
litellm.Router._deployment_ids_to_skip_on_retry(exception, already_skipped, healthy_deployments) == expected
|
||||
)
|
||||
assert litellm.Router._deployment_ids_to_skip_on_retry(exception, already_skipped) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"target_order,deployment_orders,expected",
|
||||
"deployment_ids,skipped,expected",
|
||||
[
|
||||
(2, {"rejecting": 2, "sibling": 1}, ()),
|
||||
(2, {"rejecting": 2, "sibling": 2}, ("rejecting",)),
|
||||
(1, {"rejecting": 1, "sibling": 2}, ()),
|
||||
(None, {"rejecting": 1, "sibling": 2}, ()),
|
||||
(None, {"rejecting": 1, "sibling": 1}, ("rejecting",)),
|
||||
(3, {"rejecting": 2, "sibling": 1}, ()),
|
||||
(["rejecting", "sibling"], ("rejecting",), ["sibling"]),
|
||||
(["rejecting"], ("rejecting",), ["rejecting"]),
|
||||
(["rejecting", "sibling"], ("rejecting", "sibling"), ["rejecting", "sibling"]),
|
||||
(["rejecting", "sibling"], (), ["rejecting", "sibling"]),
|
||||
(["rejecting", "sibling"], None, ["rejecting", "sibling"]),
|
||||
(["rejecting", "sibling"], ("absent",), ["rejecting", "sibling"]),
|
||||
],
|
||||
)
|
||||
def test_router_deployment_ids_to_skip_on_retry_honors_order_fallback_target(
|
||||
target_order, deployment_orders, expected
|
||||
):
|
||||
exception = Exception("upstream refused this request")
|
||||
exception.status_code = 400
|
||||
exception.failed_deployment_id = "rejecting"
|
||||
healthy_deployments = [
|
||||
{"model_info": {"id": deployment_id}, "litellm_params": {"order": order}}
|
||||
for deployment_id, order in deployment_orders.items()
|
||||
]
|
||||
|
||||
assert (
|
||||
litellm.Router._deployment_ids_to_skip_on_retry(
|
||||
exception, None, healthy_deployments, target_order=target_order
|
||||
)
|
||||
== expected
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_healthy_deployments_keep_the_last_candidate_a_retry_skipped(deployment_ids, skipped, expected):
|
||||
router = litellm.Router(
|
||||
model_list=[_retry_skip_deployment(deployment_id, deployment_id) for deployment_id in deployment_ids],
|
||||
disable_cooldowns=True,
|
||||
)
|
||||
request_kwargs = {"_retry_skipped_deployment_ids": skipped}
|
||||
|
||||
healthy_deployments = await router.async_get_healthy_deployments(model="gpt-5.6", request_kwargs=request_kwargs)
|
||||
|
||||
assert sorted(deployment["model_info"]["id"] for deployment in healthy_deployments) == sorted(expected)
|
||||
assert "_retry_skipped_deployment_ids" not in request_kwargs
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_retry_policy_400_keeps_upstream_error_on_order_fallback_hop(monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
_retry_skip_deployment("order1", "order1", litellm_params={"order": 1}),
|
||||
_retry_skip_deployment("order2", "order2", litellm_params={"order": 2}),
|
||||
],
|
||||
num_retries=2,
|
||||
retry_policy={"BadRequestErrorRetries": 2},
|
||||
disable_cooldowns=True,
|
||||
)
|
||||
|
||||
with respx.mock as respx_mock:
|
||||
order1 = respx_mock.post("https://order1.local/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(400, json={"error": _UPSTREAM_400})
|
||||
)
|
||||
order2 = respx_mock.post("https://order2.local/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(400, json={"error": _UPSTREAM_400})
|
||||
)
|
||||
with pytest.raises(litellm.BadRequestError) as raised:
|
||||
await router.acompletion(model="gpt-5.6", messages=[{"role": "user", "content": "hi"}])
|
||||
|
||||
assert "upstream refused this request" in str(raised.value)
|
||||
assert "No deployments available" not in str(raised.value)
|
||||
assert order1.call_count >= 1
|
||||
assert order2.call_count >= 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_retry_policy_400_keeps_upstream_error_when_tags_narrow_the_group(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
):
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
_retry_skip_deployment(
|
||||
"tagged", "tagged", litellm_params={"tags": ["free"]}, model_info={"enable_tag_filtering": True}
|
||||
),
|
||||
_retry_skip_deployment("untagged", "untagged", model_info={"enable_tag_filtering": True}),
|
||||
],
|
||||
num_retries=2,
|
||||
retry_policy={"BadRequestErrorRetries": 2},
|
||||
disable_cooldowns=True,
|
||||
)
|
||||
|
||||
with respx.mock as respx_mock:
|
||||
tagged = respx_mock.post("https://tagged.local/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(400, json={"error": _UPSTREAM_400})
|
||||
)
|
||||
untagged = respx_mock.post("https://untagged.local/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(400, json={"error": _UPSTREAM_400})
|
||||
)
|
||||
with pytest.raises(litellm.BadRequestError) as raised:
|
||||
await router.acompletion(
|
||||
model="gpt-5.6",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
metadata={"tags": ["free"]},
|
||||
)
|
||||
|
||||
assert "upstream refused this request" in str(raised.value)
|
||||
assert "No deployments available" not in str(raised.value)
|
||||
assert tagged.call_count == 3
|
||||
assert untagged.call_count == 0
|
||||
|
||||
|
||||
def _make_failure_logging_obj():
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue