mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
Merge pull request #40455 from BerriAI/litellm_backport_1_100_x_retry_breadcrumb_growth
fix(router): backport #39491 to stable/1.100.x so retry breadcrumbs stop retaining every earlier request
This commit is contained in:
commit
e4e811ce2b
2 changed files with 119 additions and 37 deletions
|
|
@ -546,16 +546,20 @@ set_live_deployment_replay(_replay_live_router_model_cost)
|
|||
|
||||
|
||||
# Kwargs that carry no signal about the failed attempt, so log_retry drops them from a
|
||||
# breadcrumb entirely: the request payload and the router-internal walk state. Credentials are
|
||||
# handled separately by mask_credentials_in_payload, which scrubs credential-named values from
|
||||
# whatever kwargs remain rather than trying to enumerate every credential-bearing key here.
|
||||
# breadcrumb entirely: the request payload, the proxy's snapshot of the inbound request (its body
|
||||
# aliases the live request metadata, earlier breadcrumbs included, so copying it would nest every
|
||||
# breadcrumb inside the next one), and the router-internal walk state. Credentials are handled
|
||||
# separately by mask_credentials_in_payload, which scrubs credential-named values from whatever
|
||||
# kwargs remain rather than trying to enumerate every credential-bearing key here.
|
||||
RETRY_BREADCRUMB_EXCLUDED_KWARGS: Final = frozenset(
|
||||
(
|
||||
"messages",
|
||||
"original_function",
|
||||
"attempted_targets",
|
||||
"proxy_server_request",
|
||||
)
|
||||
)
|
||||
RETRY_BREADCRUMB_LIMIT: Final = 4
|
||||
|
||||
|
||||
class Router:
|
||||
|
|
@ -909,7 +913,6 @@ class Router:
|
|||
self.total_calls: defaultdict = defaultdict(int) # dict to store total calls made to each model
|
||||
self.fail_calls: defaultdict = defaultdict(int) # dict to store fail_calls made to each model
|
||||
self.success_calls: defaultdict = defaultdict(int) # dict to store success_calls made to each model
|
||||
self.previous_models: list = [] # list to store failed calls (passed in as metadata to next call)
|
||||
|
||||
# make Router.chat.completions.create compatible for openai.chat.completions.create
|
||||
default_litellm_params = default_litellm_params or {}
|
||||
|
|
@ -7826,35 +7829,31 @@ class Router:
|
|||
"""
|
||||
When a retry or fallback happens, log the details of the just failed model call - similar to Sentry breadcrumbing
|
||||
"""
|
||||
try:
|
||||
_metadata_var: Final = "litellm_metadata" if "litellm_metadata" in kwargs else "metadata"
|
||||
# Log failed model as the previous model
|
||||
previous_model: Final = {
|
||||
_metadata_var: Final = "litellm_metadata" if "litellm_metadata" in kwargs else "metadata"
|
||||
request_metadata: Final[Mapping[str, object]] = kwargs[_metadata_var]
|
||||
attempt_kwargs: Final = MappingProxyType(
|
||||
{k: v for k, v in kwargs.items() if k != _metadata_var and k not in RETRY_BREADCRUMB_EXCLUDED_KWARGS}
|
||||
)
|
||||
attempt_metadata: Final = MappingProxyType(
|
||||
{k: v for k, v in request_metadata.items() if k != "previous_models"}
|
||||
)
|
||||
previous_model: Final = MappingProxyType(
|
||||
{
|
||||
"exception_type": type(e).__name__,
|
||||
"exception_string": str(e),
|
||||
**attempt_kwargs,
|
||||
_metadata_var: attempt_metadata,
|
||||
}
|
||||
for (
|
||||
k,
|
||||
v,
|
||||
) in kwargs.items(): # log everything in kwargs except the old previous_models value - prevent nesting
|
||||
if k != _metadata_var and k not in RETRY_BREADCRUMB_EXCLUDED_KWARGS:
|
||||
previous_model[k] = v
|
||||
elif k == _metadata_var and isinstance(v, dict):
|
||||
previous_model[_metadata_var] = {}
|
||||
for metadata_k, metadata_v in kwargs[_metadata_var].items():
|
||||
if metadata_k != "previous_models":
|
||||
previous_model[k][metadata_k] = metadata_v
|
||||
|
||||
# check current size of self.previous_models, if it's larger than 3, remove the first element
|
||||
if len(self.previous_models) > 3:
|
||||
self.previous_models.pop(0)
|
||||
|
||||
scrubbed_previous_model: Final = mask_credentials_in_payload(previous_model)
|
||||
self.previous_models.append(scrubbed_previous_model)
|
||||
kwargs[_metadata_var]["previous_models"] = self.previous_models
|
||||
return kwargs
|
||||
except Exception as e:
|
||||
raise e
|
||||
)
|
||||
earlier_breadcrumbs: Final = request_metadata.get("previous_models")
|
||||
kept_breadcrumbs: Final[tuple[object, ...]] = (
|
||||
tuple(earlier_breadcrumbs)[-(RETRY_BREADCRUMB_LIMIT - 1) :]
|
||||
if isinstance(earlier_breadcrumbs, (list, tuple))
|
||||
else ()
|
||||
)
|
||||
breadcrumbs: Final = (*kept_breadcrumbs, mask_credentials_in_payload(previous_model))
|
||||
kwargs[_metadata_var]["previous_models"] = breadcrumbs # rebind-ok: the logging object already holds this dict
|
||||
return kwargs
|
||||
|
||||
def _update_usage(self, deployment_id: str, parent_otel_span: Span | None) -> int:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -8554,9 +8554,11 @@ class _FallbackAttemptRecorder(CustomLogger):
|
|||
def __init__(self):
|
||||
super().__init__()
|
||||
self.failed_targets = []
|
||||
self.breadcrumbs_per_target = []
|
||||
|
||||
async def log_failure_fallback_event(self, original_model_group, kwargs, original_exception):
|
||||
self.failed_targets.append(kwargs.get("model"))
|
||||
self.breadcrumbs_per_target.append(kwargs.get("metadata", {}).get("previous_models", ()))
|
||||
|
||||
|
||||
def _cyclic_fallback_router(num_retries=0):
|
||||
|
|
@ -8627,14 +8629,16 @@ async def test_retry_breadcrumbs_do_not_carry_the_walk_state():
|
|||
A retry has to be configured for the walk state to reach log_retry at all."""
|
||||
router = _cyclic_fallback_router(num_retries=1)
|
||||
capture = _LogCapture(logging.ERROR)
|
||||
recorder = _FallbackAttemptRecorder()
|
||||
|
||||
await _drive_cyclic_fallback(router, capture)
|
||||
await _drive_cyclic_fallback(router, capture, recorder)
|
||||
|
||||
assert router.previous_models, "no retry breadcrumbs were recorded"
|
||||
breadcrumbs = [breadcrumb for hop in recorder.breadcrumbs_per_target for breadcrumb in hop]
|
||||
assert breadcrumbs, "no retry breadcrumbs were recorded"
|
||||
assert any(
|
||||
"fallback_depth" in breadcrumb for breadcrumb in router.previous_models
|
||||
"fallback_depth" in breadcrumb for breadcrumb in breadcrumbs
|
||||
), "no breadcrumb carried router walk state, so this test cannot see the leak"
|
||||
for breadcrumb in router.previous_models:
|
||||
for breadcrumb in breadcrumbs:
|
||||
assert "attempted_targets" not in breadcrumb
|
||||
|
||||
|
||||
|
|
@ -8672,15 +8676,94 @@ async def test_retry_breadcrumbs_never_carry_a_forwarded_credential(container_ke
|
|||
container still reaches the breadcrumb, but the raw secret never does, whatever key holds it."""
|
||||
router = _cyclic_fallback_router(num_retries=1)
|
||||
capture = _LogCapture(logging.ERROR)
|
||||
metadata = {}
|
||||
|
||||
await _drive_cyclic_fallback(router, capture, **request_kwargs)
|
||||
await _drive_cyclic_fallback(router, capture, metadata=metadata, **request_kwargs)
|
||||
|
||||
assert router.previous_models, "no retry breadcrumbs were recorded"
|
||||
dumped = json.dumps(router.previous_models, default=str)
|
||||
breadcrumbs = metadata["previous_models"]
|
||||
assert breadcrumbs, "no retry breadcrumbs were recorded"
|
||||
dumped = json.dumps(breadcrumbs, default=str)
|
||||
assert container_key in dumped, "the credential-bearing kwarg never reached the breadcrumb, so this test cannot see the leak"
|
||||
assert _BREADCRUMB_CREDENTIAL_CANARY not in dumped
|
||||
|
||||
|
||||
def _always_failing_router(num_retries: int) -> litellm.Router:
|
||||
return litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "broken-group",
|
||||
"litellm_params": {
|
||||
"model": "openai/gpt-4o-mini",
|
||||
"api_key": "sk-fake",
|
||||
"mock_response": "litellm.InternalServerError",
|
||||
},
|
||||
}
|
||||
],
|
||||
num_retries=num_retries,
|
||||
)
|
||||
|
||||
|
||||
async def _fail_one_proxy_shaped_request(router: litellm.Router, request_marker: str):
|
||||
"""The proxy hands the router a metadata dict and a proxy_server_request whose body is a
|
||||
shallow copy of the request, so body["metadata"] is the very same dict the router later
|
||||
stamps previous_models onto."""
|
||||
metadata = {"request_marker": request_marker}
|
||||
with pytest.raises(litellm.InternalServerError):
|
||||
await router.acompletion(
|
||||
model="broken-group",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
metadata=metadata,
|
||||
proxy_server_request={
|
||||
"url": "http://localhost:4000/v1/chat/completions",
|
||||
"method": "POST",
|
||||
"headers": {},
|
||||
"body": {"model": "broken-group", "metadata": metadata},
|
||||
},
|
||||
)
|
||||
return metadata["previous_models"]
|
||||
|
||||
|
||||
def _nested_breadcrumb_lists(node: object) -> list[object]:
|
||||
if isinstance(node, dict):
|
||||
return [v for k, v in node.items() if k == "previous_models"] + [
|
||||
found for v in node.values() for found in _nested_breadcrumb_lists(v)
|
||||
]
|
||||
if isinstance(node, (list, tuple)):
|
||||
return [found for item in node for found in _nested_breadcrumb_lists(item)]
|
||||
return []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_retry_breadcrumbs_stay_per_request_and_flat_across_failing_requests():
|
||||
"""Every failed attempt appends a breadcrumb to metadata["previous_models"], and the proxy's
|
||||
request snapshot aliases that same metadata dict. Kept on the Router and copied wholesale,
|
||||
each breadcrumb embedded every earlier one from every earlier request, so the breadcrumb
|
||||
tree, and with it the debug repr of the kwargs, roughly doubled on each failed attempt until
|
||||
a single-worker proxy spent minutes in the redaction regex and stopped answering."""
|
||||
router = _always_failing_router(num_retries=2)
|
||||
|
||||
breadcrumbs_per_request = [
|
||||
await _fail_one_proxy_shaped_request(router, f"request-{request_number}") for request_number in range(1, 7)
|
||||
]
|
||||
|
||||
for request_number, breadcrumbs in enumerate(breadcrumbs_per_request, start=1):
|
||||
assert len(breadcrumbs) == 3, "one initial attempt plus two retries failed, each leaving one breadcrumb"
|
||||
assert {breadcrumb["metadata"]["request_marker"] for breadcrumb in breadcrumbs} == {f"request-{request_number}"}
|
||||
for breadcrumb in breadcrumbs:
|
||||
assert _nested_breadcrumb_lists(breadcrumb) == []
|
||||
assert len({len(repr(breadcrumbs)) for breadcrumbs in breadcrumbs_per_request}) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_retry_breadcrumbs_keep_only_the_last_four_attempts():
|
||||
router = _always_failing_router(num_retries=6)
|
||||
|
||||
breadcrumbs = await _fail_one_proxy_shaped_request(router, "request-1")
|
||||
|
||||
assert len(breadcrumbs) == 4
|
||||
assert [breadcrumb["metadata"]["attempted_retries"] for breadcrumb in breadcrumbs] == [3, 4, 5, 6]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fallback_traceback_stays_available_at_debug_level():
|
||||
"""Dropping the stack from the ERROR line is only safe because the fallback path still
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue