mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(router): don't cool down parent deployment on advisor sub-call failure (#33792)
* fix(router): don't cool down parent deployment on advisor sub-call failure Advisor orchestration issues a sub-call to a different provider/credentials than the selected deployment. When that sub-call fails (e.g. a 401 because no advisor API key is configured), the exception propagates up and the router's deployment_callback_on_failure attributes it to the healthy parent deployment's model_info.id, cooling it down and rejecting unrelated callers to the same model group. Tag advisor sub-call failures on the exception and skip cooldown for them in deployment_callback_on_failure. The exception is tagged rather than wrapped so its type is preserved and retry/fallback classification and the client-facing error are unchanged. Genuine executor/deployment failures are untagged and still cool down as before. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(router): tag advisor orchestration failures via provider-neutral util Address review on LIT-4565: move the cooldown-exemption marker into litellm/router_utils/cooldown_handlers.py so the router imports it at module top instead of an in-function anthropic import, and extend the exemption to AdvisorMaxIterationsError so a max-iterations orchestration failure no longer cools down the healthy executor deployment. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: shivam <shivam@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
2a55d23731
commit
96f58fac53
5 changed files with 245 additions and 16 deletions
|
|
@ -21,6 +21,7 @@ import litellm
|
|||
import litellm.constants as _c
|
||||
from litellm.litellm_core_utils.url_utils import validate_url
|
||||
from litellm.llms.anthropic.common_utils import strip_advisor_blocks_from_messages
|
||||
from litellm.router_utils.cooldown_handlers import mark_advisor_orchestration_failure
|
||||
from litellm.types.llms.anthropic_messages.anthropic_response import (
|
||||
AnthropicMessagesResponse,
|
||||
)
|
||||
|
|
@ -124,30 +125,36 @@ class AdvisorOrchestrationHandler(MessagesInterceptor):
|
|||
|
||||
iteration += 1
|
||||
if iteration > max_uses:
|
||||
raise AdvisorMaxIterationsError(
|
||||
max_iterations_error = AdvisorMaxIterationsError(
|
||||
f"Advisor orchestration loop exceeded max_uses={max_uses}. "
|
||||
"Increase max_uses in the advisor tool definition or cap the request."
|
||||
)
|
||||
mark_advisor_orchestration_failure(max_iterations_error)
|
||||
raise max_iterations_error
|
||||
|
||||
# --- Build advisor context ---
|
||||
advisor_messages = _build_advisor_context(current_messages, executor_response, advisor_use_block)
|
||||
|
||||
# --- Advisor sub-call (always non-streaming, no tools) ---
|
||||
advisor_response: AnthropicMessagesResponse = await _call_messages_handler(
|
||||
model=advisor_model,
|
||||
messages=advisor_messages,
|
||||
tools=None,
|
||||
stream=False,
|
||||
max_tokens=max_tokens,
|
||||
custom_llm_provider=None, # let litellm resolve from model name
|
||||
metadata={
|
||||
**metadata_base,
|
||||
"advisor_sub_call": True,
|
||||
"parent_request_id": parent_request_id,
|
||||
},
|
||||
api_key=advisor_api_key,
|
||||
api_base=advisor_api_base,
|
||||
)
|
||||
try:
|
||||
advisor_response: AnthropicMessagesResponse = await _call_messages_handler(
|
||||
model=advisor_model,
|
||||
messages=advisor_messages,
|
||||
tools=None,
|
||||
stream=False,
|
||||
max_tokens=max_tokens,
|
||||
custom_llm_provider=None, # let litellm resolve from model name
|
||||
metadata={
|
||||
**metadata_base,
|
||||
"advisor_sub_call": True,
|
||||
"parent_request_id": parent_request_id,
|
||||
},
|
||||
api_key=advisor_api_key,
|
||||
api_base=advisor_api_base,
|
||||
)
|
||||
except Exception as advisor_sub_call_exception:
|
||||
mark_advisor_orchestration_failure(advisor_sub_call_exception)
|
||||
raise
|
||||
|
||||
advisor_text = _extract_response_text(advisor_response)
|
||||
|
||||
|
|
|
|||
|
|
@ -126,6 +126,7 @@ from litellm.router_utils.cooldown_handlers import (
|
|||
_async_get_cooldown_deployments_with_debug_info,
|
||||
_get_cooldown_deployments,
|
||||
_set_cooldown_deployments,
|
||||
is_advisor_orchestration_failure,
|
||||
)
|
||||
from litellm.router_utils.fallback_event_handlers import (
|
||||
_check_non_standard_fallback_format,
|
||||
|
|
@ -7005,6 +7006,14 @@ class Router:
|
|||
verbose_router_logger.debug("Router: Entering 'deployment_callback_on_failure'")
|
||||
try:
|
||||
exception = kwargs.get("exception", None)
|
||||
|
||||
if is_advisor_orchestration_failure(exception):
|
||||
verbose_router_logger.debug(
|
||||
"Router: Exiting 'deployment_callback_on_failure' without cooldown. "
|
||||
"Failure originated from advisor orchestration, not the selected deployment."
|
||||
)
|
||||
return False
|
||||
|
||||
exception_status = getattr(exception, "status_code", "")
|
||||
|
||||
# Cache litellm_params to avoid repeated dict lookups
|
||||
|
|
|
|||
|
|
@ -36,6 +36,27 @@ else:
|
|||
LitellmRouter = Any
|
||||
Span = Any
|
||||
|
||||
_ADVISOR_ORCHESTRATION_FAILURE_ATTR = "_litellm_advisor_orchestration_failure"
|
||||
|
||||
|
||||
def mark_advisor_orchestration_failure(exception: BaseException) -> None:
|
||||
"""Tag an exception as originating from advisor orchestration rather than the
|
||||
health of the router-selected deployment.
|
||||
|
||||
Advisor orchestration failures (an advisor sub-call that targets different
|
||||
provider/credentials, or the orchestration loop exceeding max_uses) are not
|
||||
caused by the selected deployment, so they must not be attributed to (and
|
||||
cool down) that otherwise-healthy deployment. The exception object is tagged
|
||||
rather than wrapped so its type is preserved and the router's retry/fallback
|
||||
classification and the client-facing error are unchanged.
|
||||
"""
|
||||
setattr(exception, _ADVISOR_ORCHESTRATION_FAILURE_ATTR, True)
|
||||
|
||||
|
||||
def is_advisor_orchestration_failure(exception: BaseException | None) -> bool:
|
||||
"""Whether ``exception`` was tagged by ``mark_advisor_orchestration_failure``."""
|
||||
return bool(getattr(exception, _ADVISOR_ORCHESTRATION_FAILURE_ATTR, False))
|
||||
|
||||
|
||||
def _is_cooldown_required(
|
||||
litellm_router_instance: LitellmRouter,
|
||||
|
|
|
|||
|
|
@ -919,3 +919,125 @@ def test_resolve_advisor_credentials_allows_real_public_ip_address():
|
|||
):
|
||||
result = _resolve_advisor_credentials(tool)
|
||||
assert result == ("sk-other", "https://8.8.8.8")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 14. Advisor orchestration failures (a sub-call failure or the loop exceeding
|
||||
# max_uses) are tagged so the router does not cool down the (healthy) parent
|
||||
# deployment; executor failures are NOT tagged (regression for LIT-4565).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_sub_call_failure_is_tagged():
|
||||
"""When the advisor sub-call raises, the exception that propagates out of
|
||||
handle() must be tagged as an advisor orchestration failure."""
|
||||
import litellm
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
from litellm.router_utils.cooldown_handlers import is_advisor_orchestration_failure
|
||||
|
||||
call_count = 0
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return _make_advisor_tool_use_response() # executor: calls advisor
|
||||
raise litellm.AuthenticationError( # advisor sub-call: 401
|
||||
message="x-api-key header is required",
|
||||
llm_provider="anthropic",
|
||||
model=model,
|
||||
)
|
||||
|
||||
with patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
with pytest.raises(litellm.AuthenticationError) as exc_info:
|
||||
await h.handle(
|
||||
model="openai/gpt-4o-mini",
|
||||
messages=MESSAGES,
|
||||
tools=[ADVISOR_TOOL],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="openai",
|
||||
)
|
||||
|
||||
assert call_count == 2
|
||||
assert is_advisor_orchestration_failure(exc_info.value) is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_max_iterations_failure_is_tagged():
|
||||
"""When the orchestration loop exceeds max_uses (the executor keeps calling
|
||||
the advisor), the AdvisorMaxIterationsError must be tagged so the healthy
|
||||
executor deployment is not cooled down."""
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorMaxIterationsError,
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
from litellm.router_utils.cooldown_handlers import is_advisor_orchestration_failure
|
||||
|
||||
advisor_tool_with_max = {**ADVISOR_TOOL, "max_uses": 1}
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
# Executor always asks for the advisor; advisor always succeeds, so the
|
||||
# loop is driven purely by max_uses rather than any deployment failure.
|
||||
if tools is None:
|
||||
return _make_text_response("Here is my advice.")
|
||||
return _make_advisor_tool_use_response()
|
||||
|
||||
with patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
with pytest.raises(AdvisorMaxIterationsError) as exc_info:
|
||||
await h.handle(
|
||||
model="openai/gpt-4o-mini",
|
||||
messages=MESSAGES,
|
||||
tools=[advisor_tool_with_max],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="openai",
|
||||
)
|
||||
|
||||
assert is_advisor_orchestration_failure(exc_info.value) is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_executor_failure_is_not_tagged():
|
||||
"""A failure of the executor call (not advisor orchestration) must NOT be
|
||||
tagged — the selected deployment genuinely failed and should cool down."""
|
||||
import litellm
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
from litellm.router_utils.cooldown_handlers import is_advisor_orchestration_failure
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
raise litellm.AuthenticationError( # executor (first call) fails
|
||||
message="invalid deployment credentials",
|
||||
llm_provider="openai",
|
||||
model=model,
|
||||
)
|
||||
|
||||
with patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
with pytest.raises(litellm.AuthenticationError) as exc_info:
|
||||
await h.handle(
|
||||
model="openai/gpt-4o-mini",
|
||||
messages=MESSAGES,
|
||||
tools=[ADVISOR_TOOL],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="openai",
|
||||
)
|
||||
|
||||
assert is_advisor_orchestration_failure(exc_info.value) is False
|
||||
|
|
|
|||
|
|
@ -5730,6 +5730,76 @@ class TestRouterRequestTimeoutPropagation:
|
|||
)
|
||||
|
||||
|
||||
class TestAdvisorSubCallCooldown:
|
||||
"""Regression for LIT-4565: an advisor orchestration failure must not cool
|
||||
down the selected (healthy) deployment, which would reject unrelated
|
||||
callers to the same model group."""
|
||||
|
||||
def _router(self):
|
||||
return litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "claude-sonnet-5",
|
||||
"litellm_params": {"model": "bedrock/us.anthropic.claude-opus-4-8"},
|
||||
"model_info": {"id": "dep-1"},
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
def _kwargs(self, exception):
|
||||
return {
|
||||
"exception": exception,
|
||||
"litellm_params": {"model_info": {"id": "dep-1"}, "metadata": {}},
|
||||
}
|
||||
|
||||
def _auth_error(self):
|
||||
return litellm.AuthenticationError(
|
||||
message="x-api-key header is required",
|
||||
llm_provider="anthropic",
|
||||
model="claude-opus-4-8",
|
||||
)
|
||||
|
||||
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_untagged_auth_error_cools_down_deployment(self):
|
||||
from datetime import datetime
|
||||
|
||||
router = self._router()
|
||||
now = datetime.now()
|
||||
assert (
|
||||
router.deployment_callback_on_failure(
|
||||
self._kwargs(self._auth_error()), None, now, now
|
||||
)
|
||||
is True
|
||||
)
|
||||
assert "dep-1" in self._cooled_down_ids(router)
|
||||
|
||||
def test_advisor_orchestration_failure_does_not_cool_down_deployment(self):
|
||||
from datetime import datetime
|
||||
|
||||
from litellm.router_utils.cooldown_handlers import (
|
||||
mark_advisor_orchestration_failure,
|
||||
)
|
||||
|
||||
router = self._router()
|
||||
exception = self._auth_error()
|
||||
mark_advisor_orchestration_failure(exception)
|
||||
|
||||
now = datetime.now()
|
||||
assert (
|
||||
router.deployment_callback_on_failure(
|
||||
self._kwargs(exception), None, now, now
|
||||
)
|
||||
is False
|
||||
)
|
||||
assert "dep-1" not in self._cooled_down_ids(router)
|
||||
|
||||
|
||||
def test_get_configured_token_limits_reads_deployment_model_info():
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue