feat: Extend error handling for the UI #4601
Conversation
|
This draft is actually for the UI. The frontend looks for error observations and displays them inline in the chat interface. This just makes it more human-readable. I'm looking for more cases where we would expect an error in the UI but don't receive it Don't know if that would still mean we should consider doing it in agent session. LMK |
amanape
changed the title
Yes, I know it's for the UI, and it's welcome that the UI would present some errors to the user, in a user-friendly way. We talked about this before, sorry if something was unclear in my comment. What I meant is simply that:
Up to you, IMHO this PR doesn't have to adapt to the change before it happens. I am fine with this PR as it is. I can take care of merging this appropriately in 4575, depending what that solution will look like. |
| @@ -144,8 +144,11 @@ async def report_error(self, message: str, exception: Exception | None = None): | |||
| if exception: | |||
| self.state.last_error += f': {exception}' | |||
| detail = str(exception) if exception is not None else '' | |||
There was a problem hiding this comment.
This line will send to the chat box the rest of exceptions, and that's actually surprising. For example:
I think the (currently) intended behavior is to not show these, or at least make a generic user-friendly message for them. Or did we really change that intention?
There was a problem hiding this comment.
The current default behaviour is display errors that are ErrorObservation's (e.g., those called with report_error) and a temporary toast for errors sent directly to the WebSocket such as response errors (e.g., those called with send_error).
Errors like the one in your screenshot probably shouldn't be displayed but only because the developer should handle it in the code (I doubt OpenHands can do something about internal request errors such as these).
In general, it is in my TODO list to display the errors collapsed by default, and probably truncated if they are super long.
There was a problem hiding this comment.
Yeah, part of the mess right now can be solved if these are not ErrorObs in the first place. OK, cool.
That would be appreciated. I just wanted to extend the app to display errors with whatever we have now. I do agree that we should refactor. I imagine we're going to want to extend this in the future to a point where we can efficiently handle any type of errors, error observations or otherwise. |
End-user friendly description of the problem this fixes or functionality that this introduces
Give a summary of what the PR does, explaining any non-trivial design decisions
Link of any specific issues this addresses