From d664ca139ef6cf2a2d79a8e98bdfb4985bc3ed32 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sun, 6 Sep 2026 00:53:24 -0700 Subject: [PATCH] chore(router): suppress the retry-skip kwargs writes and correct the filter docstring The two writes that hand the skip list to the next attempt now carry a `# rebind-ok` reason, which is the sanctioned escape hatch for an unavoidable parameter mutation and matches how `log_retry` already writes into the same kwargs dict a few lines above `get_excluded_filtered_deployments`'s docstring said returning the unfiltered list would re-include the deployment that just failed. The retry skip does exactly that on purpose, so the docstring now says each caller decides what an empty result means The reliability registry cell the new e2e test claims is marked `fail_before_fix: proven`: the same config returns 400 at the merge base and 200 off a sibling deployment at the tip --- litellm/router.py | 12 ++++++------ litellm/utils.py | 10 ++++++---- tests/e2e/coverage_registry/reliability.yaml | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index e42bc2af398..85586ccab17 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -7572,12 +7572,12 @@ class Router: ## LOGGING if num_retries > 0: kwargs = self.log_retry(kwargs=kwargs, e=original_exception) - skipped_deployment_ids: Final = self._deployment_ids_to_skip_on_retry( + first_skipped_ids: Final = self._deployment_ids_to_skip_on_retry( exception=original_exception, already_skipped=kwargs.get("_retry_skipped_deployment_ids"), ) - if skipped_deployment_ids: - kwargs["_retry_skipped_deployment_ids"] = skipped_deployment_ids + if first_skipped_ids: + kwargs["_retry_skipped_deployment_ids"] = first_skipped_ids # rebind-ok: the next attempt reads it else: raise @@ -7647,12 +7647,12 @@ class Router: except Exception: raise e - retry_skipped_deployment_ids = self._deployment_ids_to_skip_on_retry( + skipped_ids = self._deployment_ids_to_skip_on_retry( exception=e, already_skipped=kwargs.get("_retry_skipped_deployment_ids"), ) - if retry_skipped_deployment_ids: - kwargs["_retry_skipped_deployment_ids"] = retry_skipped_deployment_ids + if skipped_ids: + kwargs["_retry_skipped_deployment_ids"] = skipped_ids # rebind-ok: the next attempt reads it _timeout = self._time_to_sleep_before_retry( e=e, remaining_retries=remaining_retries, diff --git a/litellm/utils.py b/litellm/utils.py index 238225eff99..f99d7e6a4b7 100644 --- a/litellm/utils.py +++ b/litellm/utils.py @@ -4919,10 +4919,12 @@ def get_excluded_filtered_deployments( across the remaining deployments in the same model group after one of them has failed. - If the filter would leave no deployments, an empty list is returned so the - caller raises its usual no-deployments error and the weighted-failover - helper falls through to the cross-group fallback path. Returning the - original unfiltered list here would re-include the just-failed deployment. + If the filter would leave no deployments, an empty list is returned and the + caller decides what that means. Weighted failover lets it raise the usual + no-deployments error and fall through to the cross-group fallback path; the + retry skip in `async_get_healthy_deployments` deliberately falls back to the + unfiltered list, so a request every deployment refused still comes back with + the provider's own error rather than a no-deployments one. """ if not excluded_deployment_ids: return healthy_deployments diff --git a/tests/e2e/coverage_registry/reliability.yaml b/tests/e2e/coverage_registry/reliability.yaml index b50551ec105..6b69677d490 100644 --- a/tests/e2e/coverage_registry/reliability.yaml +++ b/tests/e2e/coverage_registry/reliability.yaml @@ -7,7 +7,7 @@ - {id: reliability.retry.timeout.succeeds_within_retries, module: reliability, tier: P0, behavior: retry, variant: timeout, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:44", rationale: "Timeout retried per policy"} - {id: reliability.retry.429.succeeds_within_retries, module: reliability, tier: P0, behavior: retry, variant: "429", assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:46", rationale: "429 retried per RateLimitErrorRetries policy"} - {id: reliability.retry.auth.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: auth, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:42", rationale: "Transient auth glitch retry"} -- {id: reliability.retry.context_window.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: context_window, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:51", rationale: "Multi-attempt on context error"} +- {id: reliability.retry.context_window.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: context_window, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:51", fail_before_fix: proven, rationale: "A context-window 400 under BadRequestErrorRetries retries onto a sibling deployment in the same model group, instead of coming straight back as the 400 the deployment that just refused it returned"} - {id: reliability.cooldown.5xx.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "5xx", assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:40", rationale: "Deployment cools after repeated 5xx, recovers after cooldown_time"} - {id: reliability.cooldown.429.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "429", assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:69", rationale: "Cools on 429, avoids hammering exhausted provider"} - {id: reliability.cooldown.auth.trips_then_recovers, module: reliability, tier: P1, behavior: cooldown, variant: auth, assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:74", rationale: "Cools on 401 auth error"}