fix(router): allow APIConnectionError to trigger cooldown for failover - #27423
fix(router): allow APIConnectionError to trigger cooldown for failover#27423Jwrede wants to merge 2 commits into
Conversation
Greptile SummaryThis PR removes a hardcoded
Confidence Score: 4/5Safe to merge — the change is a small, well-tested deletion that restores correct failover behavior for unreachable deployments. The change is a clear bug fix with direct test coverage. No files require special attention; both changed files are straightforward.
|
| Filename | Overview |
|---|---|
| litellm/router_utils/cooldown_handlers.py | Removes the ignored_strings = ["APIConnectionError"] guard from _is_cooldown_required(), allowing connection errors to trigger deployment cooldown and failover as intended. |
| tests/test_litellm/router_utils/test_cooldown_handlers.py | New unit-test file covering _is_cooldown_required() with mock-only tests for APIConnectionError, 5xx, 429, 400, and empty-status inputs. All tests use mocks; no real network calls. |
Comments Outside Diff (1)
-
litellm/router_utils/cooldown_handlers.py, line 40-45 (link)After removing the
ignored_stringsblock,exception_stris no longer read anywhere inside_is_cooldown_required()— it is accepted but silently ignored. Callers (e.g._should_run_cooldown_logic) still pass it, so there is no breakage, but the dead parameter can mislead future contributors into thinking it affects the outcome. Consider removing the parameter from the signature (and the call sites) or adding a comment that it is reserved for future use.
Reviews (1): Last reviewed commit: "fix(router): allow APIConnectionError to..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
[Infra] Promote Internal Staging to main
Remove APIConnectionError from the ignored_strings list in _is_cooldown_required(). The blanket exclusion prevented unreachable hosts from being cooled down, so the router retried the same dead deployment on every attempt instead of failing over to healthy ones. APIConnectionError indicates the host is unreachable -- exactly the scenario where cooldown and failover should activate. Single-deployment model groups are already protected from unnecessary cooldown by the traffic-based logic in _should_cooldown_deployment(). Fixes BerriAI#27362
0dbc9a7 to
54beb8a
Compare
|
🤖 litellm-agent: This PR was marked BLOCKED 7 days ago with no subsequent activity. Closing automatically. |
Relevant issues
Fixes #27362
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
Changes
_is_cooldown_required()incooldown_handlers.pycontained a hardcodedignored_strings = ["APIConnectionError"]that suppressed cooldown for anyexception whose string representation included "APIConnectionError". This
prevented the router from cooling down unreachable hosts, so all retries hit
the same dead deployment instead of failing over to healthy ones.
The fix removes that blanket exclusion.
APIConnectionErrorindicates thehost is unreachable -- exactly the error that should trigger cooldown and
failover. Single-deployment model groups are already protected from
unnecessary cooldown by the traffic-based logic in
_should_cooldown_deployment(), which only cools down single-deploymentgroups when 100% of requests fail under high traffic.
Files changed
litellm/router_utils/cooldown_handlers.py-- removedignored_stringscheck from
_is_cooldown_required()tests/test_litellm/router_utils/test_cooldown_handlers.py-- added unittests for
_is_cooldown_required()covering APIConnectionError, 5xx, 429,4xx, and empty status scenarios