mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
refactor: narrow to exact OpenAIError type to keep transient errors retryable
Address review feedback (#36044): using isinstance(e, openai.OpenAIError) was too broad — openai.APIConnectionError and openai.APITimeoutError subclass OpenAIError and carry no status_code, so they hit the 401 branch and became non-retryable AuthenticationError, which is wrong for transient failures. Narrow to type(e) is openai.OpenAIError so only the bare base class (missing-credentials, raised at client construction) maps to 401; transient subclasses keep the retryable 500 default. Add a regression test asserting APIConnectionError / APITimeoutError stay at 500. Co-authored-by: TRAE CLI <noreply@bytedance.com>
This commit is contained in:
parent
a1aa1528df
commit
11ae1d4997
2 changed files with 22 additions and 3 deletions
|
|
@ -336,9 +336,16 @@ def _get_openai_exception_status_code(e: Exception) -> int:
|
|||
status_code = getattr(e, "status_code", None)
|
||||
if status_code is not None:
|
||||
return status_code
|
||||
if isinstance(e, openai.OpenAIError):
|
||||
# openai.OpenAIError with no status_code == raised before any HTTP
|
||||
# exchange (e.g. missing api_key), which is an auth/config problem.
|
||||
# Match the *bare* ``openai.OpenAIError`` base class exactly. The
|
||||
# missing-credentials error is raised as the base class at client
|
||||
# construction (before any HTTP request), so it has no ``status_code`` and
|
||||
# should surface as a non-retryable 401 ``AuthenticationError``.
|
||||
#
|
||||
# Do NOT use ``isinstance`` here: ``openai.APIConnectionError`` and
|
||||
# ``openai.APITimeoutError`` subclass ``OpenAIError`` and also carry no
|
||||
# ``status_code``, but they are genuinely transient and must stay retryable
|
||||
# (keep the 500 default).
|
||||
if type(e) is openai.OpenAIError:
|
||||
return 401
|
||||
return 500
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ so a permanent credential/config error is retried instead of failing fast.
|
|||
It should map to 401 (AuthenticationError) instead.
|
||||
"""
|
||||
|
||||
import httpx
|
||||
import openai
|
||||
|
||||
from litellm.llms.openai.openai import _get_openai_exception_status_code
|
||||
|
|
@ -20,6 +21,17 @@ class TestGetOpenAIExceptionStatusCode:
|
|||
assert not hasattr(err, "status_code") or getattr(err, "status_code") is None
|
||||
assert _get_openai_exception_status_code(err) == 401
|
||||
|
||||
def test_transient_openai_subclasses_stay_retryable(self):
|
||||
"""APIConnectionError / APITimeoutError subclass OpenAIError and carry no
|
||||
status_code, but they are genuinely transient and must stay retryable
|
||||
(500), not be turned into a non-retryable 401. Only the bare
|
||||
OpenAIError base class (missing credentials) maps to 401."""
|
||||
conn_err = openai.APIConnectionError(request=httpx.Request("POST", "https://api.openai.com/v1"))
|
||||
timeout_err = openai.APITimeoutError(request=httpx.Request("POST", "https://api.openai.com/v1"))
|
||||
assert _get_openai_exception_status_code(conn_err) == 500
|
||||
assert _get_openai_exception_status_code(timeout_err) == 500
|
||||
|
||||
|
||||
def test_existing_status_code_is_preserved(self):
|
||||
"""An exception that already carries a status_code keeps it (real HTTP response)."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue