Skip to content

Commit 6136ba9

Browse files
hannesrudolphhannesrudolph
committed
feat: improve error display with collapsible UI and contextual titles
- Add collapsible error display in ChatRow component with expand/collapse functionality - Implement contextual error titles for 13 different tool types - Add i18n support for error titles with fallback to generic message - Include comprehensive test coverage for new error display features - Preserve full error details in collapsed state for debugging - Add visual indicators (chevron icons) for error state management
1 parent c25cfde commit 6136ba9

21 files changed

+706
-87
lines changed

‎packages/types/src/message.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export const clineMessageSchema = z.object({
205205
ask: clineAskSchema.optional(),
206206
say: clineSaySchema.optional(),
207207
text: z.string().optional(),
208+
title: z.string().optional(), // Custom title for error messages and other displays
208209
images: z.array(z.string()).optional(),
209210
partial: z.boolean().optional(),
210211
reasoning: z.string().optional(),

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,14 @@ export async function presentAssistantMessage(cline: Task) {
323323
await cline.say(
324324
"error",
325325
`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
326+
undefined, // images
327+
undefined, // partial
328+
undefined, // checkpoint
329+
undefined, // progressStatus
330+
{ title: `Tool Call Error: ${block.name}` }, // Custom title with tool name
326331
)
327332

328-
pushToolResult(formatResponse.toolError(errorString))
333+
pushToolResult(formatResponse.toolError(errorString, block.name))
329334
}
330335

331336
// If block is partial, remove partial closing tag so its not
@@ -377,7 +382,7 @@ export async function presentAssistantMessage(cline: Task) {
377382
)
378383
} catch (error) {
379384
cline.consecutiveMistakeCount++
380-
pushToolResult(formatResponse.toolError(error.message))
385+
pushToolResult(formatResponse.toolError(error.message, block.name))
381386
break
382387
}
383388

@@ -416,6 +421,7 @@ export async function presentAssistantMessage(cline: Task) {
416421
pushToolResult(
417422
formatResponse.toolError(
418423
`Tool call repetition limit reached for ${block.name}. Please try a different approach.`,
424+
block.name,
419425
),
420426
)
421427
break

‎src/core/prompts/__tests__/responses-tool-error.spec.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, it, expect } from "vitest"
2+
import { formatResponse } from "../responses"
3+
4+
describe("formatResponse.toolError", () => {
5+
it("should format error without tool name when not provided", () => {
6+
const error = "Something went wrong"
7+
const result = formatResponse.toolError(error)
8+
9+
expect(result).toBe("Tool Execution Error\n<error>\nSomething went wrong\n</error>")
10+
})
11+
12+
it("should format error with tool name when provided", () => {
13+
const error = "Invalid mode: test_mode"
14+
const toolName = "switch_mode"
15+
const result = formatResponse.toolError(error, toolName)
16+
17+
expect(result).toBe("Tool Call Error: switch_mode\n<error>\nInvalid mode: test_mode\n</error>")
18+
})
19+
20+
it("should handle undefined error message", () => {
21+
const result = formatResponse.toolError(undefined, "new_task")
22+
23+
expect(result).toBe("Tool Call Error: new_task\n<error>\nundefined\n</error>")
24+
})
25+
26+
it("should work with various tool names", () => {
27+
const testCases = [
28+
{ toolName: "write_to_file", expected: "Tool Call Error: write_to_file" },
29+
{ toolName: "execute_command", expected: "Tool Call Error: execute_command" },
30+
{ toolName: "apply_diff", expected: "Tool Call Error: apply_diff" },
31+
{ toolName: "new_task", expected: "Tool Call Error: new_task" },
32+
{ toolName: "use_mcp_tool", expected: "Tool Call Error: use_mcp_tool" },
33+
]
34+
35+
testCases.forEach(({ toolName, expected }) => {
36+
const result = formatResponse.toolError("Test error", toolName)
37+
expect(result).toContain(expected)
38+
})
39+
})
40+
41+
it("should maintain backward compatibility when tool name is not provided", () => {
42+
// This ensures existing code that doesn't pass toolName still works
43+
const error = "Legacy error"
44+
const result = formatResponse.toolError(error)
45+
46+
// Should not contain "Tool Call Error:" prefix
47+
expect(result).not.toContain("Tool Call Error:")
48+
// Should contain generic title
49+
expect(result).toContain("Tool Execution Error")
50+
})
51+
})

‎src/core/prompts/responses.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ export const formatResponse = {
1313
toolApprovedWithFeedback: (feedback?: string) =>
1414
`The user approved this operation and provided the following context:\n<feedback>\n${feedback}\n</feedback>`,
1515

16-
toolError: (error?: string) => `The tool execution failed with the following error:\n<error>\n${error}\n</error>`,
16+
toolError: (error?: string, toolName?: string) => {
17+
const title = toolName ? `Tool Call Error: ${toolName}` : "Tool Execution Error"
18+
return `${title}\n<error>\n${error}\n</error>`
19+
},
1720

1821
rooIgnoreError: (path: string) =>
1922
`Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`,

‎src/core/task/Task.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
10251025
options: {
10261026
isNonInteractive?: boolean
10271027
metadata?: Record<string, unknown>
1028+
title?: string // Optional custom title for error messages
10281029
} = {},
10291030
contextCondense?: ContextCondense,
10301031
): Promise<undefined> {
@@ -1059,6 +1060,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
10591060
type: "say",
10601061
say: type,
10611062
text,
1063+
title: options.title, // Include custom title if provided
10621064
images,
10631065
partial,
10641066
contextCondense,
@@ -1106,6 +1108,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
11061108
type: "say",
11071109
say: type,
11081110
text,
1111+
title: options.title, // Include custom title if provided
11091112
images,
11101113
contextCondense,
11111114
metadata: options.metadata,
@@ -1129,6 +1132,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
11291132
type: "say",
11301133
say: type,
11311134
text,
1135+
title: options.title, // Include custom title if provided
11321136
images,
11331137
checkpoint,
11341138
contextCondense,
@@ -1142,8 +1146,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
11421146
`Roo tried to use ${toolName}${
11431147
relPath ? ` for '${relPath.toPosix()}'` : ""
11441148
} without value for required parameter '${paramName}'. Retrying...`,
1149+
undefined, // images
1150+
undefined, // partial
1151+
undefined, // checkpoint
1152+
undefined, // progressStatus
1153+
{ title: `Tool Call Error: ${toolName}` }, // Custom title for the error
11451154
)
1146-
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
1155+
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName), toolName)
11471156
}
11481157

11491158
// Lifecycle
@@ -2269,6 +2278,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
22692278
await this.say(
22702279
"error",
22712280
"Unexpected API Response: The language model did not provide any assistant messages. This may indicate an issue with the API or the model's output.",
2281+
undefined,
2282+
undefined,
2283+
undefined,
2284+
undefined,
2285+
{ title: "API Response Error" },
22722286
)
22732287

22742288
await this.addToApiConversationHistory({

‎src/core/tools/__tests__/insertContentTool.spec.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,15 @@ describe("insertContentTool", () => {
226226
expect(mockedFsReadFile).not.toHaveBeenCalled()
227227
expect(mockCline.consecutiveMistakeCount).toBe(1)
228228
expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content")
229-
expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file"))
229+
expect(mockCline.say).toHaveBeenCalledWith(
230+
"error",
231+
expect.stringContaining("non-existent file"),
232+
undefined,
233+
undefined,
234+
undefined,
235+
undefined,
236+
{ title: "Invalid Line Number" },
237+
)
230238
expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled()
231239
expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled()
232240
})

‎src/core/tools/applyDiffTool.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { fileExistsAtPath } from "../../utils/fs"
1313
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1414
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1515
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
16+
import { t } from "../../i18n"
1617

1718
export async function applyDiffToolLegacy(
1819
cline: Task,
@@ -72,7 +73,7 @@ export async function applyDiffToolLegacy(
7273

7374
if (!accessAllowed) {
7475
await cline.say("rooignore_error", relPath)
75-
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
76+
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath), "apply_diff"))
7677
return
7778
}
7879

@@ -83,7 +84,9 @@ export async function applyDiffToolLegacy(
8384
cline.consecutiveMistakeCount++
8485
cline.recordToolError("apply_diff")
8586
const formattedError = `File does not exist at path: ${absolutePath}\n\n<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>`
86-
await cline.say("error", formattedError)
87+
await cline.say("error", formattedError, undefined, undefined, undefined, undefined, {
88+
title: t("tools:errors.fileNotFound"),
89+
})
8790
pushToolResult(formattedError)
8891
return
8992
}

‎src/core/tools/askFollowupQuestionTool.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,16 @@ export async function askFollowupQuestionTool(
4848
} catch (error) {
4949
cline.consecutiveMistakeCount++
5050
cline.recordToolError("ask_followup_question")
51-
await cline.say("error", `Failed to parse operations: ${error.message}`)
52-
pushToolResult(formatResponse.toolError("Invalid operations xml format"))
51+
await cline.say(
52+
"error",
53+
`Failed to parse operations: ${error.message}`,
54+
undefined,
55+
undefined,
56+
undefined,
57+
undefined,
58+
{ title: "Parse Error" },
59+
)
60+
pushToolResult(formatResponse.toolError("Invalid operations xml format", "ask_followup_question"))
5361
return
5462
}
5563

‎src/core/tools/attemptCompletionTool.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export async function attemptCompletionTool(
4646
pushToolResult(
4747
formatResponse.toolError(
4848
"Cannot complete task while there are incomplete todos. Please finish all todos before attempting completion.",
49+
"attempt_completion",
4950
),
5051
)
5152

‎src/core/tools/executeCommandTool.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ export async function executeCommandTool(
4747

4848
if (ignoredFileAttemptedToAccess) {
4949
await task.say("rooignore_error", ignoredFileAttemptedToAccess)
50-
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess)))
50+
pushToolResult(
51+
formatResponse.toolError(
52+
formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess),
53+
"execute_command",
54+
),
55+
)
5156
return
5257
}
5358

@@ -271,7 +276,15 @@ export async function executeCommand(
271276
if (isTimedOut) {
272277
const status: CommandExecutionStatus = { executionId, status: "timeout" }
273278
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
274-
await task.say("error", t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
279+
await task.say(
280+
"error",
281+
t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }),
282+
undefined,
283+
undefined,
284+
undefined,
285+
undefined,
286+
{ title: t("tools:errors.commandTimeout") },
287+
)
275288
task.terminalProcess = undefined
276289

277290
return [

0 commit comments

Comments
 (0)