fix(proxy): return 503 from CLI SSO endpoints when the configured Redis is unreachable - #34878
Open
ryan-crabbe-berri wants to merge 1 commit into
Open
fix(proxy): return 503 from CLI SSO endpoints when the configured Redis is unreachable#34878ryan-crabbe-berri wants to merge 1 commit into
ryan-crabbe-berri wants to merge 1 commit into
Conversation
…is is unreachable /sso/cli/start propagated a raw redis ConnectionError as a 500 because the per-IP rate-limit increment is the one cache path that re-raises. The Redis-authoritative flow write had the opposite problem: it silently swallowed the failure, stored the login session nowhere, and deferred the error to a confusing 400 at poll time. Both now fail closed with a deliberate 503 telling the operator the configured Redis cache is unreachable. RedisCache.set_cache gains an opt-in raise_on_error flag (default False, existing callers unchanged) so callers whose data lives only in Redis can hear about failed writes.
Contributor
Greptile SummaryThe PR makes CLI SSO fail closed when its configured Redis backend is unavailable.
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified. The changed paths preserve existing cache-write behavior by default while making Redis-authoritative CLI SSO operations fail explicitly before issuing an unusable login session.
|
| Filename | Overview |
|---|---|
| litellm/caching/redis_cache.py | Adds backward-compatible opt-in propagation of synchronous Redis write errors. |
| litellm/proxy/management_endpoints/ui_sso.py | Maps CLI SSO rate-limit and authoritative session-write failures to deliberate 503 responses without introducing a fallback. |
| tests/test_litellm/caching/test_redis_cache.py | Verifies that Redis writes still swallow errors by default and propagate them when explicitly requested. |
| tests/test_litellm/proxy/management_endpoints/test_ui_sso.py | Adds regression coverage for unavailable Redis during CLI SSO rate limiting and session persistence. |
Reviews (1): Last reviewed commit: "fix(proxy): return 503 from CLI SSO endp..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
How it solves it:
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Setup: throwaway Redis on :6390 (
docker run -d --rm --name qa-redis-cli-sso -p 6390:6379 redis:7-alpine), local proxy on :4155 with the coordination Redis pointed at it:Healthy Redis, fix applied (commit 8fc7946); CLI login start works as before:
Before the fix (same setup, ui_sso.py and redis_cache.py at litellm_internal_staging daf22ec), with Redis stopped (
docker stop qa-redis-cli-sso):with the proxy log showing the unhandled error:
After the fix (commit 8fc7946), same stopped Redis:
Type
🐛 Bug Fix
Changes
litellm-proxy loginruns a device-code flow whose session blob is deliberately Redis-authoritative when a coordination Redis is configured (PR #33261), so the browser callback and the CLI poll can land on different pods. When that Redis is unreachable, two things went wrong in/sso/cli/start. The per-IP rate-limit increment goes throughDualCache.increment_cache, the one sync cache path that re-raises, so the rawredis.exceptions.ConnectionErrorescaped as a 500. The login-session write had the opposite problem: syncRedisCache.set_cacheswallows errors, so the session was silently stored nowhere and the failure resurfaced later as a misleading 400 "session not found" at poll timeBoth paths in
ui_sso.pynow fail closed with a single deliberate 503 telling the operator the configured Redis cache is unreachable. Failing closed keeps the login rate limiter intact instead of failing open exactly when the shared counter is gone. There is intentionally no in-memory fallback: it could only help a single-pod deployment whose Redis is down, and in multi-pod it would replace one clear error with intermittent hard-to-debug 400sTo make the write loud without changing any existing caller,
RedisCache.set_cachegains an opt-inraise_on_errorflag (default False, so the fire-and-forget contract everywhere else is unchanged)Deployments without any Redis configured are unaffected and keep the in-memory single-pod behavior
Tests: regression tests assert
/sso/cli/startreturns 503 (not 500) when the increment raises, that a failed Redis session write raises 503 instead of silently no-op'ing, and thatset_cachestill swallows by default while raising on opt-inFinal Attestation