Fix internal server error display in frontend #7699
Conversation
This commit fixes an issue where internal server error messages were displaying the translation key (e.g., STATUS) instead of the actual translated error message. Changes: 1. Modified ExpandableMessage component to properly handle translation keys in message content 2. Added proper detection of translation keys 3. Hides the expand/collapse arrow when there are no details to show 4. Added test for translation key handling
xingyaoww
force-pushed
the
fix-internal-server-error-display
branch
from
8e0ff07 to
e422a3d
Compare
This commit simplifies the test for translation key handling to avoid issues with mocking the useTranslation hook.
| if (key === "STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR") { | ||
| return "The request failed with an internal server error."; | ||
| } | ||
| if (key === "STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR_MESSAGE") { | ||
| return "Something went wrong with the AI provider. This could be due to server overload or temporary issues. Please try again later."; | ||
| } |
There was a problem hiding this comment.
You should just modify the translations for this!
There was a problem hiding this comment.
Oh this is a test file...
This file really shouldn't need to be edited I don't think...
| @@ -300,6 +300,7 @@ export enum I18nKey { | |||
| STATUS$ERROR_LLM_AUTHENTICATION = "STATUS$ERROR_LLM_AUTHENTICATION", | |||
| STATUS$ERROR_LLM_SERVICE_UNAVAILABLE = "STATUS$ERROR_LLM_SERVICE_UNAVAILABLE", | |||
| STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR = "STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR", | |||
| STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR_MESSAGE = "STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR_MESSAGE", | |||
There was a problem hiding this comment.
We generally don't suffix with _MESSAGE...
something feels off here. Why do we have two different keys?
There was a problem hiding this comment.
One for the red text and one for the blue one
Otherwise user ends up having the same error message twice which IMO is even more confusing
| @@ -248,7 +248,7 @@ async def _react_to_exception( | |||
| self.state.last_error = err_id | |||
| elif isinstance(e, InternalServerError): | |||
| err_id = 'STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR' | |||
| self.state.last_error = err_id | |||
| self.state.last_error = 'STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR_MESSAGE' | |||
There was a problem hiding this comment.
this seems like a weird abuse of last_error...is that what's getting plumbed through as the message?
There was a problem hiding this comment.
Yeah, if you scroll down a bit, we send .last_error as message to the status callback to FE
There was a problem hiding this comment.
ooh that sounds like the bug
End-user friendly description of the problem this fixes or functionality that this introduces.
This PR fixes an issue where internal server error messages were displaying the raw translation key (e.g., STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR) instead of the actual translated error message. Users will now see properly formatted error messages when internal server errors occur.
Give a summary of what the PR does, explaining any non-trivial design decisions.
The PR makes the following changes:
These changes ensure that error messages are properly translated and displayed to users without unnecessary UI elements or showing raw translation keys.
Before fix:
After fix:
Link of any specific issues this addresses.
N/A
To run this PR locally, use the following command: