mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix: deep copy kwargs in run_async_fallback to prevent mutation across fallback attempts
When a provider handler (e.g. Bedrock's converse_handler) mutates kwargs via .pop() calls during request processing and the request fails, the fallback loop reuses the same kwargs dict. This leaves subsequent fallback providers with missing keys — for example, tool_choice present but tools removed — causing Azure OpenAI to reject the request with HTTP 400. Fix: use safe_deep_copy(kwargs) before each fallback attempt so every provider receives the original, unmodified parameters. Fixes #24764
This commit is contained in:
parent
360c4f47a9
commit
dc5dcfd7f5
2 changed files with 139 additions and 10 deletions
|
|
@ -4,6 +4,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union
|
|||
import litellm
|
||||
from litellm._logging import verbose_router_logger
|
||||
from litellm.integrations.custom_logger import CustomLogger
|
||||
from litellm.litellm_core_utils.core_helpers import safe_deep_copy
|
||||
from litellm.router_utils.add_retry_fallback_headers import (
|
||||
add_fallback_headers_to_response,
|
||||
)
|
||||
|
|
@ -123,21 +124,31 @@ async def run_async_fallback(
|
|||
if mg == original_model_group:
|
||||
continue
|
||||
try:
|
||||
# Deep copy kwargs so each fallback attempt starts from the
|
||||
# original, unmodified parameters. Without this, provider-
|
||||
# specific handlers (e.g. Bedrock's converse_handler) can
|
||||
# mutate kwargs via .pop() calls, leaving subsequent fallback
|
||||
# providers with missing keys (e.g. `tool_choice` present but
|
||||
# `tools` removed). See https://github.com/BerriAI/litellm/issues/24764
|
||||
fallback_kwargs = safe_deep_copy(kwargs)
|
||||
|
||||
# LOGGING
|
||||
kwargs = litellm_router.log_retry(kwargs=kwargs, e=original_exception)
|
||||
fallback_kwargs = litellm_router.log_retry(
|
||||
kwargs=fallback_kwargs, e=original_exception
|
||||
)
|
||||
verbose_router_logger.info(f"Falling back to model_group = {mg}")
|
||||
if isinstance(mg, str):
|
||||
kwargs["model"] = mg
|
||||
fallback_kwargs["model"] = mg
|
||||
elif isinstance(mg, dict):
|
||||
kwargs.update(mg)
|
||||
kwargs.setdefault("metadata", {}).update(
|
||||
{"model_group": kwargs.get("model", None)}
|
||||
fallback_kwargs.update(mg)
|
||||
fallback_kwargs.setdefault("metadata", {}).update(
|
||||
{"model_group": fallback_kwargs.get("model", None)}
|
||||
) # update model_group used, if fallbacks are done
|
||||
fallback_depth = fallback_depth + 1
|
||||
kwargs["fallback_depth"] = fallback_depth
|
||||
kwargs["max_fallbacks"] = max_fallbacks
|
||||
fallback_kwargs["fallback_depth"] = fallback_depth
|
||||
fallback_kwargs["max_fallbacks"] = max_fallbacks
|
||||
response = await litellm_router.async_function_with_fallbacks(
|
||||
*args, **kwargs
|
||||
*args, **fallback_kwargs
|
||||
)
|
||||
verbose_router_logger.info("Successful fallback b/w models.")
|
||||
response = add_fallback_headers_to_response(
|
||||
|
|
@ -147,7 +158,7 @@ async def run_async_fallback(
|
|||
# callback for successfull_fallback_event():
|
||||
await log_success_fallback_event(
|
||||
original_model_group=original_model_group,
|
||||
kwargs=kwargs,
|
||||
kwargs=fallback_kwargs,
|
||||
original_exception=original_exception,
|
||||
)
|
||||
return response
|
||||
|
|
@ -155,7 +166,7 @@ async def run_async_fallback(
|
|||
error_from_fallbacks = e
|
||||
await log_failure_fallback_event(
|
||||
original_model_group=original_model_group,
|
||||
kwargs=kwargs,
|
||||
kwargs=fallback_kwargs,
|
||||
original_exception=original_exception,
|
||||
)
|
||||
raise error_from_fallbacks
|
||||
|
|
|
|||
|
|
@ -316,3 +316,121 @@ async def test_multiple_fallbacks(function_name):
|
|||
result._hidden_params["api_base"]
|
||||
== "https://exampleopenaiendpoint-production.up.railway.app/"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fallback_kwargs_not_mutated():
|
||||
"""
|
||||
Verify that each fallback attempt receives a fresh copy of kwargs.
|
||||
|
||||
Regression test for https://github.com/BerriAI/litellm/issues/24764:
|
||||
When a provider handler mutates kwargs (e.g. Bedrock pops `tools`),
|
||||
subsequent fallback attempts should still see the original parameters.
|
||||
"""
|
||||
call_received_kwargs: List[Dict[str, Any]] = []
|
||||
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "primary-model",
|
||||
"litellm_params": {
|
||||
"model": "openai/primary",
|
||||
"api_key": "fake-key",
|
||||
"api_base": "http://localhost:1/",
|
||||
},
|
||||
},
|
||||
{
|
||||
"model_name": "fallback-a",
|
||||
"litellm_params": {
|
||||
"model": "openai/fallback-a",
|
||||
"api_key": "fake-key",
|
||||
"api_base": "http://localhost:2/",
|
||||
},
|
||||
},
|
||||
{
|
||||
"model_name": "fallback-b",
|
||||
"litellm_params": {
|
||||
"model": "openai/fallback-b",
|
||||
"api_key": "fake-key",
|
||||
"api_base": "http://localhost:3/",
|
||||
},
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
original_fn = router.async_function_with_fallbacks
|
||||
|
||||
async def mock_async_function_with_fallbacks(*args, **kwargs):
|
||||
"""
|
||||
Capture kwargs snapshot, then simulate the first provider mutating
|
||||
them (like Bedrock popping 'tools') before raising an error.
|
||||
"""
|
||||
import copy
|
||||
|
||||
call_received_kwargs.append(copy.deepcopy(kwargs))
|
||||
|
||||
# Simulate a provider handler mutating kwargs — pop 'tools'
|
||||
kwargs.pop("tools", None)
|
||||
kwargs.pop("stream", None)
|
||||
|
||||
raise litellm.exceptions.ServiceUnavailableError(
|
||||
message="simulated timeout",
|
||||
model="test",
|
||||
llm_provider="openai",
|
||||
)
|
||||
|
||||
router.async_function_with_fallbacks = mock_async_function_with_fallbacks
|
||||
|
||||
original_tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "classify",
|
||||
"parameters": {"type": "object", "properties": {}},
|
||||
},
|
||||
}
|
||||
]
|
||||
original_tool_choice = {
|
||||
"type": "function",
|
||||
"function": {"name": "classify"},
|
||||
}
|
||||
|
||||
request_kwargs: Dict[str, Any] = {
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"tools": original_tools,
|
||||
"tool_choice": original_tool_choice,
|
||||
"stream": True,
|
||||
"metadata": {},
|
||||
}
|
||||
|
||||
with pytest.raises(Exception):
|
||||
await run_async_fallback(
|
||||
litellm_router=router,
|
||||
original_function=router._acompletion,
|
||||
num_retries=0,
|
||||
fallback_model_group=["fallback-a", "fallback-b"],
|
||||
original_model_group="primary-model",
|
||||
original_exception=Exception("primary failed"),
|
||||
max_fallbacks=5,
|
||||
fallback_depth=0,
|
||||
**request_kwargs,
|
||||
)
|
||||
|
||||
# Both fallback attempts should have received 'tools' and 'stream'
|
||||
assert len(call_received_kwargs) == 2, (
|
||||
f"Expected 2 fallback attempts, got {len(call_received_kwargs)}"
|
||||
)
|
||||
|
||||
for i, received in enumerate(call_received_kwargs):
|
||||
assert "tools" in received, (
|
||||
f"Fallback attempt {i} missing 'tools' — kwargs were mutated by previous attempt"
|
||||
)
|
||||
assert "stream" in received, (
|
||||
f"Fallback attempt {i} missing 'stream' — kwargs were mutated by previous attempt"
|
||||
)
|
||||
assert "tool_choice" in received, (
|
||||
f"Fallback attempt {i} missing 'tool_choice' — kwargs were mutated by previous attempt"
|
||||
)
|
||||
assert received["tools"] == original_tools, (
|
||||
f"Fallback attempt {i} has modified 'tools' value"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue