fix: improve MCP tool error handling with retry logic and better messages

- Add retry logic with exponential backoff (up to 3 retries) for transient failures
- Implement comprehensive error handling with user-friendly messages
- Add parameter validation before MCP tool execution
- Enhance error messages to provide actionable guidance
- Check server connection status before attempting tool calls
- Add localized error messages for better user experience

Fixes #6189

Author: roomote

src/core/tools/__tests__/useMcpToolTool.spec.ts

@@ -136,8 +136,13 @@ 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(mockPushToolResult).toHaveBeenCalledWith("Tool error: Invalid args for test_server:test_tool")
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("Invalid JSON arguments for tool 'test_tool'"),
+			)
+			expect(mockPushToolResult).toHaveBeenCalledWith(
+				expect.stringContaining("Invalid JSON arguments for test_server.test_tool"),
+			)
 		})
 	})
 

src/core/tools/useMcpToolTool.ts

@@ -54,29 +54,54 @@ async function validateParams(
 		return { isValid: false }
 	}
 
+	// Validate server name format
+	const serverName = params.server_name.trim()
+	if (!serverName) {
+		cline.consecutiveMistakeCount++
+		cline.recordToolError("use_mcp_tool")
+		await cline.say("error", "Server name cannot be empty or contain only whitespace")
+		pushToolResult(formatResponse.toolError("Invalid server name: cannot be empty"))
+		return { isValid: false }
+	}
+
+	// Validate tool name format
+	const toolName = params.tool_name.trim()
+	if (!toolName) {
+		cline.consecutiveMistakeCount++
+		cline.recordToolError("use_mcp_tool")
+		await cline.say("error", "Tool name cannot be empty or contain only whitespace")
+		pushToolResult(formatResponse.toolError("Invalid tool name: cannot be empty"))
+		return { isValid: false }
+	}
+
 	let parsedArguments: Record<string, unknown> | undefined
 
 	if (params.arguments) {
 		try {
 			parsedArguments = JSON.parse(params.arguments)
+
+			// Validate that arguments is an object (not array or primitive)
+			if ((parsedArguments !== null && typeof parsedArguments !== "object") || Array.isArray(parsedArguments)) {
+				throw new Error("Arguments must be a JSON object, not an array or primitive value")
+			}
 		} catch (error) {
 			cline.consecutiveMistakeCount++
 			cline.recordToolError("use_mcp_tool")
-			await cline.say("error", t("mcp:errors.invalidJsonArgument", { toolName: params.tool_name }))
+
+			const errorMessage = error instanceof Error ? error.message : "Invalid JSON"
+			await cline.say("error", `Invalid JSON arguments for tool '${toolName}': ${errorMessage}`)
 
 			pushToolResult(
-				formatResponse.toolError(
-					formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name),
-				),
+				formatResponse.toolError(`Invalid JSON arguments for ${serverName}.${toolName}: ${errorMessage}`),
 			)
 			return { isValid: false }
 		}
 	}
 
 	return {
 		isValid: true,
-		serverName: params.server_name,
-		toolName: params.tool_name,
+		serverName,
+		toolName,
 		parsedArguments,
 	}
 }
@@ -127,41 +152,74 @@ async function executeToolAndProcessResult(
 		toolName,
 	})
 
-	const toolResult = await cline.providerRef.deref()?.getMcpHub()?.callTool(serverName, toolName, parsedArguments)
+	let retryCount = 0
+	const maxRetries = 3
+	let lastError: Error | null = null
 
-	let toolResultPretty = "(No response)"
+	while (retryCount <= maxRetries) {
+		try {
+			const mcpHub = cline.providerRef.deref()?.getMcpHub()
+			if (!mcpHub) {
+				throw new Error("MCP Hub is not available. Please ensure MCP servers are properly configured.")
+			}
 
-	if (toolResult) {
-		const outputText = processToolContent(toolResult)
+			const toolResult = await mcpHub.callTool(serverName, toolName, parsedArguments)
 
-		if (outputText) {
-			await sendExecutionStatus(cline, {
-				executionId,
-				status: "output",
-				response: outputText,
-			})
+			if (toolResult) {
+				const outputText = processToolContent(toolResult)
 
-			toolResultPretty = (toolResult.isError ? "Error:\n" : "") + outputText
-		}
+				if (outputText) {
+					await sendExecutionStatus(cline, {
+						executionId,
+						status: "output",
+						response: outputText,
+					})
 
-		// Send completion status
-		await sendExecutionStatus(cline, {
-			executionId,
-			status: toolResult.isError ? "error" : "completed",
-			response: toolResultPretty,
-			error: toolResult.isError ? "Error executing MCP tool" : undefined,
-		})
-	} else {
-		// Send error status if no result
-		await sendExecutionStatus(cline, {
-			executionId,
-			status: "error",
-			error: "No response from MCP server",
-		})
+					const toolResultPretty = (toolResult.isError ? "Error:\n" : "") + outputText
+
+					// Send completion status
+					await sendExecutionStatus(cline, {
+						executionId,
+						status: toolResult.isError ? "error" : "completed",
+						response: toolResultPretty,
+						error: toolResult.isError ? outputText : undefined,
+					})
+
+					await cline.say("mcp_server_response", toolResultPretty)
+					pushToolResult(formatResponse.toolResult(toolResultPretty))
+					return
+				}
+			}
+
+			// If we get here, toolResult was null/undefined
+			throw new Error(`No response received from MCP server '${serverName}' for tool '${toolName}'`)
+		} catch (error) {
+			lastError = error instanceof Error ? error : new Error(String(error))
+			retryCount++
+
+			if (retryCount <= maxRetries) {
+				const delay = Math.min(1000 * Math.pow(2, retryCount - 1), 5000) // Exponential backoff with max 5s
+				await cline.say(
+					"error",
+					`MCP tool execution failed (attempt ${retryCount}/${maxRetries}). Retrying in ${delay / 1000}s...`,
+				)
+				await new Promise((resolve) => setTimeout(resolve, delay))
+			}
+		}
 	}
 
-	await cline.say("mcp_server_response", toolResultPretty)
-	pushToolResult(formatResponse.toolResult(toolResultPretty))
+	// All retries failed
+	const errorMessage = lastError?.message || "Unknown error occurred"
+	const userFriendlyError = `Failed to execute MCP tool '${toolName}' on server '${serverName}' after ${maxRetries} attempts. ${errorMessage}`
+
+	await sendExecutionStatus(cline, {
+		executionId,
+		status: "error",
+		error: userFriendlyError,
+	})
+
+	await cline.say("mcp_server_response", `Error: ${userFriendlyError}`)
+	pushToolResult(formatResponse.toolError(userFriendlyError))
 }
 
 export async function useMcpToolTool(

src/i18n/locales/en/mcp.json

@@ -5,7 +5,9 @@
 		"invalid_settings_validation": "Invalid MCP settings format: {{errorMessages}}",
 		"create_json": "Failed to create or open .roo/mcp.json: {{error}}",
 		"failed_update_project": "Failed to update project MCP servers",
-		"invalidJsonArgument": "Roo tried to use {{toolName}} with an invalid JSON argument. Retrying..."
+		"invalidJsonArgument": "Roo tried to use {{toolName}} with an invalid JSON argument. Retrying...",
+		"connection_failed": "Failed to connect to MCP server '{{serverName}}'. Please check your server configuration and ensure the server is running.",
+		"tool_execution_failed": "Failed to execute MCP tool '{{toolName}}' on server '{{serverName}}': {{error}}"
 	},
 	"info": {
 		"server_restarting": "Restarting {{serverName}} MCP server...",

src/services/mcp/McpHub.ts

@@ -1464,12 +1464,39 @@ export class McpHub {
 	): Promise<McpToolCallResponse> {
 		const connection = this.findConnection(serverName, source)
 		if (!connection) {
+			// Provide helpful suggestions
+			const availableServers = this.getAllServers()
+				.filter((s) => !s.disabled)
+				.map((s) => s.name)
+				.join(", ")
+
+			const errorMsg =
+				availableServers.length > 0
+					? `MCP server '${serverName}' not found. Available servers: ${availableServers}`
+					: `No MCP servers are currently connected. Please configure MCP servers in your settings.`
+
+			throw new Error(errorMsg)
+		}
+
+		if (connection.server.disabled) {
 			throw new Error(
-				`No connection found for server: ${serverName}${source ? ` with source ${source}` : ""}. Please make sure to use MCP servers available under 'Connected MCP Servers'.`,
+				`MCP server '${serverName}' is currently disabled. Please enable it in the MCP settings to use it.`,
 			)
 		}
-		if (connection.server.disabled) {
-			throw new Error(`Server "${serverName}" is disabled and cannot be used`)
+
+		// Check if server is connected
+		if (connection.server.status !== "connected") {
+			throw new Error(
+				`MCP server '${serverName}' is not connected (status: ${connection.server.status}). Please check the server configuration and logs.`,
+			)
+		}
+
+		// Validate tool exists
+		const availableTools = connection.server.tools || []
+		const toolExists = availableTools.some((t) => t.name === toolName)
+		if (!toolExists && availableTools.length > 0) {
+			const toolNames = availableTools.map((t) => t.name).join(", ")
+			throw new Error(`Tool '${toolName}' not found on server '${serverName}'. Available tools: ${toolNames}`)
 		}
 
 		let timeout: number
@@ -1482,19 +1509,44 @@ export class McpHub {
 			timeout = 60 * 1000
 		}
 
-		return await connection.client.request(
-			{
-				method: "tools/call",
-				params: {
-					name: toolName,
-					arguments: toolArguments,
+		try {
+			const result = await connection.client.request(
+				{
+					method: "tools/call",
+					params: {
+						name: toolName,
+						arguments: toolArguments,
+					},
 				},
-			},
-			CallToolResultSchema,
-			{
-				timeout,
-			},
-		)
+				CallToolResultSchema,
+				{
+					timeout,
+				},
+			)
+
+			return result
+		} catch (error) {
+			// Enhance error messages for common issues
+			if (error instanceof Error) {
+				if (error.message.includes("timeout")) {
+					throw new Error(
+						`MCP tool '${toolName}' on server '${serverName}' timed out after ${timeout / 1000} seconds. The tool may be taking too long to respond or the server may be unresponsive.`,
+					)
+				}
+				if (error.message.includes("ECONNREFUSED")) {
+					throw new Error(
+						`Cannot connect to MCP server '${serverName}'. The server may not be running or may be configured incorrectly.`,
+					)
+				}
+				if (error.message.includes("EPIPE") || error.message.includes("ENOTCONN")) {
+					throw new Error(
+						`Lost connection to MCP server '${serverName}'. The server may have crashed or been terminated.`,
+					)
+				}
+			}
+			// Re-throw with original error if not a known issue
+			throw error
+		}
 	}
 
 	/**

src/services/mcp/__tests__/McpHub.spec.ts

@@ -545,7 +545,7 @@ describe("McpHub", () => {
 			mcpHub.connections = [mockConnection]
 
 			await expect(mcpHub.callTool("disabled-server", "some-tool", {})).rejects.toThrow(
-				'Server "disabled-server" is disabled and cannot be used',
+				"MCP server 'disabled-server' is currently disabled. Please enable it in the MCP settings to use it.",
 			)
 		})
 
@@ -610,7 +610,7 @@ describe("McpHub", () => {
 
 		it("should throw error if server not found", async () => {
 			await expect(mcpHub.callTool("non-existent-server", "some-tool", {})).rejects.toThrow(
-				"No connection found for server: non-existent-server",
+				"No MCP servers are currently connected. Please configure MCP servers in your settings.",
 			)
 		})