fix(proxy): extend WARN/ERROR exception routing to LLM hot path (re-target ship) - #41
Merged
songkuan-zheng merged 1 commit intoJun 3, 2026
Conversation
PR #39 covered management endpoints; this finishes the job for the LLM request path and shared utilities that wrap it. Same policy: known business exceptions (4xx with stable type, upstream 502/503/504) log at WARN with a single structured line; truly unexpected exceptions (bare RuntimeError, KeyError, real 500) keep ERROR + traceback. Scope: - proxy_server.py: 20 route handlers — completion, moderations, audio_transcriptions, realtime websocket (pre-call / upstream-status / internal-error), token_counter, login_v2/v3/exchange, async_data_generator + async_assistants_data_generator streaming wrappers, plus 9 reload/schedule/cancel/status admin endpoints for model_cost_map and anthropic_beta_headers. Deliberately NOT migrated: 25 startup / background-health-check / config-reload / model-list-filter helpers. Those run outside the request path, and failures there ARE the bug — they should keep ERROR + traceback. - common_request_processing.py: stream first-chunk consumer, deferred-logging hooks, post-call streaming guardrail, orphaned streaming logging, async_data_generator wrapper, and the central _handle_llm_api_exception classifier. - route_llm_request.py: shared aiohttp session recreation fallback — failure is non-fatal (we fall back to None and continue), shouldn't flood ERROR. - litellm_pre_call_utils.py: api-version query-param parse. - utils.py: post_call_failure_hook callback failures (both inner and setup), post_call_response_headers_hook, send_email. - auth/user_api_key_auth.py: websocket auth wrapper (was logging via the noisy code path). Also fixes a misuse of verbose_proxy_logger .exception() outside an except block (custom_litellm_key_header branch) — there's no current exception, so demoted to a single WARNING line that no longer prints "(NoneType: None)". - auth/auth_checks.py: get_team_membership, access_group lookup, team alias lookup, organization alias lookup. All four were ERROR + traceback for routine 404 outcomes. - common_utils/http_parsing_utils.py: invalid-JSON 400 was logged at ERROR (no traceback, but still polluted the ERROR stream on every malformed client payload); demoted to WARN. The unexpected-exception catch-all stays ERROR + traceback — that's the path that catches real server faults. E2E verification on this branch: - 7 distinct 4xx scenarios (unknown model / bad provider key / unauthenticated /schedule/* / bogus bearer / malformed JSON body / bogus login) → all WARN single line, 0 traceback. - 50 successive 401s from rotated bogus bearer tokens → 50 WARN, 0 ERROR, 0 traceback. - Pre-existing 79 unit tests in test_auth_exception_handler + test_exception_logging still pass (RuntimeError/ValueError still route to ERROR + traceback).
4 tasks
songkuan-zheng
added a commit
that referenced
this pull request
Jun 4, 2026
…a) (#56) * fix(proxy): downgrade auth failures from ERROR+traceback to WARN verbose_proxy_logger.exception() unconditionally logged every auth failure at ERROR with a full traceback. Routine 401/403 outcomes (no key, expired key, invalid key, role mismatch, budget exceeded) hit this path constantly from probes / scanners / fat-fingered keys and buried genuine errors under noise. The log line itself also lost signal in two ways: 1. When ProxyException.message was empty, str(e) was empty, so the formatted message collapsed to a bare exception type with no hint why the request was rejected. 2. The requester IP was double-encoded (string concat into message + extra dict), bloating structured-log size. 3. No correlation id — x-litellm-call-id existed but wasn't carried into the log, making it hard to grep "the log entry for that one 401 the user reported". Fix: - Demote known auth-failure exception types (ProxyException, HTTPException) to WARNING without exc_info. - Keep ERROR + full traceback for truly unexpected exceptions so real bugs are still visible. - Fall back to type(e).__name__ when str(e) is empty so there's always *something* to grep on. - Drop the IP from the format string (kept in extra only). - Add request_id, route, exception_type, http_status to extra for structured-log aggregation. Test plan: - python3 -m pytest tests/test_litellm/proxy/auth/test_auth_exception_handler.py -v → 15 passed (3 new + 12 existing) - New cases: - test_known_auth_failure_logs_at_warning_without_traceback — ProxyException → WARN, no traceback, extras populated - test_unknown_exception_logs_at_error_with_traceback — ValueError → ERROR + traceback - test_empty_exception_message_falls_back_to_type_name — ProxyException(message="") → log message contains "ProxyException" * fix(proxy): classify auth failures into specific types instead of auth_error (#29) * fix(proxy): classify auth failures into specific types instead of auth_error The wrapper in auth_exception_handler collapsed every wrapped HTTPException into the generic `auth_error` type, leaving UI clients no structured way to tell apart "your session is gone" from "you're logged in but not authorized for this endpoint" — both arrived as 401 with `type=auth_error`. The frontend had to fall back to regex- matching free-text messages, which broke on any backend wording drift. Add three specific ProxyErrorTypes and let the wrapper classify into them based on the HTTPException's status_code + detail content: auth_session_expired - token was once valid, no longer is (expired/revoked/deleted). UI redirect to login. auth_invalid_credentials - credential malformed / never existed / not in DB. UI redirect to login (same recovery: re-auth). auth_permission_denied - authenticated but lacks role/scope/ admin/master-key for this endpoint. UI must NOT redirect — toast only. Mapping rules (in priority order): - HTTP 403 -> auth_permission_denied (semantics of 403) - HTTP 401 + expired/revoked marker -> auth_session_expired - HTTP 401 + invalid/missing-credential marker -> auth_invalid_credentials - HTTP 401 + permission/role marker -> auth_permission_denied (LiteLLM uses 401 for role mismatch too) - otherwise -> auth_error (UI falls back to heuristic) The inner ProxyException pass-through is unchanged — places that already raise specific types (token_not_found_in_db, expired_key, *_access_denied, *_permission_error) keep flowing those types end- to-end. This change only fixes the HTTPException collapse case. Test plan: - python3 -m pytest tests/test_litellm/proxy/auth/test_auth_exception_handler.py -v -> 42 passed (10 new pure-function tests for _classify_auth_failure covering 403, expired markers, invalid markers, permission markers, unknown fallback, empty detail, priority ordering; 2 new integration tests asserting the wrapper's resulting ProxyException carries the classified type end-to-end) Companion PR: fix/ui-401-error-type-routing — frontend reads `error.type` and dispatches per a static map, with the current cookie + marker heuristic preserved as fallback when `type` is missing or is the generic auth_error (graceful degradation if backend is on older build). * fix(proxy): also classify bare-Exception path, not just HTTPException Live e2e revealed that the vast majority of auth failures in LiteLLM flow through the bare-Exception catch-all in the wrapper, NOT HTTPException — e.g.: Exception("No api key passed in.") Exception("LiteLLM Virtual Key expected. Received=..., expected to start with 'sk-'.") Exception("Malformed API Key passed in. Ensure Key has `Bearer` prefix.") The first iteration of _classify_auth_failure only inspected HTTPException.detail, so the catch-all kept emitting plain `auth_error` for these very-common cases, leaving the UI with no useful type to dispatch on. Generalize the classifier to fall back to str(e) when there's no detail attribute, and call it from the catch-all raise. Also expand the invalid-credentials markers list to cover the actual phrasings the auth pipeline emits. Existing tests (42) still pass — the HTTPException path tests continue to work because detail still takes precedence. * fix(proxy): add permission markers found via live e2e (proxy admin / your role) Live e2e against /key/generate with a non-admin key surfaced this real permission-denied message: "Only proxy admin can be used to generate, delete, update info for new keys/users/teams. Route=/key/generate. Your role=unknown. Your user_id=unknown" The original marker list missed "proxy admin" and "your role", so the classifier dropped this into auth_error. Add both. Tests stay green. * fix(proxy): classify management-endpoint exceptions into WARN vs ERROR (#39) * fix(proxy): classify management-endpoint exceptions into WARN vs ERROR Centralizes exception-log routing for proxy management endpoints in a new `log_proxy_exception` helper. Known business errors (HTTPException, ProxyException, BudgetExceededError, RateLimitError, AuthenticationError, NotFoundError, BadRequestError, UnsupportedParamsError, etc.) and any exception carrying a 4xx or upstream-5xx (502/503/504) status_code are logged at WARN as a single structured line with no traceback. Truly unexpected exceptions (RuntimeError, KeyError, real 500s) remain ERROR with a full traceback. Replaces every catch-all `verbose_proxy_logger.exception(...)` across the management endpoints (user, key, team, organization, customer, model, mcp, scim, ui_sso, tag, project, tool, cache_settings, team_callback, config_override, access_group, model_access_group, common_daily_activity). This is the same policy the auth pipeline adopted in commit 168feaf — extending it to the rest of the proxy control plane so client-induced 4xx errors (409 "already exists", 401 "token_not_found_in_db", etc.) stop polluting the ERROR stream with multi-line tracebacks. Tests pin the WARN vs ERROR classification for every business exception type, the 4xx/5xx status-code fallback, and the emission shape (level, exc_info presence, structured extra fields). * fix(proxy): drop traceback from handle_exception_on_proxy and bare-Exception auth path E2E verification of the management-endpoint WARN/ERROR routing surfaced two paths that were still emitting ERROR + traceback for routine 4xx outcomes: 1. ``handle_exception_on_proxy`` (litellm/proxy/utils.py) called ``verbose_proxy_logger.exception("Exception: %s")`` unconditionally. Every endpoint that uses this helper as its raise wrapper therefore re-logged 401/404/409 with a full traceback right after the WARN line the catch-all just produced. Demoted to DEBUG — the caller has already classified and logged. 2. ``_handle_authentication_error`` only treated ``ProxyException`` and ``HTTPException`` as known. The auth pipeline also raises bare ``Exception(...)`` for missing/malformed credentials (``Exception("No api key passed in.")``, ``Exception("Malformed API Key passed in. ...")``, ``Exception("LiteLLM Virtual Key expected. ...")``). These flooded the ERROR stream with tracebacks on every unauthenticated probe. Extended the known-check to consult ``_classify_auth_failure`` — if the classifier confidently routes to a specific ``auth_*`` type (auth_session_expired, auth_invalid_credentials, auth_permission_denied), we treat the exception as known and emit WARN. Only the catch-all ``auth_error`` outcome (genuinely ambiguous) keeps ERROR + traceback. Verified end-to-end against the e2e proxy: - 50 successive 401s with bogus bearer tokens → 50 WARN, 0 ERROR, 0 traceback - /user/new duplicate → single WARN, no traceback (was 2 lines incl. ERROR+stack) - nonexistent user 404 → single WARN - ValueError("totally unexpected") still ERROR + traceback (unit test pins this) Regression tests parametrize the three known bare-Exception messages that the auth pipeline actually raises so a future refactor of ``_KNOWN_AUTH_ERROR_TYPES`` can't silently re-introduce the flood. * fix(proxy): extend WARN/ERROR exception routing to LLM hot path (#41) PR #39 covered management endpoints; this finishes the job for the LLM request path and shared utilities that wrap it. Same policy: known business exceptions (4xx with stable type, upstream 502/503/504) log at WARN with a single structured line; truly unexpected exceptions (bare RuntimeError, KeyError, real 500) keep ERROR + traceback. Scope: - proxy_server.py: 20 route handlers — completion, moderations, audio_transcriptions, realtime websocket (pre-call / upstream-status / internal-error), token_counter, login_v2/v3/exchange, async_data_generator + async_assistants_data_generator streaming wrappers, plus 9 reload/schedule/cancel/status admin endpoints for model_cost_map and anthropic_beta_headers. Deliberately NOT migrated: 25 startup / background-health-check / config-reload / model-list-filter helpers. Those run outside the request path, and failures there ARE the bug — they should keep ERROR + traceback. - common_request_processing.py: stream first-chunk consumer, deferred-logging hooks, post-call streaming guardrail, orphaned streaming logging, async_data_generator wrapper, and the central _handle_llm_api_exception classifier. - route_llm_request.py: shared aiohttp session recreation fallback — failure is non-fatal (we fall back to None and continue), shouldn't flood ERROR. - litellm_pre_call_utils.py: api-version query-param parse. - utils.py: post_call_failure_hook callback failures (both inner and setup), post_call_response_headers_hook, send_email. - auth/user_api_key_auth.py: websocket auth wrapper (was logging via the noisy code path). Also fixes a misuse of verbose_proxy_logger .exception() outside an except block (custom_litellm_key_header branch) — there's no current exception, so demoted to a single WARNING line that no longer prints "(NoneType: None)". - auth/auth_checks.py: get_team_membership, access_group lookup, team alias lookup, organization alias lookup. All four were ERROR + traceback for routine 404 outcomes. - common_utils/http_parsing_utils.py: invalid-JSON 400 was logged at ERROR (no traceback, but still polluted the ERROR stream on every malformed client payload); demoted to WARN. The unexpected-exception catch-all stays ERROR + traceback — that's the path that catches real server faults. E2E verification on this branch: - 7 distinct 4xx scenarios (unknown model / bad provider key / unauthenticated /schedule/* / bogus bearer / malformed JSON body / bogus login) → all WARN single line, 0 traceback. - 50 successive 401s from rotated bogus bearer tokens → 50 WARN, 0 ERROR, 0 traceback. - Pre-existing 79 unit tests in test_auth_exception_handler + test_exception_logging still pass (RuntimeError/ValueError still route to ERROR + traceback).
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
1 participant
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.
Re-targeting #40 onto ship/v1.83.10.
PR #40 was merged into the intermediate branch
fix/proxy-exception-log-classification(which was PR #39's branch). When that branch was deleted post-merge, #40's commits never propagated toship/v1.83.10. Verified: `git grep log_proxy_exception origin/ship/v1.83.10` returns 0 hits in proxy_server.py / common_request_processing.py / route_llm_request.py, all of which #40 modified.This PR cherry-picks #40's commit (
b02586bda4) directly onto ship.(Original #40 body below.)
Summary
log_proxy_exceptionhelper, same policy: known business / 4xx / upstream-502/503/504 → WARN single structured line, no traceback. Real faults → ERROR + traceback.proxy_server.pycarefully: 20 sites are route-handler catch-alls (migrated), 25 are startup / config-reload / background-health-check helpers (deliberately left as ERROR — failures there ARE the bug).Files
proxy_server.pycommon_request_processing.py_handle_llm_api_exceptionutils.pyauth/auth_checks.pycommon_utils/http_parsing_utils.pyauth/user_api_key_auth.pyroute_llm_request.pylitellm_pre_call_utils.pyE2E verification (originally on PR #40 branch; identical content)
7 distinct 4xx scenarios — all WARN single line, 0 traceback. 50-request 401 stress test →
ERROR=0 WARNING=57 Traceback=0. Pre-existing 79 unit tests intest_auth_exception_handler.py+test_exception_logging.pystill pass.Test plan
git diff origin/ship/v1.83.10..HEAD --statmatches PR fix(proxy): extend WARN/ERROR exception routing to LLM hot path #40 exactly (8 files, +126/-135)