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.

src/core/assistant-message/presentAssistantMessage.ts

@@ -304,10 +304,16 @@ export async function presentAssistantMessage(cline: Task) {
 			const handleError = async (action: string, error: Error) => {
 				const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}`
 
-				await cline.say(
-					"error",
-					`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
-				)
+				// Enhanced error logging for debugging
+				console.error(`[Tool Error] ${block.name} - ${action}:`, error)
+
+				// More detailed error message for user
+				const userErrorMessage = `Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}\n\nTool: ${block.name}\nAction: ${action}`
+
+				await cline.say("error", userErrorMessage)
+
+				// Record the tool error for tracking
+				cline.recordToolError(block.name as ToolName, errorString)
 
 				pushToolResult(formatResponse.toolError(errorString))
 			}
@@ -406,27 +412,48 @@ export async function presentAssistantMessage(cline: Task) {
 				}
 			}
 
-			switch (block.name) {
-				case "write_to_file":
-					await writeToFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "apply_diff": {
-					// Get the provider and state to check experiment settings
-					const provider = cline.providerRef.deref()
-					let isMultiFileApplyDiffEnabled = false
+			// Wrap tool execution in try-catch to ensure errors are properly handled
+			try {
+				switch (block.name) {
+					case "write_to_file":
+						await writeToFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "apply_diff": {
+						// Get the provider and state to check experiment settings
+						const provider = cline.providerRef.deref()
+						let isMultiFileApplyDiffEnabled = false
 
-					if (provider) {
-						const state = await provider.getState()
-						isMultiFileApplyDiffEnabled = experiments.isEnabled(
-							state.experiments ?? {},
-							EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF,
-						)
+						if (provider) {
+							const state = await provider.getState()
+							isMultiFileApplyDiffEnabled = experiments.isEnabled(
+								state.experiments ?? {},
+								EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF,
+							)
+						}
+
+						if (isMultiFileApplyDiffEnabled) {
+							await applyDiffTool(
+								cline,
+								block,
+								askApproval,
+								handleError,
+								pushToolResult,
+								removeClosingTag,
+							)
+						} else {
+							await applyDiffToolLegacy(
+								cline,
+								block,
+								askApproval,
+								handleError,
+								pushToolResult,
+								removeClosingTag,
+							)
+						}
+						break
 					}
-
-					if (isMultiFileApplyDiffEnabled) {
-						await applyDiffTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					} else {
-						await applyDiffToolLegacy(
+					case "insert_content":
+						await insertContentTool(
 							cline,
 							block,
 							askApproval,
@@ -434,88 +461,120 @@ export async function presentAssistantMessage(cline: Task) {
 							pushToolResult,
 							removeClosingTag,
 						)
-					}
-					break
+						break
+					case "search_and_replace":
+						await searchAndReplaceTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "read_file":
+						await readFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "fetch_instructions":
+						await fetchInstructionsTool(cline, block, askApproval, handleError, pushToolResult)
+						break
+					case "list_files":
+						await listFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "codebase_search":
+						await codebaseSearchTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "list_code_definition_names":
+						await listCodeDefinitionNamesTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "search_files":
+						await searchFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "browser_action":
+						await browserActionTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "execute_command":
+						await executeCommandTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "use_mcp_tool":
+						await useMcpToolTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "access_mcp_resource":
+						await accessMcpResourceTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "ask_followup_question":
+						await askFollowupQuestionTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+						)
+						break
+					case "switch_mode":
+						await switchModeTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "new_task":
+						await newTaskTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
+						break
+					case "attempt_completion":
+						await attemptCompletionTool(
+							cline,
+							block,
+							askApproval,
+							handleError,
+							pushToolResult,
+							removeClosingTag,
+							toolDescription,
+							askFinishSubTaskApproval,
+						)
+						break
+					default:
+						// Handle unknown tool names
+						const unknownToolError = new Error(`Unknown tool: ${block.name}`)
+						await handleError(`executing unknown tool '${block.name}'`, unknownToolError)
+						break
 				}
-				case "insert_content":
-					await insertContentTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "search_and_replace":
-					await searchAndReplaceTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "read_file":
-					await readFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-
-					break
-				case "fetch_instructions":
-					await fetchInstructionsTool(cline, block, askApproval, handleError, pushToolResult)
-					break
-				case "list_files":
-					await listFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "codebase_search":
-					await codebaseSearchTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "list_code_definition_names":
-					await listCodeDefinitionNamesTool(
-						cline,
-						block,
-						askApproval,
-						handleError,
-						pushToolResult,
-						removeClosingTag,
-					)
-					break
-				case "search_files":
-					await searchFilesTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "browser_action":
-					await browserActionTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "execute_command":
-					await executeCommandTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "use_mcp_tool":
-					await useMcpToolTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "access_mcp_resource":
-					await accessMcpResourceTool(
-						cline,
-						block,
-						askApproval,
-						handleError,
-						pushToolResult,
-						removeClosingTag,
-					)
-					break
-				case "ask_followup_question":
-					await askFollowupQuestionTool(
-						cline,
-						block,
-						askApproval,
-						handleError,
-						pushToolResult,
-						removeClosingTag,
-					)
-					break
-				case "switch_mode":
-					await switchModeTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "new_task":
-					await newTaskTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
-					break
-				case "attempt_completion":
-					await attemptCompletionTool(
-						cline,
-						block,
-						askApproval,
-						handleError,
-						pushToolResult,
-						removeClosingTag,
-						toolDescription,
-						askFinishSubTaskApproval,
-					)
-					break
+			} catch (toolExecutionError) {
+				// Catch any unhandled errors during tool execution
+				console.error(`[Tool Execution Error] ${block.name}:`, toolExecutionError)
+				await handleError(`executing tool '${block.name}'`, toolExecutionError as Error)
 			}
 
 			break

src/core/tools/applyDiffTool.ts

@@ -143,9 +143,33 @@ export async function applyDiffToolLegacy(
 
 			// Show diff view before asking for approval
 			cline.diffViewProvider.editType = "modify"
-			await cline.diffViewProvider.open(relPath)
-			await cline.diffViewProvider.update(diffResult.content, true)
-			await cline.diffViewProvider.scrollToFirstDiff()
+
+			try {
+				await cline.diffViewProvider.open(relPath)
+			} catch (openError) {
+				console.error(`[applyDiffTool] Failed to open diff view for '${relPath}':`, openError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("apply_diff", `Failed to open diff view: ${openError.message}`)
+				pushToolResult(
+					formatResponse.toolError(`Failed to open file editor for '${relPath}': ${openError.message}`),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
+
+			try {
+				await cline.diffViewProvider.update(diffResult.content, true)
+				await cline.diffViewProvider.scrollToFirstDiff()
+			} catch (updateError) {
+				console.error(`[applyDiffTool] Failed to update diff view for '${relPath}':`, updateError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("apply_diff", `Failed to update diff view: ${updateError.message}`)
+				pushToolResult(
+					formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
 
 			const completeMessage = JSON.stringify({
 				...sharedMessageProps,

src/core/tools/insertContentTool.ts

@@ -107,11 +107,35 @@ export async function insertContentTool(
 		// Show changes in diff view
 		if (!cline.diffViewProvider.isEditing) {
 			await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {})
-			// First open with original content
-			await cline.diffViewProvider.open(relPath)
-			await cline.diffViewProvider.update(fileContent, false)
-			cline.diffViewProvider.scrollToFirstDiff()
-			await delay(200)
+
+			try {
+				// First open with original content
+				await cline.diffViewProvider.open(relPath)
+			} catch (openError) {
+				console.error(`[insertContentTool] Failed to open diff view for '${relPath}':`, openError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("insert_content", `Failed to open diff view: ${openError.message}`)
+				pushToolResult(
+					formatResponse.toolError(`Failed to open file editor for '${relPath}': ${openError.message}`),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
+
+			try {
+				await cline.diffViewProvider.update(fileContent, false)
+				cline.diffViewProvider.scrollToFirstDiff()
+				await delay(200)
+			} catch (updateError) {
+				console.error(`[insertContentTool] Failed to update diff view for '${relPath}':`, updateError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("insert_content", `Failed to update diff view: ${updateError.message}`)
+				pushToolResult(
+					formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
 		}
 
 		const diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent)
@@ -121,7 +145,21 @@ export async function insertContentTool(
 			return
 		}
 
-		await cline.diffViewProvider.update(updatedContent, true)
+		try {
+			await cline.diffViewProvider.update(updatedContent, true)
+		} catch (updateError) {
+			console.error(
+				`[insertContentTool] Failed to update diff view with new content for '${relPath}':`,
+				updateError,
+			)
+			cline.consecutiveMistakeCount++
+			cline.recordToolError("insert_content", `Failed to update diff view: ${updateError.message}`)
+			pushToolResult(
+				formatResponse.toolError(`Failed to update file editor for '${relPath}': ${updateError.message}`),
+			)
+			await cline.diffViewProvider.reset()
+			return
+		}
 
 		const completeMessage = JSON.stringify({
 			...sharedMessageProps,

src/core/tools/searchAndReplaceTool.ts

@@ -201,13 +201,53 @@ export async function searchAndReplaceTool(
 		// Show changes in diff view
 		if (!cline.diffViewProvider.isEditing) {
 			await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {})
-			await cline.diffViewProvider.open(validRelPath)
-			await cline.diffViewProvider.update(fileContent, false)
-			cline.diffViewProvider.scrollToFirstDiff()
-			await delay(200)
+
+			try {
+				await cline.diffViewProvider.open(validRelPath)
+			} catch (openError) {
+				console.error(`[searchAndReplaceTool] Failed to open diff view for '${validRelPath}':`, openError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("search_and_replace", `Failed to open diff view: ${openError.message}`)
+				pushToolResult(
+					formatResponse.toolError(`Failed to open file editor for '${validRelPath}': ${openError.message}`),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
+
+			try {
+				await cline.diffViewProvider.update(fileContent, false)
+				cline.diffViewProvider.scrollToFirstDiff()
+				await delay(200)
+			} catch (updateError) {
+				console.error(`[searchAndReplaceTool] Failed to update diff view for '${validRelPath}':`, updateError)
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("search_and_replace", `Failed to update diff view: ${updateError.message}`)
+				pushToolResult(
+					formatResponse.toolError(
+						`Failed to update file editor for '${validRelPath}': ${updateError.message}`,
+					),
+				)
+				await cline.diffViewProvider.reset()
+				return
+			}
 		}
 
-		await cline.diffViewProvider.update(newContent, true)
+		try {
+			await cline.diffViewProvider.update(newContent, true)
+		} catch (updateError) {
+			console.error(
+				`[searchAndReplaceTool] Failed to update diff view with new content for '${validRelPath}':`,
+				updateError,
+			)
+			cline.consecutiveMistakeCount++
+			cline.recordToolError("search_and_replace", `Failed to update diff view: ${updateError.message}`)
+			pushToolResult(
+				formatResponse.toolError(`Failed to update file editor for '${validRelPath}': ${updateError.message}`),
+			)
+			await cline.diffViewProvider.reset()
+			return
+		}
 
 		// Request user approval for changes
 		const completeMessage = JSON.stringify({

src/core/tools/writeToFileTool.ts

@@ -104,15 +104,29 @@ export async function writeToFileTool(
 
 			// update editor
 			if (!cline.diffViewProvider.isEditing) {
-				// open the editor and prepare to stream content in
-				await cline.diffViewProvider.open(relPath)
+				try {
+					// open the editor and prepare to stream content in
+					await cline.diffViewProvider.open(relPath)
+				} catch (openError) {
+					console.error(`[writeToFileTool] Failed to open diff view for '${relPath}':`, openError)
+					await handleError("writing file", openError)
+					await cline.diffViewProvider.reset()
+					return
+				}
 			}
 
-			// editor is open, stream content in
-			await cline.diffViewProvider.update(
-				everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent,
-				false,
-			)
+			try {
+				// editor is open, stream content in
+				await cline.diffViewProvider.update(
+					everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent,
+					false,
+				)
+			} catch (updateError) {
+				console.error(`[writeToFileTool] Failed to update diff view for '${relPath}':`, updateError)
+				await handleError("writing file", updateError)
+				await cline.diffViewProvider.reset()
+				return
+			}
 
 			return
 		} else {
@@ -155,13 +169,28 @@ export async function writeToFileTool(
 				// show gui message before showing edit animation
 				const partialMessage = JSON.stringify(sharedMessageProps)
 				await cline.ask("tool", partialMessage, true).catch(() => {}) // sending true for partial even though it's not a partial, cline shows the edit row before the content is streamed into the editor
-				await cline.diffViewProvider.open(relPath)
+
+				try {
+					await cline.diffViewProvider.open(relPath)
+				} catch (openError) {
+					console.error(`[writeToFileTool] Failed to open diff view for '${relPath}':`, openError)
+					await handleError("writing file", openError)
+					await cline.diffViewProvider.reset()
+					return
+				}
 			}
 
-			await cline.diffViewProvider.update(
-				everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent,
-				true,
-			)
+			try {
+				await cline.diffViewProvider.update(
+					everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent,
+					true,
+				)
+			} catch (updateError) {
+				console.error(`[writeToFileTool] Failed to update diff view for '${relPath}':`, updateError)
+				await handleError("writing file", updateError)
+				await cline.diffViewProvider.reset()
+				return
+			}
 
 			await delay(300) // wait for diff view to update
 			cline.diffViewProvider.scrollToFirstDiff()

src/integrations/editor/DiffViewProvider.ts

@@ -40,65 +40,93 @@ export class DiffViewProvider {
 		this.relPath = relPath
 		const fileExists = this.editType === "modify"
 		const absolutePath = path.resolve(this.cwd, relPath)
-		this.isEditing = true
 
-		// If the file is already open, ensure it's not dirty before getting its
-		// contents.
-		if (fileExists) {
-			const existingDocument = vscode.workspace.textDocuments.find((doc) =>
-				arePathsEqual(doc.uri.fsPath, absolutePath),
-			)
+		try {
+			this.isEditing = true
 
-			if (existingDocument && existingDocument.isDirty) {
-				await existingDocument.save()
+			// Validate the file path
+			if (!relPath || relPath.trim() === "") {
+				throw new Error("Invalid file path: path cannot be empty")
 			}
-		}
 
-		// Get diagnostics before editing the file, we'll compare to diagnostics
-		// after editing to see if cline needs to fix anything.
-		this.preDiagnostics = vscode.languages.getDiagnostics()
+			// If the file is already open, ensure it's not dirty before getting its
+			// contents.
+			if (fileExists) {
+				const existingDocument = vscode.workspace.textDocuments.find((doc) =>
+					arePathsEqual(doc.uri.fsPath, absolutePath),
+				)
 
-		if (fileExists) {
-			this.originalContent = await fs.readFile(absolutePath, "utf-8")
-		} else {
-			this.originalContent = ""
-		}
-
-		// For new files, create any necessary directories and keep track of new
-		// directories to delete if the user denies the operation.
-		this.createdDirs = await createDirectoriesForFile(absolutePath)
-
-		// Make sure the file exists before we open it.
-		if (!fileExists) {
-			await fs.writeFile(absolutePath, "")
-		}
-
-		// If the file was already open, close it (must happen after showing the
-		// diff view since if it's the only tab the column will close).
-		this.documentWasOpen = false
-
-		// Close the tab if it's open (it's already saved above).
-		const tabs = vscode.window.tabGroups.all
-			.map((tg) => tg.tabs)
-			.flat()
-			.filter(
-				(tab) => tab.input instanceof vscode.TabInputText && arePathsEqual(tab.input.uri.fsPath, absolutePath),
-			)
-
-		for (const tab of tabs) {
-			if (!tab.isDirty) {
-				await vscode.window.tabGroups.close(tab)
+				if (existingDocument && existingDocument.isDirty) {
+					await existingDocument.save()
+				}
 			}
-			this.documentWasOpen = true
-		}
 
-		this.activeDiffEditor = await this.openDiffEditor()
-		this.fadedOverlayController = new DecorationController("fadedOverlay", this.activeDiffEditor)
-		this.activeLineController = new DecorationController("activeLine", this.activeDiffEditor)
-		// Apply faded overlay to all lines initially.
-		this.fadedOverlayController.addLines(0, this.activeDiffEditor.document.lineCount)
-		this.scrollEditorToLine(0) // Will this crash for new files?
-		this.streamedLines = []
+			// Get diagnostics before editing the file, we'll compare to diagnostics
+			// after editing to see if cline needs to fix anything.
+			this.preDiagnostics = vscode.languages.getDiagnostics()
+
+			if (fileExists) {
+				try {
+					this.originalContent = await fs.readFile(absolutePath, "utf-8")
+				} catch (readError) {
+					throw new Error(`Failed to read existing file '${relPath}': ${readError.message}`)
+				}
+			} else {
+				this.originalContent = ""
+			}
+
+			// For new files, create any necessary directories and keep track of new
+			// directories to delete if the user denies the operation.
+			try {
+				this.createdDirs = await createDirectoriesForFile(absolutePath)
+			} catch (dirError) {
+				throw new Error(`Failed to create directories for '${relPath}': ${dirError.message}`)
+			}
+
+			// Make sure the file exists before we open it.
+			if (!fileExists) {
+				try {
+					await fs.writeFile(absolutePath, "")
+				} catch (writeError) {
+					throw new Error(`Failed to create new file '${relPath}': ${writeError.message}`)
+				}
+			}
+
+			// If the file was already open, close it (must happen after showing the
+			// diff view since if it's the only tab the column will close).
+			this.documentWasOpen = false
+
+			// Close the tab if it's open (it's already saved above).
+			const tabs = vscode.window.tabGroups.all
+				.map((tg) => tg.tabs)
+				.flat()
+				.filter(
+					(tab) =>
+						tab.input instanceof vscode.TabInputText && arePathsEqual(tab.input.uri.fsPath, absolutePath),
+				)
+
+			for (const tab of tabs) {
+				if (!tab.isDirty) {
+					await vscode.window.tabGroups.close(tab)
+				}
+				this.documentWasOpen = true
+			}
+
+			this.activeDiffEditor = await this.openDiffEditor()
+			this.fadedOverlayController = new DecorationController("fadedOverlay", this.activeDiffEditor)
+			this.activeLineController = new DecorationController("activeLine", this.activeDiffEditor)
+			// Apply faded overlay to all lines initially.
+			this.fadedOverlayController.addLines(0, this.activeDiffEditor.document.lineCount)
+			this.scrollEditorToLine(0) // Will this crash for new files?
+			this.streamedLines = []
+		} catch (error) {
+			// Reset state on error
+			this.isEditing = false
+			this.relPath = undefined
+			this.originalContent = undefined
+			this.createdDirs = []
+			throw error
+		}
 	}
 
 	async update(accumulatedContent: string, isFinal: boolean) {