From 5da497f4acff18a13161c41359e962c2d92598dd Mon Sep 17 00:00:00 2001 From: yassin Date: Tue, 15 Sep 2026 20:19:20 +0000 Subject: [PATCH] fix(router): only exempt 408s that arrive after the caller's timeout from cooldown client_side_timeout records that the caller configured a timeout, not that the timeout fired. A 408 the provider returns before that deadline is a deployment failure and must still count toward cooldown. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/router.py | 2 +- litellm/router_utils/cooldown_handlers.py | 16 +++++- .../router_utils/fallback_event_handlers.py | 6 ++- .../test_fallback_event_handlers.py | 50 +++++++++++++++++++ tests/test_litellm/test_router.py | 30 ++++++++--- 5 files changed, 94 insertions(+), 10 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index fc698ecb4b9..2c20e810839 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -8298,7 +8298,7 @@ class Router: litellm_params: Final = kwargs.get("litellm_params", {}) _model_info: Final = litellm_params.get("model_info", {}) - if is_caller_timeout_408(litellm_params.get("client_side_timeout"), exception_status): + 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." diff --git a/litellm/router_utils/cooldown_handlers.py b/litellm/router_utils/cooldown_handlers.py index e21567684eb..bef07c68e9e 100644 --- a/litellm/router_utils/cooldown_handlers.py +++ b/litellm/router_utils/cooldown_handlers.py @@ -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 @@ -639,5 +640,16 @@ def cast_exception_status_to_int(exception_status: str | int) -> int: return exception_status -def is_caller_timeout_408(client_side_timeout: object, exception_status: str | int) -> bool: - return bool(client_side_timeout) and cast_exception_status_to_int(exception_status) == 408 +def is_caller_timeout_408(model_call_details: Mapping[str, object], exception_status: str | int) -> bool: + """A 408 that arrives before the caller-set timeout could have fired came from the provider.""" + 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") + ended: Final = model_call_details.get("end_time") + if not isinstance(timeout, (int, float)) or not isinstance(started, datetime) or not isinstance(ended, datetime): + return False + return (ended - started).total_seconds() >= timeout diff --git a/litellm/router_utils/fallback_event_handlers.py b/litellm/router_utils/fallback_event_handlers.py index 527a6b484e0..eeea9b9faf8 100644 --- a/litellm/router_utils/fallback_event_handlers.py +++ b/litellm/router_utils/fallback_event_handlers.py @@ -3,6 +3,7 @@ import json from collections.abc import Mapping, Sequence from dataclasses import dataclass from enum import Enum +from types import MappingProxyType from typing import TYPE_CHECKING, Any, Final import litellm @@ -37,12 +38,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. @@ -81,7 +84,7 @@ 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 is_caller_timeout_408(kwargs.get("client_side_timeout"), exception_status): + if is_caller_timeout_408(model_call_details, exception_status): verbose_router_logger.debug( "Not triggering cooldown for fallback deployment: a caller-supplied " "x-litellm-timeout caused this 408, not deployment health." @@ -580,6 +583,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 diff --git a/tests/test_litellm/router_utils/test_fallback_event_handlers.py b/tests/test_litellm/router_utils/test_fallback_event_handlers.py index a4965c49f07..15db22dc758 100644 --- a/tests/test_litellm/router_utils/test_fallback_event_handlers.py +++ b/tests/test_litellm/router_utils/test_fallback_event_handlers.py @@ -1,4 +1,5 @@ import json +from datetime import datetime, timedelta from typing import NoReturn from unittest.mock import MagicMock, patch @@ -973,11 +974,60 @@ 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(), + }, ) mock_set_cooldown.assert_not_called() mock_increment.assert_not_called() + 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) diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 53eb91e6c63..fe01df04351 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -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 @@ -8523,8 +8523,9 @@ class TestAdvisorSubCallCooldown: 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 is - the provider's and keeps cooling the deployment down.""" + 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( @@ -8540,10 +8541,12 @@ class TestCallerTimeoutCooldown: num_retries=0, ) - def _kwargs(self, marker): + 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}, } @@ -8561,8 +8564,10 @@ class TestCallerTimeoutCooldown: @pytest.mark.asyncio async def test_caller_timeout_408_leaves_failure_counter_and_cooldown_untouched(self): router = self._router() - now = datetime.now() - assert router.deployment_callback_on_failure(self._kwargs({"client_side_timeout": True}), None, now, now) is False + 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) == [] @@ -8574,6 +8579,19 @@ class TestCallerTimeoutCooldown: 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()