mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(router): run cooldown callback on every failed deployment attempt
Router retries/fallbacks reuse the same Logging object, so the sync_failure dedup in failure_handler skipped deployment_callback_on_failure after the first failure and left later failed deployments out of cooldown. Run the router cooldown/usage callbacks on every failed attempt while keeping observability failure callbacks deduplicated once per request. Fixes #32574
This commit is contained in:
parent
eb7e4a567a
commit
0be6e9c568
2 changed files with 79 additions and 6 deletions
|
|
@ -2753,8 +2753,7 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
|
||||
def failure_handler(self, exception, traceback_exception, start_time=None, end_time=None):
|
||||
verbose_logger.debug(f"Logging Details LiteLLM-Failure Call: {litellm.failure_callback}")
|
||||
if not self.should_run_logging(event_type="sync_failure"): # prevent double logging
|
||||
return
|
||||
should_log_failure = self.should_run_logging(event_type="sync_failure") # prevent double logging
|
||||
litellm_params = self.model_call_details.get("litellm_params", {})
|
||||
is_sync_request = self._is_sync_litellm_request(litellm_params)
|
||||
|
||||
|
|
@ -2776,9 +2775,12 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
model_call_details=(self.model_call_details if hasattr(self, "model_call_details") else {}),
|
||||
result=result,
|
||||
)
|
||||
self.has_run_logging(event_type="sync_failure")
|
||||
if should_log_failure:
|
||||
self.has_run_logging(event_type="sync_failure")
|
||||
for callback in callbacks:
|
||||
try:
|
||||
if not should_log_failure and not self._is_router_cooldown_callback(callback):
|
||||
continue
|
||||
should_run = self.should_run_callback(
|
||||
callback=callback,
|
||||
litellm_params=litellm_params,
|
||||
|
|
@ -2925,8 +2927,7 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
Implementing async callbacks, to handle asyncio event loop issues when custom integrations need to use async functions.
|
||||
"""
|
||||
await self.special_failure_handlers(exception=exception)
|
||||
if not self.should_run_logging(event_type="async_failure"): # prevent double logging
|
||||
return
|
||||
should_log_failure = self.should_run_logging(event_type="async_failure") # prevent double logging
|
||||
start_time, end_time = self._failure_handler_helper_fn(
|
||||
exception=exception,
|
||||
traceback_exception=traceback_exception,
|
||||
|
|
@ -2941,10 +2942,13 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
|
||||
result = None # result sent to all loggers, init this to None incase it's not created
|
||||
|
||||
self.has_run_logging(event_type="async_failure")
|
||||
if should_log_failure:
|
||||
self.has_run_logging(event_type="async_failure")
|
||||
for callback in callbacks:
|
||||
try:
|
||||
litellm_params = self.model_call_details.get("litellm_params", {})
|
||||
if not should_log_failure and not self._is_router_cooldown_callback(callback):
|
||||
continue
|
||||
should_run = self.should_run_callback(
|
||||
callback=callback,
|
||||
litellm_params=litellm_params,
|
||||
|
|
@ -3107,6 +3111,14 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
return cb.__class__.__name__
|
||||
return str(cb)
|
||||
|
||||
def _is_router_cooldown_callback(self, cb) -> bool:
|
||||
if not callable(cb):
|
||||
return False
|
||||
return self._get_callback_name(cb) in (
|
||||
"deployment_callback_on_failure",
|
||||
"async_deployment_callback_on_failure",
|
||||
)
|
||||
|
||||
def _is_internal_litellm_proxy_callback(self, cb) -> bool:
|
||||
"""Helper to check if a callback is internal"""
|
||||
INTERNAL_PREFIXES = [
|
||||
|
|
|
|||
|
|
@ -2871,6 +2871,67 @@ def test_failure_handler_runs_sync_callbacks_for_non_pass_through_requests(
|
|||
dummy_logger.log_failure_event.assert_called_once()
|
||||
|
||||
|
||||
def test_failure_handler_runs_router_cooldown_callback_on_every_attempt(logging_obj):
|
||||
"""Regression for #32574: router retries/fallbacks reuse the same Logging object, so
|
||||
sync_failure dedup used to skip deployment_callback_on_failure after the first failure
|
||||
and leave later failed deployments out of cooldown. The cooldown callback must fire on
|
||||
every failed attempt while observability callbacks stay deduplicated (once per request).
|
||||
"""
|
||||
cooldown_calls = []
|
||||
observability_calls = []
|
||||
|
||||
def deployment_callback_on_failure(kwargs, completion_response, start_time, end_time):
|
||||
cooldown_calls.append(kwargs.get("exception"))
|
||||
|
||||
def observability_cb(kwargs, completion_response, start_time, end_time):
|
||||
observability_calls.append(kwargs.get("exception"))
|
||||
|
||||
logging_obj.stream = False
|
||||
logging_obj.model_call_details["litellm_params"] = {}
|
||||
logging_obj.litellm_params = {}
|
||||
|
||||
with patch.object(
|
||||
logging_obj,
|
||||
"get_combined_callback_list",
|
||||
return_value=[deployment_callback_on_failure, observability_cb],
|
||||
):
|
||||
for _ in range(2):
|
||||
logging_obj.failure_handler(exception=Exception("429"), traceback_exception="")
|
||||
|
||||
assert len(cooldown_calls) == 2
|
||||
assert len(observability_calls) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_failure_handler_runs_router_cooldown_callback_on_every_attempt(logging_obj):
|
||||
"""Async counterpart of #32574: async_deployment_callback_on_failure must run on every
|
||||
failed attempt even though observability failure callbacks are deduplicated.
|
||||
"""
|
||||
cooldown_calls = []
|
||||
observability_calls = []
|
||||
|
||||
async def async_deployment_callback_on_failure(kwargs, completion_response, start_time, end_time):
|
||||
cooldown_calls.append(kwargs.get("exception"))
|
||||
|
||||
async def observability_cb(kwargs, completion_response, start_time, end_time):
|
||||
observability_calls.append(kwargs.get("exception"))
|
||||
|
||||
logging_obj.stream = False
|
||||
logging_obj.model_call_details["litellm_params"] = {}
|
||||
logging_obj.litellm_params = {}
|
||||
|
||||
with patch.object(
|
||||
logging_obj,
|
||||
"get_combined_callback_list",
|
||||
return_value=[async_deployment_callback_on_failure, observability_cb],
|
||||
):
|
||||
for _ in range(2):
|
||||
await logging_obj.async_failure_handler(exception=Exception("429"), traceback_exception="")
|
||||
|
||||
assert len(cooldown_calls) == 2
|
||||
assert len(observability_calls) == 1
|
||||
|
||||
|
||||
def test_merge_hidden_params_from_response_into_metadata_populates_metadata():
|
||||
"""Streaming completion path should mirror non-stream: metadata.hidden_params from response."""
|
||||
from litellm.litellm_core_utils.litellm_logging import Logging as LiteLLMLoggingObj
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue