Skip to content

fix(proxy): restore broad is_database_connection_error; add is_database_transport_error for reconnect - #21796

Merged
ishaan-jaff merged 1 commit into
mainfrom
fix/restore-broad-db-connection-error
Feb 21, 2026
Merged

fix(proxy): restore broad is_database_connection_error; add is_database_transport_error for reconnect#21796
ishaan-jaff merged 1 commit into
mainfrom
fix/restore-broad-db-connection-error

Conversation

@ishaan-jaff

Copy link
Copy Markdown
Contributor

Relevant issues

Fixes regression from #21706 which narrowed is_database_connection_error to only match PrismaError with specific connection keywords. This broke:

  • test_delete_access_group_503_on_db_connection_error (assert 500 == 503)
  • test_handle_authentication_error_db_unavailable[prisma_error*] (10 parametrized tests)

Changes

litellm/proxy/db/exception_handler.py

  • Restored is_database_connection_error to treat any PrismaError as a DB connection error — this is what drives 503 responses and allow_requests_on_db_unavailable
  • Added is_database_transport_error with the narrow keyword-based check — only for reconnect logic where we need to distinguish "DB unreachable" from "DB returned a data error like UniqueViolationError"

litellm/proxy/auth/auth_checks.py

  • Changed reconnect call site to use is_database_transport_error instead of is_database_connection_error

tests/test_litellm/proxy/db/test_exception_handler.py

  • Renamed test_is_database_connection_error_non_connection_prisma_errorstest_is_database_transport_error_non_connection_prisma_errors
  • Updated assertion to use is_database_transport_error (data-layer errors should not trigger reconnect, but they should still trigger 503)

Pre-Submission checklist

  • Added tests — existing tests updated to cover is_database_transport_error
  • make test-unit passes locally

Type

  • Bug fix

Changes

…se_transport_error for reconnect

Any PrismaError should be treated as a DB connection error for the
allow_requests_on_db_unavailable feature and 503 responses. The narrow
keyword-based check is now in is_database_transport_error, which is
what the reconnect logic in auth_checks.py should use.

Fixes test_delete_access_group_503_on_db_connection_error and
test_handle_authentication_error_db_unavailable failures caused by
PR #21706 narrowing is_database_connection_error.
@vercel

vercel Bot commented Feb 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 21, 2026 8:01pm

Request Review

@greptile-apps

greptile-apps Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a regression from #21706 by splitting database error classification into two functions with distinct purposes:

  • is_database_connection_error (broad): treats any PrismaError as a DB connection error, used for 503 responses and allow_requests_on_db_unavailable logic. This restores the original behavior.
  • is_database_transport_error (narrow): only matches transport/connectivity failures (keyword-based), used for reconnect logic where data-layer errors like UniqueViolationError should not trigger a reconnect.

Key observations:

  • The separation is well-motivated: reconnecting on a UniqueViolationError is pointless (DB is reachable), but returning 503 for any PrismaError is correct (the request failed due to a DB issue).
  • One inconsistency remains: _db_health_watchdog_loop in litellm/proxy/utils.py:4128 still uses the broad is_database_connection_error for reconnect decisions. For consistency with the PR's design, this should also use is_database_transport_error.
  • Tests are updated but could be strengthened with a positive test confirming is_database_connection_error returns True for data-layer PrismaError subtypes (the new broadened behavior).

Confidence Score: 4/5

  • This PR is safe to merge — it correctly restores prior behavior and adds a well-scoped narrow check for reconnect logic.
  • The changes are focused and well-motivated, fixing a clear regression. The only concern is an inconsistency in utils.py where the health watchdog still uses the broad check for reconnect, but this is low-risk since SELECT 1 is unlikely to produce data-layer errors.
  • litellm/proxy/utils.py — the health watchdog reconnect check at line 4128 should be reviewed for consistency with the new is_database_transport_error pattern.

Important Files Changed

Filename Overview
litellm/proxy/db/exception_handler.py Restores broad is_database_connection_error (any PrismaError → True) for 503/allow-on-unavailable logic; adds narrow is_database_transport_error for reconnect logic. Clean separation of concerns.
litellm/proxy/auth/auth_checks.py Single-line change: reconnect call site now uses is_database_transport_error instead of is_database_connection_error, correctly avoiding reconnect attempts for data-layer errors.
tests/test_litellm/proxy/db/test_exception_handler.py Renamed test to cover is_database_transport_error for non-connection PrismaErrors. No network calls; mock-only. Could benefit from an additional test asserting the broadened is_database_connection_error returns True for data-layer PrismaErrors.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Exception raised from DB operation] --> B{Exception type?}
    B -->|httpx.ConnectError / ReadError / ReadTimeout| C[DB_CONNECTION_ERROR_TYPES]
    B -->|prisma.errors.PrismaError subclass| D{Which check?}
    B -->|ProxyException no_db_connection| E[Both checks return True]
    B -->|Other Exception| F[Both checks return False]

    C --> G["is_database_connection_error → True"]
    C --> H["is_database_transport_error → True"]

    D -->|is_database_connection_error| I["Always True — any PrismaError qualifies"]
    D -->|is_database_transport_error| J{Error message has connection keywords?}

    J -->|Yes: 'can't reach', 'timed out', etc.| K["True — trigger reconnect"]
    J -->|No: UniqueViolation, DataError, etc.| L["False — skip reconnect"]

    I --> M["Used for: 503 responses, allow_requests_on_db_unavailable"]
    K --> N["Used for: reconnect logic in auth_checks.py"]
    L --> O["DB is reachable — reconnect is pointless"]
Loading

Last reviewed commit: 6f71cd4

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps

greptile-apps Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor
Additional Comments (1)

litellm/proxy/utils.py
Inconsistent reconnect check

The health watchdog loop still uses is_database_connection_error to decide whether to reconnect. Per this PR's design, reconnect logic should use is_database_transport_error — the narrower check that distinguishes "DB unreachable" from "DB returned a data error."

With the broadened is_database_connection_error (any PrismaErrorTrue), a data-layer error from SELECT 1 (unlikely but possible, e.g. during a schema migration) would now trigger a reconnect here, which is the opposite of this PR's stated intent.

For consistency with _fetch_key_object_from_db_with_reconnect in auth_checks.py, consider switching this to is_database_transport_error:

                ) or PrismaDBExceptionHandler.is_database_transport_error(e):

@ishaan-jaff
ishaan-jaff merged commit a939fa3 into main Feb 21, 2026
32 of 35 checks passed
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…se_transport_error for reconnect (BerriAI#21796)

Any PrismaError should be treated as a DB connection error for the
allow_requests_on_db_unavailable feature and 503 responses. The narrow
keyword-based check is now in is_database_transport_error, which is
what the reconnect logic in auth_checks.py should use.

Fixes test_delete_access_group_503_on_db_connection_error and
test_handle_authentication_error_db_unavailable failures caused by
PR BerriAI#21706 narrowing is_database_connection_error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant