feat(python-sdk): introduce LangfuseAuthException for auth_check failures#1650
feat(python-sdk): introduce LangfuseAuthException for auth_check failures#1650Ashut0sh-mishra wants to merge 3 commits into
Conversation
…ures
Bare Exception raises are bad practice in Python because they cannot be
cleanly caught without also masking unrelated bugs (ImportError, NameError,
SyntaxError, etc.).
This commit introduces LangfuseAuthException(Exception), a dedicated
exception class for authentication failures, and replaces the bare
raise Exception(...) in Langfuse.auth_check() with it.
Users can now guard specifically against auth failures:
from langfuse import Langfuse, LangfuseAuthException
try:
Langfuse().auth_check()
except LangfuseAuthException as e:
logger.warning('Langfuse auth failed: %s', e)
LangfuseAuthException is exported from the top-level langfuse package.
Closes langfuse/langfuse#906
Co-authored-by: nik464 <nik464@users.noreply.github.com>
Addresses greptile review on langfuse#1650: previously LangfuseAuthException was only raised on the "valid credentials, zero projects" path. Invalid API keys produce HTTP 401 / 403, which fell through to the generic `except Error` branch and re-raised the underlying Fern error -- so `except LangfuseAuthException` silently missed the most common auth-failure scenario. Now catch `UnauthorizedError` (401) and `AccessDeniedError` (403) explicitly and re-raise them as `LangfuseAuthException` with the original exception chained via `raise ... from e`. The docstring is updated so the exception's documented scope matches its name. Co-authored-by: nik464 <nik464@users.noreply.github.com>
|
Thanks @greptile-apps! You're right -- the original scope was too narrow. Updated auth_check to also catch UnauthorizedError (401) and AccessDeniedError (403) from the Fern API and re-raise them as LangfuseAuthException (with the original exception chained via raise ... from e). The docstring is updated so the documented scope matches the exception's name. Pushed. |
|
The fix looks good — One small follow-up: the class docstring in
Since the Everything else looks solid — the exception ordering, the Tip: You can customize Greptile's behavior for this repo with |
Co-authored-by: nik464 <nik464@users.noreply.github.com>
|
Good catch @greptile-apps -- updated the class docstring in |
|
The full PR diff shows this was already handled before except (UnauthorizedError, AccessDeniedError) as e:
handle_fern_exception(e)
raise LangfuseAuthException(
f"Auth check failed: {type(e).__name__} returned by Langfuse API. ..."
) from eThat block is correctly ordered before the generic |
Summary
Closes #906 (filed in langfuse/langfuse).
Bare Exception raises are bad practice in Python because catching them also masks unrelated bugs (ImportError, NameError, SyntaxError, AttributeError, etc.). This is particularly problematic in a pattern like:
\\python
try:
langfuse.auth_check()
except Exception as e:
if is_non_prod:
logger.info('langfuse not started')
# silently continues even if e is a SyntaxError in your code!
\\
Changes
aise Exception(...) in �uth_check() with
aise LangfuseAuthException(...) and updated the docstring.
After this change
Users can catch auth failures precisely without masking other bugs:
\\python
from langfuse import Langfuse, LangfuseAuthException
def initialize():
Langfuse().auth_check() # any typo here now raises the appropriate error
try:
initialize()
except LangfuseAuthException as e:
if environment.is_non_prod:
logger.info('Langfuse not started: %s', e)
# SyntaxError, NameError, etc. will still propagate and fail loudly
\\
Checklist
Greptile Summary
This PR introduces
LangfuseAuthException, a dedicated exception subclass ofException, to give callers a precise type to catch whenauth_check()fails, rather than relying on bareException. The exception is defined inlangfuse/_utils/request.py, used inauth_check(), and exported from the top-levellangfusepackage.LangfuseAuthExceptionis only raised for the "valid credentials, zero projects" code path. Invalid API keys produce an HTTP 401, which is caught by theexcept Errorbranch and re-raised as a FernError— not asLangfuseAuthException— soexcept LangfuseAuthExceptionwill silently miss the most common auth-failure scenario.LangfuseAuthExceptionacknowledges this scope ("credentials are valid but no projects are accessible"), but the exception's name and its placement in the public API imply broader coverage, which is likely to confuse users.Confidence Score: 3/5
Safe to merge if the narrow scope of
LangfuseAuthException(empty-projects path only) is the intended design; risky if the goal is a single catch-all type for every auth failure.The new exception only fires in one of three failure branches inside
auth_check(). A caller with bad API keys will receive an unwrapped FernError, notLangfuseAuthException, so users who adopt the new type expecting full coverage will still see unhandled exceptions in the most common failure scenario.langfuse/_client/client.py— specifically theexcept Errorre-raise inauth_check()at line 3192–3194, which bypasses the new exception type.Sequence Diagram
sequenceDiagram participant User participant Langfuse participant API as projects.get() User->>Langfuse: auth_check() Langfuse->>API: GET /projects alt HTTP 401 / bad credentials API-->>Langfuse: Fern Error (401) Langfuse->>Langfuse: except Error → handle_fern_exception(e) Langfuse-->>User: raises Fern Error (NOT LangfuseAuthException) end alt Valid credentials, empty project list API-->>Langfuse: "200 OK, data=[]" Langfuse-->>User: raises LangfuseAuthException ✓ end alt Valid credentials, project found API-->>Langfuse: "200 OK, data=[project]" Langfuse-->>User: returns True end alt Client not initialized (AttributeError) Langfuse->>Langfuse: except AttributeError → log warning Langfuse-->>User: returns False endPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(python-sdk): introduce LangfuseAuth..." | Re-trigger Greptile