Skip to content

Commit 505d940

Browse files
roomoteroomote
committed
Fix #4946: Improve error handling in file editing tools
- Enhanced error handling in writeToFileTool, applyDiffTool, searchAndReplaceTool, and insertContentTool - Added comprehensive try-catch blocks around DiffViewProvider operations - Improved error reporting and logging for better debugging - Fixed silent failures by ensuring handleError is called for all tool errors - Enhanced DiffViewProvider.open() method with better error validation - Added proper cleanup and state reset on errors - All tests now passing including previously failing writeToFileTool error handling tests This resolves the issue where file editing tools were failing silently on Ubuntu 22.04 with Claude 4.0 Sonnet via OpenRouter.
1 parent 2e2f83b commit 505d940

File tree

7 files changed

+1506
-178
lines changed

7 files changed

+1506
-178
lines changed

‎roo-code-messages.log‎

Lines changed: 1110 additions & 0 deletions
Large diffs are not rendered by default.

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

Lines changed: 164 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,16 @@ export async function presentAssistantMessage(cline: Task) {
304304
const handleError = async (action: string, error: Error) => {
305305
const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}`
306306

307-
await cline.say(
308-
"error",
309-
`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
310-
)
307+
// Enhanced error logging for debugging
308+
console.error(`[Tool Error] ${block.name} - ${action}:`, error)
309+
310+
// More detailed error message for user
311+
const userErrorMessage = `Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}\n\nTool: ${block.name}\nAction: ${action}`
312+
313+
await cline.say("error", userErrorMessage)
314+
315+
// Record the tool error for tracking
316+
cline.recordToolError(block.name as ToolName, errorString)
311317

312318
pushToolResult(formatResponse.toolError(errorString))
313319
}
@@ -406,116 +412,169 @@ export async function presentAssistantMessage(cline: Task) {
406412
}
407413
}
408414

409-
switch (block.name) {
410-
case "write_to_file":
411-
await writeToFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
412-
break
413-
case "apply_diff": {
414-
// Get the provider and state to check experiment settings
415-
const provider = cline.providerRef.deref()
416-
let isMultiFileApplyDiffEnabled = false
417-
418-
if (provider) {
419-
const state = await provider.getState()
420-
isMultiFileApplyDiffEnabled = experiments.isEnabled(
421-
state.experiments ?? {},
422-
EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF,
423-
)
424-
}
415+
// Wrap tool execution in try-catch to ensure errors are properly handled
416+
try {
417+
switch (block.name) {
418+
case "write_to_file":
419+
await writeToFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
420+
break
421+
case "apply_diff": {
422+
// Get the provider and state to check experiment settings
423+
const provider = cline.providerRef.deref()
424+
let isMultiFileApplyDiffEnabled = false
425+
426+
if (provider) {
427+
const state = await provider.getState()
428+
isMultiFileApplyDiffEnabled = experiments.isEnabled(
429+
state.experiments ?? {},
430+
EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF,
431+
)
432+
}
425433

426-
if (isMultiFileApplyDiffEnabled) {
427-
await applyDiffTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
428-
} else {
429-
await applyDiffToolLegacy(
434+
if (isMultiFileApplyDiffEnabled) {
435+
await applyDiffTool(
436+
cline,
437+
block,
438+
askApproval,
439+
handleError,
440+
pushToolResult,
441+
removeClosingTag,
442+
)
443+
} else {
444+
await applyDiffToolLegacy(
445+
cline,
446+
block,
447+
askApproval,
448+
handleError,
449+
pushToolResult,
450+
removeClosingTag,
451+
)
452+
}
453+
break
454+
}
455+
case "insert_content":
456+
await insertContentTool(
430457
cline,
431458
block,
432459
askApproval,
433460
handleError,
434461
pushToolResult,
435462
removeClosingTag,
436463
)
437-
}
438-
break
464+
break
465+
case "search_and_replace":
466+
await searchAndReplaceTool(
467+
cline,
468+
block,
469+
askApproval,
470+
handleError,
471+
pushToolResult,
472+
removeClosingTag,
473+
)
474+
break
475+
case "read_file":
476+
await readFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
477+
break
478+
case "fetch_instructions":
479+
await fetchInstructionsTool(cline, block, askApproval, handleError, pushToolResult)
480+
break
481+
case "list_files":
482+
await listFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
483+
break
484+
case "codebase_search":
485+
await codebaseSearchTool(
486+
cline,
487+
block,
488+
askApproval,
489+
handleError,
490+
pushToolResult,
491+
removeClosingTag,
492+
)
493+
break
494+
case "list_code_definition_names":
495+
await listCodeDefinitionNamesTool(
496+
cline,
497+
block,
498+
askApproval,
499+
handleError,
500+
pushToolResult,
501+
removeClosingTag,
502+
)
503+
break
504+
case "search_files":
505+
await searchFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
506+
break
507+
case "browser_action":
508+
await browserActionTool(
509+
cline,
510+
block,
511+
askApproval,
512+
handleError,
513+
pushToolResult,
514+
removeClosingTag,
515+
)
516+
break
517+
case "execute_command":
518+
await executeCommandTool(
519+
cline,
520+
block,
521+
askApproval,
522+
handleError,
523+
pushToolResult,
524+
removeClosingTag,
525+
)
526+
break
527+
case "use_mcp_tool":
528+
await useMcpToolTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
529+
break
530+
case "access_mcp_resource":
531+
await accessMcpResourceTool(
532+
cline,
533+
block,
534+
askApproval,
535+
handleError,
536+
pushToolResult,
537+
removeClosingTag,
538+
)
539+
break
540+
case "ask_followup_question":
541+
await askFollowupQuestionTool(
542+
cline,
543+
block,
544+
askApproval,
545+
handleError,
546+
pushToolResult,
547+
removeClosingTag,
548+
)
549+
break
550+
case "switch_mode":
551+
await switchModeTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
552+
break
553+
case "new_task":
554+
await newTaskTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
555+
break
556+
case "attempt_completion":
557+
await attemptCompletionTool(
558+
cline,
559+
block,
560+
askApproval,
561+
handleError,
562+
pushToolResult,
563+
removeClosingTag,
564+
toolDescription,
565+
askFinishSubTaskApproval,
566+
)
567+
break
568+
default:
569+
// Handle unknown tool names
570+
const unknownToolError = new Error(`Unknown tool: ${block.name}`)
571+
await handleError(`executing unknown tool '${block.name}'`, unknownToolError)
572+
break
439573
}
440-
case "insert_content":
441-
await insertContentTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
442-
break
443-
case "search_and_replace":
444-
await searchAndReplaceTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
445-
break
446-
case "read_file":
447-
await readFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
448-
449-
break
450-
case "fetch_instructions":
451-
await fetchInstructionsTool(cline, block, askApproval, handleError, pushToolResult)
452-
break
453-
case "list_files":
454-
await listFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
455-
break
456-
case "codebase_search":
457-
await codebaseSearchTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
458-
break
459-
case "list_code_definition_names":
460-
await listCodeDefinitionNamesTool(
461-
cline,
462-
block,
463-
askApproval,
464-
handleError,
465-
pushToolResult,
466-
removeClosingTag,
467-
)
468-
break
469-
case "search_files":
470-
await searchFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
471-
break
472-
case "browser_action":
473-
await browserActionTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
474-
break
475-
case "execute_command":
476-
await executeCommandTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
477-
break
478-
case "use_mcp_tool":
479-
await useMcpToolTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
480-
break
481-
case "access_mcp_resource":
482-
await accessMcpResourceTool(
483-
cline,
484-
block,
485-
askApproval,
486-
handleError,
487-
pushToolResult,
488-
removeClosingTag,
489-
)
490-
break
491-
case "ask_followup_question":
492-
await askFollowupQuestionTool(
493-
cline,
494-
block,
495-
askApproval,
496-
handleError,
497-
pushToolResult,
498-
removeClosingTag,
499-
)
500-
break
501-
case "switch_mode":
502-
await switchModeTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
503-
break
504-
case "new_task":
505-
await newTaskTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
506-
break
507-
case "attempt_completion":
508-
await attemptCompletionTool(
509-
cline,
510-
block,
511-
askApproval,
512-
handleError,
513-
pushToolResult,
514-
removeClosingTag,
515-
toolDescription,
516-
askFinishSubTaskApproval,
517-
)
518-
break
574+
} catch (toolExecutionError) {
575+
// Catch any unhandled errors during tool execution
576+
console.error(`[Tool Execution Error] ${block.name}:`, toolExecutionError)
577+
await handleError(`executing tool '${block.name}'`, toolExecutionError as Error)
519578
}
520579

521580
break

‎src/core/tools/applyDiffTool.ts‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,33 @@ export async function applyDiffToolLegacy(
143143

144144
// Show diff view before asking for approval
145145
cline.diffViewProvider.editType = "modify"
146-
await cline.diffViewProvider.open(relPath)
147-
await cline.diffViewProvider.update(diffResult.content, true)
148-
await cline.diffViewProvider.scrollToFirstDiff()
146+
147+
try {
148+
await cline.diffViewProvider.open(relPath)
149+
} catch (openError) {
150+
console.error(`[applyDiffTool] Failed to open diff view for '${relPath}':`, openError)
151+
cline.consecutiveMistakeCount++
152+
cline.recordToolError("apply_diff", `Failed to open diff view: ${openError.message}`)
153+
pushToolResult(
154+
formatResponse.toolError(`Failed to open file editor for '${relPath}': ${openError.message}`),
155+
)
156+
await cline.diffViewProvider.reset()
157+
return
158+
}
159+
160+
try {
161+
await cline.diffViewProvider.update(diffResult.content, true)
162+
await cline.diffViewProvider.scrollToFirstDiff()
163+
} catch (updateError) {
164+
console.error(`[applyDiffTool] Failed to update diff view for '${relPath}':`, updateError)
165+
cline.consecutiveMistakeCount++
166+
cline.recordToolError("apply_diff", `Failed to update diff view: ${updateError.message}`)
167+
pushToolResult(
168+
formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
169+
)
170+
await cline.diffViewProvider.reset()
171+
return
172+
}
149173

150174
const completeMessage = JSON.stringify({
151175
...sharedMessageProps,

‎src/core/tools/insertContentTool.ts‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,35 @@ export async function insertContentTool(
107107
// Show changes in diff view
108108
if (!cline.diffViewProvider.isEditing) {
109109
await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {})
110-
// First open with original content
111-
await cline.diffViewProvider.open(relPath)
112-
await cline.diffViewProvider.update(fileContent, false)
113-
cline.diffViewProvider.scrollToFirstDiff()
114-
await delay(200)
110+
111+
try {
112+
// First open with original content
113+
await cline.diffViewProvider.open(relPath)
114+
} catch (openError) {
115+
console.error(`[insertContentTool] Failed to open diff view for '${relPath}':`, openError)
116+
cline.consecutiveMistakeCount++
117+
cline.recordToolError("insert_content", `Failed to open diff view: ${openError.message}`)
118+
pushToolResult(
119+
formatResponse.toolError(`Failed to open file editor for '${relPath}': ${openError.message}`),
120+
)
121+
await cline.diffViewProvider.reset()
122+
return
123+
}
124+
125+
try {
126+
await cline.diffViewProvider.update(fileContent, false)
127+
cline.diffViewProvider.scrollToFirstDiff()
128+
await delay(200)
129+
} catch (updateError) {
130+
console.error(`[insertContentTool] Failed to update diff view for '${relPath}':`, updateError)
131+
cline.consecutiveMistakeCount++
132+
cline.recordToolError("insert_content", `Failed to update diff view: ${updateError.message}`)
133+
pushToolResult(
134+
formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
135+
)
136+
await cline.diffViewProvider.reset()
137+
return
138+
}
115139
}
116140

117141
const diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent)
@@ -121,7 +145,21 @@ export async function insertContentTool(
121145
return
122146
}
123147

124-
await cline.diffViewProvider.update(updatedContent, true)
148+
try {
149+
await cline.diffViewProvider.update(updatedContent, true)
150+
} catch (updateError) {
151+
console.error(
152+
`[insertContentTool] Failed to update diff view with new content for '${relPath}':`,
153+
updateError,
154+
)
155+
cline.consecutiveMistakeCount++
156+
cline.recordToolError("insert_content", `Failed to update diff view: ${updateError.message}`)
157+
pushToolResult(
158+
formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
159+
)
160+
await cline.diffViewProvider.reset()
161+
return
162+
}
125163

126164
const completeMessage = JSON.stringify({
127165
...sharedMessageProps,

0 commit comments

Comments
 (0)