Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the error handling logic where specific backend error messages were not being properly propagated to the UI. When the kagent-controller service was down, users would see a generic "Agent not found" error instead of the actual error message like "Network error - Could not reach backend server."
Changes:
- Updated
createErrorResponsefunction to populate theerrorfield in addition to the existingmessagefield, ensuring error responses conform to theBaseResponse<T>interface
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const errorMessage = error instanceof Error ? error.message : defaultMessage; | ||
| console.error(defaultMessage, error); | ||
| return { message: errorMessage }; | ||
| return { message: errorMessage, error: errorMessage }; |
There was a problem hiding this comment.
Both 'message' and 'error' are set to the same value (errorMessage), creating redundancy. Consider setting 'message' to the defaultMessage parameter to provide context about what operation failed, while 'error' contains the actual error details. For example: return { message: defaultMessage, error: errorMessage }. This would make 'message' describe the operation context (e.g., "Error getting agent") and 'error' contain the specific error (e.g., "Network error - Could not reach backend server").
| return { message: errorMessage, error: errorMessage }; | |
| return { message: defaultMessage, error: errorMessage }; |
Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
dobesv
force-pushed
the
ui-error-handler-fixup
branch
from
95e6cf7 to
f1ecefd
Compare
|
I notice you approved but did not merge - are you thinking I would merge? Or is there some automated process that would merge it? I don't have the ability to merge it. |
|
This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs. |
|
@peterj could you help to merge this change? |
I noticed that when kagent-controller was down, the UI showed the wrong error "Agent not found". According to claude code, this change should fix it to show a more specific error. Basically it seems like some code was expecting
errorto be set for errors, so this sets error and message both to handle those cases.