feat: improve error display with custom titles and expandable UI #7794
Conversation
- Add metadata support to ClineMessage type for storing custom error titles - Update ChatRow component to display errors with expandable UI similar to diff_error - Add sayError helper method to Task class for convenient error reporting - Update existing error calls to use custom titles for better context - Fix test to match new error format with metadata
dosubot
bot
added
size:L
|
@roomote-agent can you update all of the instances, not just a couple? |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| ) | ||
| case "error": | ||
| // Extract custom title from metadata if available | ||
| const errorTitle = (message as any).metadata?.title || t("chat:error") |
There was a problem hiding this comment.
The expandable error UI implementation is effective—extracting a custom error title from message metadata and showing error details upon expansion improves user experience. Consider refactoring repeated inline styles into reusable CSS classes or style objects, and replacing the use of 'any' with proper TypeScript types for message metadata.
| const errorTitle = (message as any).metadata?.title || t("chat:error") | |
| const errorTitle = (message as ClineMessage & { metadata?: { title?: string } }).metadata?.title || t("chat:error") |
implementation-plan.md
Outdated
|
|
||
| ## Overview | ||
|
|
||
| Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: |
There was a problem hiding this comment.
Typo: In the overview, the text references cline.say("error", ...). Please confirm if 'cline' is intended, or if it should be something like 'client.say' or 'this.say'.
| Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: | |
| Currently, `this.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: |
|
Hi @mrubens! I see your request to update all instances. I'll search through the codebase to find all error reporting instances and update them with custom titles for better context. Working on it now! |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| )} | ||
| </div> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Consider extracting this expandable error UI into a reusable component. The error display logic here (lines 1252-1341) is very similar to the diff_error display above (lines 964-1050). Creating a shared component could reduce code duplication and make the UI more maintainable.
| * @param text - The error message text | ||
| * @param images - Optional images to include | ||
| */ | ||
| async sayError(title: string, text: string, images?: string[]) { |
There was a problem hiding this comment.
The helper method is a nice addition! Consider adding a JSDoc example to make it more discoverable:
Could not locate file:
implementation-plan.md
Outdated
| @@ -0,0 +1,113 @@ | |||
| # Implementation Plan: Improve Error Display in Webview | |||
There was a problem hiding this comment.
This planning document was helpful during development but could be removed from the final PR to keep the repository clean. Consider moving it to a GitHub issue or discussion if you want to preserve the implementation details.
hannesrudolph
added
the
Issue/PR - Triage
- Updated generateImageTool.ts with 7 custom error titles - Updated executeCommandTool.ts with Command Timeout error title - Updated askFollowupQuestionTool.ts with XML Parse Error title - Updated useMcpToolTool.ts with 5 MCP-related error titles - Updated searchAndReplaceTool.ts with File Not Found and File Read Error titles - Fixed all related test files to match new error format with metadata parameter This ensures all error reporting across tools uses the new custom title feature for better error display in the UI.
| const inputImageExists = await fileExistsAtPath(inputImageFullPath) | ||
| if (!inputImageExists) { | ||
| await cline.say("error", `Input image not found: ${getReadablePath(cline.cwd, inputImagePath)}`) | ||
| await cline.say( |
There was a problem hiding this comment.
Many error-reporting calls (e.g. on lines 76-86) pass multiple undefined parameters before the metadata argument. Consider refactoring these cline.say invocations into a helper function or using an options object to encapsulate optional parameters, which would improve readability and maintainability.
|
✅ All error reporting instances have been updated with custom titles! I've successfully updated all remaining error reporting instances across the tools directory to use the new custom error title format. Here's what was changed: Files Updated:
Test Files Updated:
All tests are passing ✅ and CI checks have completed successfully. The error display enhancement is now consistently applied across all tool error reporting! |
|
@roomote-agent can you switch into translate mode and internationalize them as well? |
|
Hi @mrubens! I see your request to internationalize the error messages. Let me switch to translate mode and work on adding i18n support for the custom error titles and messages introduced in this PR. I'll get started on this right away. |
- Added i18n support for error titles and messages in applyDiffTool, insertContentTool, and Task.ts - Added English translations for new error messages in tools.json - Translated error messages to all 18 supported languages (ca, de, es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, tr, vi, zh-CN, zh-TW) - Updated code to use i18n keys instead of hardcoded strings - Fixed tests to work with i18n keys - All translations validated with find-missing-translations.js script
| "common": { | ||
| "errors": { | ||
| "missingParameter": "Fehlender Parameter", | ||
| "missingParameterMessage": "Roo hat versucht, {{toolName}}{{relPath}} ohne Wert für den erforderlichen Parameter '{{paramName}}' zu verwenden. Versuche erneut..." |
There was a problem hiding this comment.
Lexicographical note: In the string "Roo hat versucht, {{toolName}}{{relPath}} ohne Wert für den erforderlichen Parameter '{{paramName}}' zu verwenden..." there might be a missing space between the {{toolName}} and {{relPath}} variables. Consider adding a space if appropriate.
| "missingParameterMessage": "Roo hat versucht, {{toolName}}{{relPath}} ohne Wert für den erforderlichen Parameter '{{paramName}}' zu verwenden. Versuche erneut..." | |
| "missingParameterMessage": "Roo hat versucht, {{toolName}} {{relPath}} ohne Wert für den erforderlichen Parameter '{{paramName}}' zu verwenden. Versuche erneut..." |
| "common": { | ||
| "errors": { | ||
| "missingParameter": "Missing Parameter Error", | ||
| "missingParameterMessage": "Roo tried to use {{toolName}}{{relPath}} without value for required parameter '{{paramName}}'. Retrying..." |
There was a problem hiding this comment.
In the error message for 'missingParameterMessage', there seems to be a potential issue: the placeholders {{toolName}} and {{relPath}} are concatenated without a space. Is this intentional, or should there be a space between them? Also, consider rephrasing 'without value for required parameter' to 'without a value for the required parameter' for improved clarity.
| "missingParameterMessage": "Roo tried to use {{toolName}}{{relPath}} without value for required parameter '{{paramName}}'. Retrying..." | |
| "missingParameterMessage": "Roo tried to use {{toolName}} {{relPath}} without a value for the required parameter '{{paramName}}'. Retrying..." |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
|
✅ Internationalization Complete! I have successfully internationalized all the error messages introduced in this PR. Here's what was done: Changes Made:
CI Status:All checks have passed successfully! ✅ The error messages will now display in the user's preferred language, improving the accessibility and user experience of the extension. |
- Updated HandleError type to accept optional title parameter - Modified handleError function in presentAssistantMessage to pass title to say method - Added custom error titles to various tool error handlers - Updated tests to expect the new third parameter
|
✅ Additional Update: handleError Enhancement I've also implemented the requested enhancement to the Changes Made:
This enhancement allows tools to provide more specific error titles when calling All tests are passing and the changes have been pushed to the PR branch. |
|
@mrubens All requested changes have been completed! ✅ Summary of changes:1. ✅ Updated
|
| }, | ||
| "newTaskTool": { | ||
| "errors": { | ||
| "createError": "Neue Aufgabe Erstellungsfehler" |
There was a problem hiding this comment.
Typo suggestion: The error message "Neue Aufgabe Erstellungsfehler" seems inconsistent with the compound style of other messages. Consider rewording it (e.g., "NeueAufgabeErstellungsfehler" or adding hyphens) for consistency.
| "createError": "Neue Aufgabe Erstellungsfehler" | |
| "createError": "NeueAufgabeErstellungsfehler" |
|
✅ All CI checks are now passing! The internationalization work is complete:
The error display feature is now fully internationalized with custom error titles for all tools across 18 languages! 🌍 |
|
Hi @mrubens! I see your request to internationalize the new error display improvements. I'll switch to translate mode and handle the internationalization of all the new error strings. Working on it now! 🌐 |
|
Hi @mrubens! Good news! I've reviewed the PR and all the new error display improvements are already fully internationalized! 🎉 Summary:✅ All translations are complete - The custom error titles and messages have been translated across all 17 supported languages (ca, de, es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, tr, vi, zh-CN, zh-TW) ✅ Translation validation passed - Running ✅ Tests are passing - All related tests are green What was internationalized:
The PR is ready to go from an internationalization perspective! The error display improvements with expandable UI and custom titles are fully localized. 🌐 |
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Needs Review]
in Roo Code Roadmap
mrubens
moved this from PR [Needs Review]
to PR [Changes Requested]
in Roo Code Roadmap
|
Closing, this will be redone |
github-project-automation
bot
moved this from PR [Changes Requested]
to Done
in Roo Code Roadmap
This PR improves how error messages are displayed in the webview by adding support for custom titles and an expandable/collapsible UI pattern similar to the existing "Edit Unsuccessful" display.
Changes
sayErrorhelper method to Task class for convenient error reporting with custom titlesapplyDiffTool: "File Not Found" errorinsertContentTool: "Invalid Line Number" errorBenefits
Testing
insertContentTool.spec.tstestImportant
Enhances error display in webview with custom titles and expandable UI in
ChatRow, adding asayErrorhelper inTaskand updating error calls for better context.ChatRowcomponent updated to display errors with expandable UI, showing custom titles or default "Error".sayErrorhelper method added toTaskclass for error reporting with custom titles.applyDiffToolandinsertContentTool.This description was created by
for 45a877d. You can customize this summary. It will automatically update as commits are pushed.