fix: /health/readiness 503 loop when DB is unreachable - #26134
Conversation
[Infra] Promote staging to main
…le_db_exception re-raising handle_db_exception() re-raises the Prisma exception inside _db_health_readiness_check's except block, which propagates out to health_readiness() and gets wrapped in a 503. The health endpoint never reached the reconnect path and the service never recovered. Fix: - Remove handle_db_exception() call from _db_health_readiness_check — that helper is for API request handlers (allow_requests_on_db_unavailable flag), not health checks - Replace raw disconnect()+connect() with attempt_db_reconnect(), which uses the proper lock, cooldown, escalation, and heavy-reconnect (recreate_prisma_client) machinery
- Remove tests that expected handle_db_exception to re-raise (old buggy behaviour) - Remove tests asserting disconnect()/connect() calls (replaced by attempt_db_reconnect) - Add regression tests covering the 503 loop fix: - transport errors never raise (ClientNotConnectedError, httpx.ConnectError, etc.) - reconnect success path returns 'connected' - reconnect failure path returns 'disconnected' without raising - non-transport errors return 'disconnected', skip reconnect
|
|
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Inactive
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 29203065 | Triggered | JSON Web Token | 26fcbc9 | tests/test_litellm/proxy/test_litellm_pre_call_utils.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Greptile SummaryThis PR fixes a Confidence Score: 5/5Safe to merge — the fix is minimal, well-understood, and all findings are P2 or lower. The root cause is clearly identified and the two-line production change is the correct minimal fix. Test coverage is comprehensive (never-raise regression, reconnect-succeeds, reconnect-fails, non-transport-error paths). No custom rules are violated. The only note is that lock_timeout_seconds is not forwarded to attempt_db_reconnect, which could theoretically block the health endpoint while a concurrent reconnect holds the lock, but this is covered by the cooldown check and is consistent with how other callers invoke the same method. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/health_endpoints/_health_endpoints.py | Removes the re-raising handle_db_exception call and replaces bare disconnect/connect with attempt_db_reconnect, correctly preventing 503 loops on DB outages. |
| tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py | Old tests that expected re-raise behavior (the bug) are replaced with regression tests that assert the function always returns a dict and calls attempt_db_reconnect appropriately. |
Sequence Diagram
sequenceDiagram
participant R as /health/readiness
participant H as _db_health_readiness_check
participant PC as PrismaClient
participant AR as attempt_db_reconnect
R->>H: call
H->>PC: health_check()
PC-->>H: raises ConnectError / ClientNotConnectedError
Note over H: BEFORE fix: handle_db_exception re-raised<br/>→ exception escaped → 503
Note over H: AFTER fix: exception caught, cache=disconnected
H->>H: is_database_transport_error(e)?
alt transport error
H->>AR: attempt_db_reconnect(reason="health_readiness_check")
AR-->>H: True / False (never raises)
alt reconnect succeeded
H->>PC: health_check()
PC-->>H: OK
H-->>R: {"status": "connected"}
else reconnect failed
H-->>R: {"status": "disconnected"}
end
else non-transport error
H-->>R: {"status": "disconnected"}
end
Reviews (1): Last reviewed commit: "test: update health readiness tests for ..." | Re-trigger Greptile
9aee0da
into
litellm_internal_staging
* fix: /health/readiness returns 503 when DB is unreachable due to handle_db_exception re-raising handle_db_exception() re-raises the Prisma exception inside _db_health_readiness_check's except block, which propagates out to health_readiness() and gets wrapped in a 503. The health endpoint never reached the reconnect path and the service never recovered. Fix: - Remove handle_db_exception() call from _db_health_readiness_check — that helper is for API request handlers (allow_requests_on_db_unavailable flag), not health checks - Replace raw disconnect()+connect() with attempt_db_reconnect(), which uses the proper lock, cooldown, escalation, and heavy-reconnect (recreate_prisma_client) machinery * test: update health readiness tests for handle_db_exception removal - Remove tests that expected handle_db_exception to re-raise (old buggy behaviour) - Remove tests asserting disconnect()/connect() calls (replaced by attempt_db_reconnect) - Add regression tests covering the 503 loop fix: - transport errors never raise (ClientNotConnectedError, httpx.ConnectError, etc.) - reconnect success path returns 'connected' - reconnect failure path returns 'disconnected' without raising - non-transport errors return 'disconnected', skip reconnect --------- Co-authored-by: yuneng-jiang <yuneng@berri.ai>
3 participants
Relevant issues
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🐛 Bug Fix
Changes
/health/readinesswas returning 503Service Unhealthy (All connection attempts failed)/Service Unhealthy (Client is not connected to the query engine...)every 1-2 hours with no recovery.Root cause:
_db_health_readiness_check()catches the Prisma exception, then callshandle_db_exception(e)which re-raises it (unlessallow_requests_on_db_unavailable=True, which is almost never set). The re-raise escapes the function, hits the outer handler inhealth_readiness(), and becomes a 503. The reconnect code below it was never reached.Two changes in
_db_health_readiness_check:Remove
handle_db_exception(e)— that helper enforces theallow_requests_on_db_unavailableflag for API request handlers. The health endpoint just needs to report DB state.Replace raw
disconnect()+connect()inline reconnect withattempt_db_reconnect()— the inline version bypassed the reconnect lock, cooldown, escalation, and heavy-reconnect (recreate_prisma_client) path.Manual Test Results
Tested against both error types seen in prod:
Before fix — exception escapes, would become 503:
After fix — returns disconnected status, no 503:
Recovery path — reconnect succeeds, next call returns connected:
Unit tests in
tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py— 24 passed (3 pre-existing errors unrelated to this change, requireprisma generate).