mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-16 23:41:43 +00:00
Merge pull request #41230 from BerriAI/litellm_client_timeout_408_skips_cooldown
fix(router): stop counting caller-set timeout 408s toward deployment cooldown
This commit is contained in:
commit
140229bc4a
6 changed files with 198 additions and 3 deletions
|
|
@ -41,6 +41,7 @@ OPTIONAL_KWARGS_KEYS: Final = (
|
|||
"azure_password",
|
||||
"azure_scope",
|
||||
"timeout",
|
||||
"client_side_timeout",
|
||||
"gcs_bucket_name",
|
||||
"bucket_name",
|
||||
"vertex_credentials",
|
||||
|
|
|
|||
|
|
@ -167,6 +167,7 @@ from litellm.router_utils.cooldown_handlers import (
|
|||
_get_cooldown_deployments,
|
||||
_set_cooldown_deployments,
|
||||
is_advisor_orchestration_failure,
|
||||
is_caller_timeout_408,
|
||||
)
|
||||
from litellm.router_utils.fallback_event_handlers import (
|
||||
AttemptedFallbackTargets,
|
||||
|
|
@ -8297,6 +8298,13 @@ class Router:
|
|||
litellm_params: Final = kwargs.get("litellm_params", {})
|
||||
_model_info: Final = litellm_params.get("model_info", {})
|
||||
|
||||
if is_caller_timeout_408(kwargs, exception_status):
|
||||
verbose_router_logger.debug(
|
||||
"Router: Exiting 'deployment_callback_on_failure' without cooldown. "
|
||||
"A timeout the caller set caused this 408, not the deployment's health."
|
||||
)
|
||||
return False
|
||||
|
||||
exception_headers: Final = litellm.litellm_core_utils.exception_mapping_utils._get_response_headers(
|
||||
original_exception=exception
|
||||
)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ Router cooldown handlers
|
|||
import asyncio
|
||||
import math
|
||||
from collections.abc import Mapping
|
||||
from datetime import datetime
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
|
||||
|
|
@ -637,3 +638,23 @@ def cast_exception_status_to_int(exception_status: str | int) -> int:
|
|||
)
|
||||
exception_status = 500
|
||||
return exception_status
|
||||
|
||||
|
||||
def is_caller_timeout_408(
|
||||
model_call_details: Mapping[str, object], exception_status: str | int, ended: datetime | None = None
|
||||
) -> bool:
|
||||
"""A 408 that arrives before the caller-set timeout could have fired came from the provider.
|
||||
|
||||
``ended`` overrides ``model_call_details["end_time"]`` for callers that run before the
|
||||
failure logger has stamped the current API call's end time."""
|
||||
if cast_exception_status_to_int(exception_status) != 408:
|
||||
return False
|
||||
litellm_params: Final = model_call_details.get("litellm_params")
|
||||
if not isinstance(litellm_params, Mapping) or not litellm_params.get("client_side_timeout"):
|
||||
return False
|
||||
timeout: Final = litellm_params.get("timeout")
|
||||
started: Final = model_call_details.get("api_call_start_time") or model_call_details.get("start_time")
|
||||
finished: Final = ended if ended is not None else model_call_details.get("end_time")
|
||||
if not isinstance(timeout, (int, float)) or not isinstance(started, datetime) or not isinstance(finished, datetime):
|
||||
return False
|
||||
return (finished - started).total_seconds() >= timeout
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@ import hashlib
|
|||
import json
|
||||
from collections.abc import Mapping, Sequence
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from enum import Enum
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
|
||||
import litellm
|
||||
|
|
@ -20,6 +22,7 @@ from litellm.router_utils.cooldown_handlers import (
|
|||
_set_cooldown_deployments, # pyright: ignore[reportPrivateUsage] - shared helper, used across router_utils
|
||||
cast_exception_status_to_int,
|
||||
is_advisor_orchestration_failure,
|
||||
is_caller_timeout_408,
|
||||
)
|
||||
from litellm.router_utils.router_callbacks.track_deployment_metrics import (
|
||||
increment_deployment_failures_for_current_minute,
|
||||
|
|
@ -36,12 +39,14 @@ else:
|
|||
# Status codes a generic API call's caller-supplied resource id can trigger on its own
|
||||
# (e.g. a nonexistent file/batch/thread id), independent of the selected deployment's health.
|
||||
_REQUEST_SCOPED_STATUS_CODES: Final = frozenset((404,))
|
||||
_NO_MODEL_CALL_DETAILS: Final[Mapping[str, object]] = MappingProxyType({})
|
||||
|
||||
|
||||
def _trigger_cooldown_for_failed_deployment(
|
||||
litellm_router: LitellmRouter,
|
||||
kwargs: Mapping[str, object],
|
||||
exception: Exception,
|
||||
model_call_details: Mapping[str, object] = _NO_MODEL_CALL_DETAILS,
|
||||
) -> None:
|
||||
"""
|
||||
Trigger cooldown for a failed fallback deployment.
|
||||
|
|
@ -80,7 +85,11 @@ def _trigger_cooldown_for_failed_deployment(
|
|||
# timeout, which litellm.Timeout reports as status 408 regardless of the deployment's
|
||||
# actual health. Left unguarded, a caller could force a 408 on every deployment in
|
||||
# the fallback chain from a single request with a near-zero timeout.
|
||||
if kwargs.get("client_side_timeout") and cast_exception_status_to_int(exception_status) == 408:
|
||||
if is_caller_timeout_408(
|
||||
model_call_details,
|
||||
exception_status,
|
||||
ended=datetime.now(), # noqa: DTZ005 # naive to match the logging pipeline's api_call_start_time
|
||||
):
|
||||
verbose_router_logger.debug(
|
||||
"Not triggering cooldown for fallback deployment: a caller-supplied "
|
||||
"x-litellm-timeout caused this 408, not deployment health."
|
||||
|
|
@ -579,6 +588,7 @@ async def run_async_fallback(
|
|||
litellm_router=litellm_router,
|
||||
kwargs=kwargs,
|
||||
exception=e,
|
||||
model_call_details=logging_obj.model_call_details,
|
||||
)
|
||||
raise error_from_fallbacks
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import json
|
||||
from datetime import datetime, timedelta
|
||||
from typing import NoReturn
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
|
@ -955,7 +956,11 @@ class TestTriggerCooldownForFailedDeployment:
|
|||
"""The proxy's x-litellm-timeout header lets a caller set an arbitrarily short
|
||||
timeout, which litellm.Timeout reports as status 408 regardless of the
|
||||
deployment's actual health. Without this guard, a caller could force a 408 on
|
||||
every deployment in the fallback chain from a single request."""
|
||||
every deployment in the fallback chain from a single request.
|
||||
|
||||
The failure logger never stamps end_time for a fallback hop (has_logged_async_failure
|
||||
is already set), so model_call_details still carries the previous hop's end_time, which
|
||||
predates this hop's api_call_start_time. The guard must not trust it."""
|
||||
mock_router = MagicMock()
|
||||
mock_router.cooldown_time = 60.0
|
||||
mock_router.get_model_info.return_value = None
|
||||
|
|
@ -973,11 +978,61 @@ class TestTriggerCooldownForFailedDeployment:
|
|||
litellm_router=mock_router,
|
||||
kwargs={"client_side_timeout": True},
|
||||
exception=exc,
|
||||
model_call_details={
|
||||
"litellm_params": {"client_side_timeout": True, "timeout": 0.5},
|
||||
"api_call_start_time": datetime.now() - timedelta(seconds=1),
|
||||
"end_time": datetime.now() - timedelta(seconds=5),
|
||||
},
|
||||
)
|
||||
|
||||
mock_set_cooldown.assert_not_called()
|
||||
mock_increment.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_still_cools_down_provider_408_before_caller_deadline(self):
|
||||
"""client_side_timeout only records that the caller configured a timeout. A 408
|
||||
that comes back before that deadline was raised by the provider itself, so it is
|
||||
a real health signal and must still cool the deployment down."""
|
||||
from litellm.router_utils.router_callbacks.track_deployment_metrics import (
|
||||
get_deployment_failures_for_current_minute,
|
||||
)
|
||||
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "fallback-model",
|
||||
"litellm_params": {"model": "openai/gpt-5.6", "api_key": "sk-fake"},
|
||||
"model_info": {"id": "fallback-deployment"},
|
||||
}
|
||||
],
|
||||
allowed_fails=0,
|
||||
cooldown_time=60,
|
||||
num_retries=0,
|
||||
)
|
||||
exc = litellm.Timeout(message="timeout", model="gpt-5.6", llm_provider="openai")
|
||||
exc.failed_deployment_id = "fallback-deployment"
|
||||
started = datetime.now()
|
||||
|
||||
_trigger_cooldown_for_failed_deployment(
|
||||
litellm_router=router,
|
||||
kwargs={"client_side_timeout": True},
|
||||
exception=exc,
|
||||
model_call_details={
|
||||
"litellm_params": {"client_side_timeout": True, "timeout": 30},
|
||||
"api_call_start_time": started,
|
||||
"end_time": started + timedelta(seconds=1),
|
||||
},
|
||||
)
|
||||
|
||||
assert (
|
||||
get_deployment_failures_for_current_minute(
|
||||
litellm_router_instance=router, deployment_id="fallback-deployment"
|
||||
)
|
||||
== 1
|
||||
)
|
||||
active = router.cooldown_cache.get_active_cooldowns(model_ids=["fallback-deployment"], parent_otel_span=None)
|
||||
assert [entry[0] for entry in active] == ["fallback-deployment"]
|
||||
|
||||
def test_still_cools_down_408_without_client_side_timeout_flag(self):
|
||||
"""The client-side-timeout guard is scoped to caller-supplied timeouts only: a
|
||||
408 that did not come from x-litellm-timeout (no client_side_timeout in kwargs)
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import os
|
|||
import sys
|
||||
import threading
|
||||
from collections.abc import Awaitable, Callable, Mapping
|
||||
from datetime import datetime
|
||||
from datetime import datetime, timedelta
|
||||
from types import SimpleNamespace
|
||||
from typing import Final, Literal
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
|
@ -8520,6 +8520,106 @@ class TestAdvisorSubCallCooldown:
|
|||
assert "dep-1" not in self._cooled_down_ids(router)
|
||||
|
||||
|
||||
class TestCallerTimeoutCooldown:
|
||||
"""A timeout the caller set (the proxy's `timeout` body field or x-litellm-timeout
|
||||
header) comes back as a 408 whatever the deployment's health, so it must neither
|
||||
count toward allowed_fails nor bench the deployment. A 408 without that marker, or
|
||||
one that arrives before the caller's deadline could have fired, is the provider's
|
||||
and keeps cooling the deployment down."""
|
||||
|
||||
def _router(self):
|
||||
return litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "slow-model",
|
||||
"litellm_params": {"model": "openai/gpt-5.6", "api_key": "sk-fake"},
|
||||
"model_info": {"id": "dep-1"},
|
||||
}
|
||||
],
|
||||
allowed_fails=0,
|
||||
cooldown_time=120,
|
||||
num_retries=0,
|
||||
)
|
||||
|
||||
def _kwargs(self, marker, started=None, ended=None):
|
||||
exception = litellm.Timeout(message="Request timed out", model="gpt-5.6", llm_provider="openai")
|
||||
return {
|
||||
"exception": exception,
|
||||
"api_call_start_time": started,
|
||||
"end_time": ended,
|
||||
"litellm_params": {"model_info": {"id": "dep-1"}, "metadata": {}, **marker},
|
||||
}
|
||||
|
||||
def _fail_count(self, router):
|
||||
from litellm.router_utils.router_callbacks.track_deployment_metrics import (
|
||||
get_deployment_failures_for_current_minute,
|
||||
)
|
||||
|
||||
return get_deployment_failures_for_current_minute(litellm_router_instance=router, deployment_id="dep-1")
|
||||
|
||||
def _cooled_down_ids(self, router):
|
||||
active = router.cooldown_cache.get_active_cooldowns(model_ids=["dep-1"], parent_otel_span=None)
|
||||
return [entry[0] for entry in active]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_caller_timeout_408_leaves_failure_counter_and_cooldown_untouched(self):
|
||||
router = self._router()
|
||||
started = datetime.now()
|
||||
ended = started + timedelta(seconds=2.05)
|
||||
kwargs = self._kwargs({"client_side_timeout": True, "timeout": 2}, started=started, ended=ended)
|
||||
assert router.deployment_callback_on_failure(kwargs, None, started, ended) is False
|
||||
assert self._fail_count(router) == 0
|
||||
assert self._cooled_down_ids(router) == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provider_timeout_408_still_counts_and_cools_down(self):
|
||||
router = self._router()
|
||||
now = datetime.now()
|
||||
assert router.deployment_callback_on_failure(self._kwargs({}), None, now, now) is True
|
||||
assert self._fail_count(router) == 1
|
||||
assert self._cooled_down_ids(router) == ["dep-1"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provider_408_before_caller_deadline_still_counts_and_cools_down(self):
|
||||
"""The marker only says the caller configured a timeout. A 408 that comes back
|
||||
well before that deadline was raised by the provider, so it is a real health
|
||||
signal and must not hide behind the caller's timeout."""
|
||||
router = self._router()
|
||||
started = datetime.now()
|
||||
ended = started + timedelta(seconds=0.4)
|
||||
kwargs = self._kwargs({"client_side_timeout": True, "timeout": 30}, started=started, ended=ended)
|
||||
assert router.deployment_callback_on_failure(kwargs, None, started, ended) is True
|
||||
assert self._fail_count(router) == 1
|
||||
assert self._cooled_down_ids(router) == ["dep-1"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_caller_timeout_marker_reaches_failure_callback_end_to_end(self):
|
||||
router = self._router()
|
||||
seen = []
|
||||
recorded = threading.Event()
|
||||
|
||||
def record(kwargs, completion_response, start_time, end_time):
|
||||
seen.append(kwargs)
|
||||
recorded.set()
|
||||
|
||||
litellm.failure_callback.append(record)
|
||||
try:
|
||||
with pytest.raises(litellm.Timeout):
|
||||
await router.acompletion(
|
||||
model="slow-model",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
mock_timeout=True,
|
||||
timeout=0.001,
|
||||
client_side_timeout=True,
|
||||
)
|
||||
assert await asyncio.to_thread(recorded.wait, 5)
|
||||
finally:
|
||||
litellm.failure_callback.remove(record)
|
||||
assert seen[0]["litellm_params"]["client_side_timeout"] is True
|
||||
assert self._fail_count(router) == 0
|
||||
assert self._cooled_down_ids(router) == []
|
||||
|
||||
|
||||
def test_stream_chunks_have_generated_content_detects_text_and_non_text():
|
||||
from litellm.router import _stream_chunks_have_generated_content
|
||||
from litellm.types.utils import (
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue