Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/types/src/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ export const clineMessageSchema = z.object({
reasoning_summary: z.string().optional(),
})
.optional(),
title: z.string().optional(), // Custom title for error messages
})
.optional(),
})
Expand Down
5 changes: 4 additions & 1 deletion src/core/assistant-message/presentAssistantMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,15 @@ export async function presentAssistantMessage(cline: Task) {
return await askApproval("tool", toolMessage)
}

const handleError = async (action: string, error: Error) => {
const handleError = async (action: string, error: Error, title?: string) => {
const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}`

await cline.say(
"error",
`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
undefined,
undefined,
title ? { title } : undefined,
)

pushToolResult(formatResponse.toolError(errorString))
Expand Down
32 changes: 29 additions & 3 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1164,15 +1164,41 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
}

async sayAndCreateMissingParamError(toolName: ToolName, paramName: string, relPath?: string) {
const relPathFormatted = relPath ? ` for '${relPath.toPosix()}'` : ""
await this.say(
"error",
`Roo tried to use ${toolName}${
relPath ? ` for '${relPath.toPosix()}'` : ""
} without value for required parameter '${paramName}'. Retrying...`,
t("tools:common.errors.missingParameterMessage", {
toolName,
relPath: relPathFormatted,
paramName,
}),
undefined,
undefined,
undefined,
undefined,
{
metadata: {
title: t("tools:common.errors.missingParameter"),
},
},
)
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
}

/**
* Helper method to say an error with a custom title
* @param title - The title to display for the error
* @param text - The error message text
* @param images - Optional images to include
*/
async sayError(title: string, text: string, images?: string[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The helper method is a nice addition! Consider adding a JSDoc example to make it more discoverable:
Could not locate file:

await this.say("error", text, images, undefined, undefined, undefined, {
metadata: {
title,
},
})
}

// Lifecycle
// Start / Resume / Abort / Dispose

Expand Down
24 changes: 22 additions & 2 deletions src/core/tools/__tests__/generateImageTool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,17 @@ describe("generateImageTool", () => {
mockRemoveClosingTag,
)

expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Input image not found"))
expect(mockCline.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("Input image not found"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "Input Image Not Found" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Input image not found"))
})

Expand All @@ -302,7 +312,17 @@ describe("generateImageTool", () => {
mockRemoveClosingTag,
)

expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Unsupported image format"))
expect(mockCline.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("Unsupported image format"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "Unsupported Image Format" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Unsupported image format"))
})
})
Expand Down
30 changes: 29 additions & 1 deletion src/core/tools/__tests__/insertContentTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ vi.mock("../../ignore/RooIgnoreController", () => ({
},
}))

vi.mock("../../../i18n", () => ({
t: vi.fn((key: string, params?: any) => {
// Return the key without the namespace prefix for testing
const keyWithoutNamespace = key.replace(/^[^:]+:/, "")
if (params) {
// Simple parameter replacement for testing
let result = keyWithoutNamespace
Object.entries(params).forEach(([key, value]) => {
result = result.replace(`{{${key}}}`, String(value))
})
return result
}
return keyWithoutNamespace
}),
}))

describe("insertContentTool", () => {
const testFilePath = "test/file.txt"
// Use a consistent mock absolute path for testing
Expand Down Expand Up @@ -226,7 +242,19 @@ describe("insertContentTool", () => {
expect(mockedFsReadFile).not.toHaveBeenCalled()
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content")
expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file"))
expect(mockCline.say).toHaveBeenCalledWith(
"error",
"insertContent.errors.cannotInsertIntoNonExistent",
undefined,
undefined,
undefined,
undefined,
expect.objectContaining({
metadata: expect.objectContaining({
title: "insertContent.errors.invalidLineNumber",
}),
}),
)
expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled()
expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled()
})
Expand Down
60 changes: 55 additions & 5 deletions src/core/tools/__tests__/useMcpToolTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,17 @@ describe("useMcpToolTool", () => {

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("invalid JSON argument"))
expect(mockTask.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("invalid JSON argument"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "Invalid JSON Arguments" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith("Tool error: Invalid args for test_server:test_tool")
})
})
Expand Down Expand Up @@ -343,7 +353,17 @@ describe("useMcpToolTool", () => {

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("does not exist"))
expect(mockTask.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("does not exist"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "MCP Tool Not Found" },
},
)
// Check that the error message contains available tools
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("existing-tool-1"))
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("existing-tool-2"))
Expand Down Expand Up @@ -390,7 +410,17 @@ describe("useMcpToolTool", () => {

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("does not exist"))
expect(mockTask.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("does not exist"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "MCP Tool Not Found" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("No tools available"))
})

Expand Down Expand Up @@ -484,7 +514,17 @@ describe("useMcpToolTool", () => {
// Assert
expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("not configured"))
expect(mockTask.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("not configured"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "MCP Server Not Found" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("s1"))
expect(callToolMock).not.toHaveBeenCalled()
expect(mockAskApproval).not.toHaveBeenCalled()
Expand Down Expand Up @@ -527,7 +567,17 @@ describe("useMcpToolTool", () => {
// Assert
expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("not configured"))
expect(mockTask.say).toHaveBeenCalledWith(
"error",
expect.stringContaining("not configured"),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "MCP Server Not Found" },
},
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("No servers available"))
expect(callToolMock).not.toHaveBeenCalled()
expect(mockAskApproval).not.toHaveBeenCalled()
Expand Down
12 changes: 10 additions & 2 deletions src/core/tools/__tests__/writeToFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,11 @@ describe("writeToFileTool", () => {

await executeWriteFileTool({})

expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
expect(mockHandleError).toHaveBeenCalledWith(
"writing file",
expect.any(Error),
"writeToFile.errors.writeFileError",
)
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})

Expand All @@ -412,7 +416,11 @@ describe("writeToFileTool", () => {

await executeWriteFileTool({}, { isPartial: true })

expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
expect(mockHandleError).toHaveBeenCalledWith(
"writing file",
expect.any(Error),
"writeToFile.errors.writeFileError",
)
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
})
})
Expand Down
9 changes: 6 additions & 3 deletions src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { fileExistsAtPath } from "../../utils/fs"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { t } from "../../i18n"

export async function applyDiffToolLegacy(
cline: Task,
Expand Down Expand Up @@ -82,8 +83,10 @@ export async function applyDiffToolLegacy(
if (!fileExists) {
cline.consecutiveMistakeCount++
cline.recordToolError("apply_diff")
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>`
await cline.say("error", formattedError)
const formattedError = `${t("tools:applyDiff.errors.fileDoesNotExist", { path: absolutePath })}\n\n<error_details>\n${t("tools:applyDiff.errors.fileDoesNotExistDetails")}\n</error_details>`
await cline.say("error", formattedError, undefined, undefined, undefined, undefined, {
metadata: { title: t("tools:applyDiff.errors.fileNotFound") },
})
pushToolResult(formattedError)
return
}
Expand Down Expand Up @@ -248,7 +251,7 @@ export async function applyDiffToolLegacy(
return
}
} catch (error) {
await handleError("applying diff", error)
await handleError("applying diff", error, t("tools:applyDiff.errors.applyDiffError"))
await cline.diffViewProvider.reset()
return
}
Expand Down
15 changes: 13 additions & 2 deletions src/core/tools/askFollowupQuestionTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Task } from "../task/Task"
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
import { formatResponse } from "../prompts/responses"
import { parseXml } from "../../utils/xml"
import { t } from "../../i18n"

export async function askFollowupQuestionTool(
cline: Task,
Expand Down Expand Up @@ -48,7 +49,17 @@ export async function askFollowupQuestionTool(
} catch (error) {
cline.consecutiveMistakeCount++
cline.recordToolError("ask_followup_question")
await cline.say("error", `Failed to parse operations: ${error.message}`)
await cline.say(
"error",
`Failed to parse operations: ${error.message}`,
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "XML Parse Error" },
},
)
pushToolResult(formatResponse.toolError("Invalid operations xml format"))
return
}
Expand Down Expand Up @@ -83,7 +94,7 @@ export async function askFollowupQuestionTool(
return
}
} catch (error) {
await handleError("asking question", error)
await handleError("asking question", error, t("tools:askFollowupQuestion.errors.askError"))
return
}
}
14 changes: 12 additions & 2 deletions src/core/tools/executeCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export async function executeCommandTool(
return
}
} catch (error) {
await handleError("executing command", error)
await handleError("executing command", error, t("tools:executeCommand.errors.executeError"))
return
}
}
Expand Down Expand Up @@ -271,7 +271,17 @@ export async function executeCommand(
if (isTimedOut) {
const status: CommandExecutionStatus = { executionId, status: "timeout" }
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
await task.say("error", t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
await task.say(
"error",
t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }),
undefined,
undefined,
undefined,
undefined,
{
metadata: { title: "Command Timeout" },
},
)
task.terminalProcess = undefined

return [
Expand Down
3 changes: 2 additions & 1 deletion src/core/tools/fetchInstructionsTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { fetchInstructions } from "../prompts/instructions/instructions"
import { ClineSayTool } from "../../shared/ExtensionMessage"
import { formatResponse } from "../prompts/responses"
import { ToolUse, AskApproval, HandleError, PushToolResult } from "../../shared/tools"
import { t } from "../../i18n"

export async function fetchInstructionsTool(
cline: Task,
Expand Down Expand Up @@ -58,6 +59,6 @@ export async function fetchInstructionsTool(
return
}
} catch (error) {
await handleError("fetch instructions", error)
await handleError("fetch instructions", error, t("tools:fetchInstructions.errors.fetchError"))
}
}
Loading
Loading