feat: improve error clarity for LLM provider failures#590
Merged
Conversation
Replace generic "An unexpected error occurred" messages with descriptive, user-friendly error messages when LLM operations fail. Errors like invalid API keys, wrong model names, and rate limits now surface clearly in the UI. Adds error classification utility, global FastAPI exception handlers, and frontend getApiErrorMessage() helper. Bumps version to 1.7.2.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
10 issues found across 22 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="open_notebook/ai/models.py">
<violation number="1" location="open_notebook/ai/models.py:110">
P2: get_default_model still catches ValueError only, so the new ConfigurationError will now bubble up and skip the intended log-and-return-None fallback when default models are missing/misconfigured. This changes behavior and can break callers that rely on the graceful fallback.</violation>
</file>
<file name="frontend/src/lib/utils/error-handler.ts">
<violation number="1" location="frontend/src/lib/utils/error-handler.ts:76">
P2: Avoid returning raw backend error strings from the error-to-i18n helper. When no mapping exists, return the fallback i18n key (or generic key) so callers always receive a translation key.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n mapping helpers.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/lib/hooks/use-transformations.ts">
<violation number="1" location="frontend/src/lib/hooks/use-transformations.ts:52">
P2: This uses getApiErrorMessage, which can return raw backend error strings when no mapping exists. Team guidance requires error-to-i18n helpers to always return a translation key (fallback to apiErrors.genericError) so UI always goes through i18n. Use getApiErrorKey with t(...) instead.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) [FEEDBACK_USED]</violation>
</file>
<file name="api/routers/source_chat.py">
<violation number="1" location="api/routers/source_chat.py:477">
P2: Trim the SSE error message before sending it to the client. classify_error can pass through raw exception strings, so sending user_message directly can leak long internal/provider details. Cap the message length (e.g., 200 chars) before emitting it to the client.
(Based on your team's feedback about sanitizing and truncating client-facing error messages.) [FEEDBACK_USED]</violation>
</file>
<file name="api/main.py">
<violation number="1" location="api/main.py:181">
P2: Return a sanitized/truncated error message instead of the raw exception string in API responses to avoid leaking internal details (e.g., truncate to 200 chars or map to a safe message).
(Based on your team's feedback about not returning raw backend error strings and truncating responses to 200 characters.) [FEEDBACK_USED]</violation>
</file>
<file name="api/routers/search.py">
<violation number="1" location="api/routers/search.py:109">
P2: `user_message` can include raw exception text from `classify_error()` (pass-through or default), so the SSE response may leak internal/provider details. Sanitize and truncate the client-facing message (<=200 chars) before returning it.
(Based on your team's feedback about sanitizing client error messages and truncating to 200 characters.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/lib/hooks/useNotebookChat.ts">
<violation number="1" location="frontend/src/lib/hooks/useNotebookChat.ts:87">
P2: This toast now uses getApiErrorMessage, which returns raw backend messages when no mapping exists. That bypasses the i18n key fallback and can leak unlocalized backend strings; the project guidance is to always return an i18n key (fallbackKey or apiErrors.genericError).
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) [FEEDBACK_USED]</violation>
</file>
<file name="open_notebook/utils/error_classifier.py">
<violation number="1" location="open_notebook/utils/error_classifier.py:83">
P2: Passing through raw exception text can leak internal/provider details to clients. Sanitize and truncate the message to 200 characters before returning it.
(Based on your team's feedback about returning sanitized, truncated error messages.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/lib/hooks/use-sources.ts">
<violation number="1" location="frontend/src/lib/hooks/use-sources.ts:134">
P2: getApiErrorMessage can return raw backend error strings when no mapping exists, so this toast may bypass i18n and display unlocalized backend messages. Use an i18n key fallback (e.g., getApiErrorKey + t) to avoid leaking raw backend text.
(Based on your team's feedback about avoiding raw backend error strings in error-to-i18n helpers.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/lib/hooks/useSourceChat.ts">
<violation number="1" location="frontend/src/lib/hooks/useSourceChat.ts:67">
P2: Avoid passing raw backend error strings through the i18n helper. Use the i18n key fallback so UI strings stay localized and sanitized.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| // No mapping: return backend message directly (backend is responsible for making it user-friendly) | ||
| return message; |
Contributor
There was a problem hiding this comment.
P2: Avoid returning raw backend error strings from the error-to-i18n helper. When no mapping exists, return the fallback i18n key (or generic key) so callers always receive a translation key.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n mapping helpers.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/utils/error-handler.ts, line 76:
<comment>Avoid returning raw backend error strings from the error-to-i18n helper. When no mapping exists, return the fallback i18n key (or generic key) so callers always receive a translation key.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n mapping helpers.) </comment>
<file context>
@@ -51,6 +51,31 @@ export function getApiErrorKey(errorOrMessage: unknown, fallbackKey?: string): s
+ }
+
+ // No mapping: return backend message directly (backend is responsible for making it user-friendly)
+ return message;
+}
+
</file context>
Suggested change
| return message; | |
| return fallbackKey ? t(fallbackKey) : t("apiErrors.genericError"); |
Owner
Author
There was a problem hiding this comment.
Intentional. The whole point of this PR
is to display backend-provided error
messages when no i18n mapping exists.
These are user-friendly messages crafted
by classify_error() (e.g.,
"Authentication failed. Please check your
API key in Settings -> Credentials.").
Falling back to a generic i18n key like
"An unexpected error occurred" is exactly
the problem this PR solves. The backend
is now responsible for message quality,
and the frontend correctly trusts it as a
fallback.
Contributor
There was a problem hiding this comment.
Thanks for the feedback! I've updated an existing learning with this new information.
| toast({ | ||
| title: t.common.error, | ||
| description: t(getApiErrorKey(error, t.common.error)), | ||
| description: getApiErrorMessage(error, (key) => t(key)), |
Contributor
There was a problem hiding this comment.
P2: This uses getApiErrorMessage, which can return raw backend error strings when no mapping exists. Team guidance requires error-to-i18n helpers to always return a translation key (fallback to apiErrors.genericError) so UI always goes through i18n. Use getApiErrorKey with t(...) instead.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/use-transformations.ts, line 52:
<comment>This uses getApiErrorMessage, which can return raw backend error strings when no mapping exists. Team guidance requires error-to-i18n helpers to always return a translation key (fallback to apiErrors.genericError) so UI always goes through i18n. Use getApiErrorKey with t(...) instead.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) </comment>
<file context>
@@ -49,7 +49,7 @@ export function useCreateTransformation() {
toast({
title: t.common.error,
- description: t(getApiErrorKey(error, t.common.error)),
+ description: getApiErrorMessage(error, (key) => t(key)),
variant: 'destructive',
})
</file context>
| _, user_message = classify_error(e) | ||
| logger.error(f"Error in source chat streaming: {str(e)}") | ||
| error_event = {"type": "error", "message": str(e)} | ||
| error_event = {"type": "error", "message": user_message} |
Contributor
There was a problem hiding this comment.
P2: Trim the SSE error message before sending it to the client. classify_error can pass through raw exception strings, so sending user_message directly can leak long internal/provider details. Cap the message length (e.g., 200 chars) before emitting it to the client.
(Based on your team's feedback about sanitizing and truncating client-facing error messages.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/routers/source_chat.py, line 477:
<comment>Trim the SSE error message before sending it to the client. classify_error can pass through raw exception strings, so sending user_message directly can leak long internal/provider details. Cap the message length (e.g., 200 chars) before emitting it to the client.
(Based on your team's feedback about sanitizing and truncating client-facing error messages.) </comment>
<file context>
@@ -470,8 +470,11 @@ async def stream_source_chat_response(
+ _, user_message = classify_error(e)
logger.error(f"Error in source chat streaming: {str(e)}")
- error_event = {"type": "error", "message": str(e)}
+ error_event = {"type": "error", "message": user_message}
yield f"data: {json.dumps(error_event)}\n\n"
</file context>
Suggested change
| error_event = {"type": "error", "message": user_message} | |
| error_event = {"type": "error", "message": user_message[:200]} |
| async def not_found_error_handler(request: Request, exc: NotFoundError): | ||
| return JSONResponse( | ||
| status_code=404, | ||
| content={"detail": str(exc)}, |
Contributor
There was a problem hiding this comment.
P2: Return a sanitized/truncated error message instead of the raw exception string in API responses to avoid leaking internal details (e.g., truncate to 200 chars or map to a safe message).
(Based on your team's feedback about not returning raw backend error strings and truncating responses to 200 characters.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/main.py, line 181:
<comment>Return a sanitized/truncated error message instead of the raw exception string in API responses to avoid leaking internal details (e.g., truncate to 200 chars or map to a safe message).
(Based on your team's feedback about not returning raw backend error strings and truncating responses to 200 characters.) </comment>
<file context>
@@ -154,6 +164,88 @@ async def custom_http_exception_handler(request: Request, exc: StarletteHTTPExce
+async def not_found_error_handler(request: Request, exc: NotFoundError):
+ return JSONResponse(
+ status_code=404,
+ content={"detail": str(exc)},
+ headers=_cors_headers(request),
+ )
</file context>
| _, user_message = classify_error(e) | ||
| logger.error(f"Error in ask streaming: {str(e)}") | ||
| error_data = {"type": "error", "message": str(e)} | ||
| error_data = {"type": "error", "message": user_message} |
Contributor
There was a problem hiding this comment.
P2: user_message can include raw exception text from classify_error() (pass-through or default), so the SSE response may leak internal/provider details. Sanitize and truncate the client-facing message (<=200 chars) before returning it.
(Based on your team's feedback about sanitizing client error messages and truncating to 200 characters.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/routers/search.py, line 109:
<comment>`user_message` can include raw exception text from `classify_error()` (pass-through or default), so the SSE response may leak internal/provider details. Sanitize and truncate the client-facing message (<=200 chars) before returning it.
(Based on your team's feedback about sanitizing client error messages and truncating to 200 characters.) </comment>
<file context>
@@ -102,8 +102,11 @@ async def stream_ask_response(
+ _, user_message = classify_error(e)
logger.error(f"Error in ask streaming: {str(e)}")
- error_data = {"type": "error", "message": str(e)}
+ error_data = {"type": "error", "message": user_message}
yield f"data: {json.dumps(error_data)}\n\n"
</file context>
| onError: (err: unknown) => { | ||
| const error = err as { response?: { data?: { detail?: string } }, message?: string }; | ||
| toast.error(t(getApiErrorKey(error.response?.data?.detail || error.message, 'apiErrors.failedToCreateSession'))) | ||
| toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToCreateSession')) |
Contributor
There was a problem hiding this comment.
P2: This toast now uses getApiErrorMessage, which returns raw backend messages when no mapping exists. That bypasses the i18n key fallback and can leak unlocalized backend strings; the project guidance is to always return an i18n key (fallbackKey or apiErrors.genericError).
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/useNotebookChat.ts, line 87:
<comment>This toast now uses getApiErrorMessage, which returns raw backend messages when no mapping exists. That bypasses the i18n key fallback and can leak unlocalized backend strings; the project guidance is to always return an i18n key (fallbackKey or apiErrors.genericError).
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) </comment>
<file context>
@@ -84,7 +84,7 @@ export function useNotebookChat({ notebookId, sources, notes, contextSelections
onError: (err: unknown) => {
const error = err as { response?: { data?: { detail?: string } }, message?: string };
- toast.error(t(getApiErrorKey(error.response?.data?.detail || error.message, 'apiErrors.failedToCreateSession')))
+ toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToCreateSession'))
}
})
</file context>
| toast({ | ||
| title: t.common.error, | ||
| description: t(getApiErrorKey(error, t.sources.failedToAddSource)), | ||
| description: getApiErrorMessage(error, (key) => t(key), t.sources.failedToAddSource), |
Contributor
There was a problem hiding this comment.
P2: getApiErrorMessage can return raw backend error strings when no mapping exists, so this toast may bypass i18n and display unlocalized backend messages. Use an i18n key fallback (e.g., getApiErrorKey + t) to avoid leaking raw backend text.
(Based on your team's feedback about avoiding raw backend error strings in error-to-i18n helpers.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/use-sources.ts, line 134:
<comment>getApiErrorMessage can return raw backend error strings when no mapping exists, so this toast may bypass i18n and display unlocalized backend messages. Use an i18n key fallback (e.g., getApiErrorKey + t) to avoid leaking raw backend text.
(Based on your team's feedback about avoiding raw backend error strings in error-to-i18n helpers.) </comment>
<file context>
@@ -131,7 +131,7 @@ export function useCreateSource() {
toast({
title: t.common.error,
- description: t(getApiErrorKey(error, t.sources.failedToAddSource)),
+ description: getApiErrorMessage(error, (key) => t(key), t.sources.failedToAddSource),
variant: 'destructive',
})
</file context>
| onError: (err: unknown) => { | ||
| const error = err as { response?: { data?: { detail?: string } }, message?: string }; | ||
| toast.error(t(getApiErrorKey(error.response?.data?.detail || error.message, 'apiErrors.failedToCreateSession'))) | ||
| toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToCreateSession')) |
Contributor
There was a problem hiding this comment.
P2: Avoid passing raw backend error strings through the i18n helper. Use the i18n key fallback so UI strings stay localized and sanitized.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/useSourceChat.ts, line 67:
<comment>Avoid passing raw backend error strings through the i18n helper. Use the i18n key fallback so UI strings stay localized and sanitized.
(Based on your team's feedback about not returning raw backend error strings from error-to-i18n helpers.) </comment>
<file context>
@@ -64,7 +64,7 @@ export function useSourceChat(sourceId: string) {
onError: (err: unknown) => {
const error = err as { response?: { data?: { detail?: string } }, message?: string };
- toast.error(t(getApiErrorKey(error.response?.data?.detail || error.message, 'apiErrors.failedToCreateSession')))
+ toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToCreateSession'))
}
})
</file context>
…r messages - Catch ConfigurationError alongside ValueError in get_default_model() to preserve graceful fallback after ValueError→ConfigurationError migration - Add _truncate() helper to error_classifier to cap pass-through and default error messages at 200 chars, avoiding verbose internal details
3 tasks
lfnovo
added a commit
that referenced
this pull request
Feb 18, 2026
* docs: update CLAUDE.md and user docs for error handling and podcast retry Add missing documentation for features introduced in v1.7.2 (#590) and v1.7.3 (#595): error classification system, global exception handlers, ConfigurationError, podcast failure recovery, and retry endpoint. * chore: update uv.lock
demobdev
pushed a commit
to demobdev/open-notebook
that referenced
this pull request
Feb 25, 2026
feat: improve error clarity for LLM provider failures
demobdev
pushed a commit
to demobdev/open-notebook
that referenced
this pull request
Feb 25, 2026
…vo#599) * docs: update CLAUDE.md and user docs for error handling and podcast retry Add missing documentation for features introduced in v1.7.2 (lfnovo#590) and v1.7.3 (lfnovo#595): error classification system, global exception handlers, ConfigurationError, podcast failure recovery, and retry endpoint. * chore: update uv.lock
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.
Summary
Closes #506
Changes
open_notebook/utils/error_classifier.py- classifies raw exceptions into user-friendly error typesapi/main.py,ConfigurationErrorin model provisioning, error classification in all LangGraph nodes and SSE streaming handlersgetApiErrorMessage()helper, migrated hooks to use it, fixed SSE error propagation bug in source chat and ask hooksConfigurationErroradded to retrystop_onlistsTest plan