From a3b34460a14a636d87728f0cf22eddbf4fb8073c Mon Sep 17 00:00:00 2001 From: Merge Resolver Date: Thu, 21 Aug 2025 18:39:44 -0600 Subject: [PATCH] feat: implement comprehensive error title extraction system - Created errorTitleExtractor utility with pattern matching for all error types - Handles MCP errors, file operations, tool errors, API errors, and embeddings errors - Extracts meaningful titles instead of generic 'Error' for better UX - Added comprehensive test suite with 44 test cases covering all error patterns - Updated ChatRow.tsx to use the new extraction utility This addresses the concern that error messages should display specific, meaningful titles rather than just 'Error' in the chat view. --- webview-ui/src/components/chat/ChatRow.tsx | 41 +-- .../__tests__/errorTitleExtractor.spec.ts | 247 ++++++++++++++++ webview-ui/src/utils/errorTitleExtractor.ts | 267 ++++++++++++++++++ 3 files changed, 517 insertions(+), 38 deletions(-) create mode 100644 webview-ui/src/utils/__tests__/errorTitleExtractor.spec.ts create mode 100644 webview-ui/src/utils/errorTitleExtractor.ts diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index db2145193d..95da901e65 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -20,6 +20,7 @@ import { findMatchingResourceOrTemplate } from "@src/utils/mcp" import { vscode } from "@src/utils/vscode" import { removeLeadingNonAlphanumeric } from "@src/utils/removeLeadingNonAlphanumeric" import { getLanguageFromPath } from "@src/utils/getLanguageFromPath" +import { extractErrorTitle } from "@src/utils/errorTitleExtractor" import { Button } from "@src/components/ui" import ChatTextArea from "./ChatTextArea" @@ -1089,45 +1090,9 @@ export const ChatRowContent = ({ ) case "error": { - // Extract error title from the message text - let errorTitle = t("chat:error") + // Extract error title from the message text using the comprehensive extractor const errorContent = message.text || "" - - // Common error patterns to extract meaningful titles from - const patterns = [ - // "Error reading file: File not found: /path/to/file" -> "File Not Found" - { regex: /^Error reading file:.*?(File not found):/i, title: "File Not Found" }, - // "Error reading file: Permission denied: /path/to/file" -> "Permission Denied" - { regex: /^Error reading file:.*?(Permission denied):/i, title: "Permission Denied" }, - // "Error reading file: " -> extract the error part - { regex: /^Error reading file:\s*(.+?)(?::|$)/i, extractTitle: true }, - // Generic "Title: rest of message" pattern - { regex: /^([^:]+):\s*/, extractTitle: true, maxLength: 50 }, - ] - - // Try each pattern to find a match - for (const pattern of patterns) { - const match = errorContent.match(pattern.regex) - if (match) { - if (pattern.title) { - // Use predefined title - errorTitle = pattern.title - } else if (pattern.extractTitle && match[1]) { - // Extract and clean up the title - let extracted = match[1].trim() - // Remove redundant "Error" prefix - extracted = extracted.replace(/^Error\s+/i, "").trim() - // Only use if it's a reasonable length - if ( - extracted.length > 0 && - (!pattern.maxLength || extracted.length <= pattern.maxLength) - ) { - errorTitle = extracted - } - } - break - } - } + const errorTitle = extractErrorTitle(errorContent, t) return (
diff --git a/webview-ui/src/utils/__tests__/errorTitleExtractor.spec.ts b/webview-ui/src/utils/__tests__/errorTitleExtractor.spec.ts new file mode 100644 index 0000000000..61604a79fb --- /dev/null +++ b/webview-ui/src/utils/__tests__/errorTitleExtractor.spec.ts @@ -0,0 +1,247 @@ +import { describe, it, expect, vi } from "vitest" +import { extractErrorTitle } from "../errorTitleExtractor" + +// Mock the translation function with proper typing +const mockT = vi.fn((key: string) => { + if (key === "chat:error") return "Error" + return key +}) as any // Cast to any to bypass TFunction type checking in tests + +describe("extractErrorTitle", () => { + describe("MCP Error Patterns", () => { + it("should extract title for invalid MCP settings format", () => { + const error = + "Invalid MCP settings JSON format. Please ensure your settings follow the correct JSON format." + expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Format") + }) + + it("should extract title for invalid MCP settings syntax", () => { + const error = "Invalid MCP settings JSON format. Please check your settings file for syntax errors." + expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Syntax") + }) + + it("should extract title for MCP settings validation error", () => { + const error = "Invalid MCP settings format: missing required field 'command'" + expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Validation") + }) + + it("should extract title for MCP configuration file error", () => { + const error = "Failed to create or open .roo/mcp.json: Permission denied" + expect(extractErrorTitle(error, mockT)).toBe("MCP Configuration File Error") + }) + + it("should extract title for MCP server update failure", () => { + const error = "Failed to update project MCP servers" + expect(extractErrorTitle(error, mockT)).toBe("MCP Server Update Failed") + }) + + it("should extract title for invalid tool arguments", () => { + const error = "Roo tried to use apply_diff with an invalid JSON argument. Retrying..." + expect(extractErrorTitle(error, mockT)).toBe("Invalid Tool Arguments") + }) + }) + + describe("File Operation Error Patterns", () => { + it("should extract title for file not found error", () => { + const error = "Error reading file: File not found: /path/to/file.txt" + expect(extractErrorTitle(error, mockT)).toBe("File Not Found") + }) + + it("should extract title for permission denied error", () => { + const error = "Error reading file: Permission denied: /path/to/file.txt" + expect(extractErrorTitle(error, mockT)).toBe("Permission Denied") + }) + + it("should extract title for generic file read error", () => { + const error = "Error reading file: Disk full" + expect(extractErrorTitle(error, mockT)).toBe("File Read Error: Disk full") + }) + + it("should extract title for file does not exist error", () => { + const error = "File does not exist at path: /path/to/file.txt" + expect(extractErrorTitle(error, mockT)).toBe("File Does Not Exist") + }) + + it("should extract title for insert into non-existent file error", () => { + const error = + "Cannot insert content at line 10 into a non-existent file. For new files, 'line' must be 0 (to append) or 1 (to insert at the beginning)." + expect(extractErrorTitle(error, mockT)).toBe("Cannot Insert Into Non-Existent File") + }) + + it("should extract title for parse operations error", () => { + const error = "Failed to parse operations: Invalid XML format" + expect(extractErrorTitle(error, mockT)).toBe("Invalid Operations Format") + }) + + it("should extract title for parse diff error", () => { + const error = "Failed to parse apply_diff XML: Unexpected token" + expect(extractErrorTitle(error, mockT)).toBe("Invalid Diff Format") + }) + }) + + describe("Tool-specific Error Patterns", () => { + it("should extract title for command execution failure", () => { + const error = "Failed to execute command: npm install" + expect(extractErrorTitle(error, mockT)).toBe("Command Execution Failed") + }) + + it("should extract title for command timeout", () => { + const error = "Command execution timed out after 30 seconds" + expect(extractErrorTitle(error, mockT)).toBe("Command Timeout") + }) + + it("should extract title for search and replace failure", () => { + const error = "Search and replace operation failed: Pattern not found" + expect(extractErrorTitle(error, mockT)).toBe("Search & Replace Failed") + }) + + it("should extract title for diff application failure", () => { + const error = "Failed to apply diff: Merge conflict" + expect(extractErrorTitle(error, mockT)).toBe("Diff Application Failed") + }) + }) + + describe("API and Service Error Patterns", () => { + it("should extract title for authentication failure", () => { + const error = "Authentication failed. Please check your API key." + expect(extractErrorTitle(error, mockT)).toBe("Authentication Failed") + }) + + it("should extract title for rate limit error", () => { + const error = "API rate limit exceeded. Please wait before making another request." + expect(extractErrorTitle(error, mockT)).toBe("Rate Limit Exceeded") + }) + + it("should extract title for API key mismatch", () => { + const error = "API key and subscription plan mismatch" + expect(extractErrorTitle(error, mockT)).toBe("API Key Mismatch") + }) + + it("should extract title for service unavailable", () => { + const error = "Service unavailable. Please try again later." + expect(extractErrorTitle(error, mockT)).toBe("Service Unavailable") + }) + + it("should extract title for network error", () => { + const error = "Network error: Unable to connect to server" + expect(extractErrorTitle(error, mockT)).toBe("Network Error") + }) + + it("should extract title for connection failure", () => { + const error = "Connection failed: Timeout" + expect(extractErrorTitle(error, mockT)).toBe("Connection Failed") + }) + }) + + describe("Embeddings and Indexing Error Patterns", () => { + it("should extract title for embeddings creation failure", () => { + const error = "Failed to create embeddings: Model not available" + expect(extractErrorTitle(error, mockT)).toBe("Embeddings Creation Failed") + }) + + it("should extract title for vector dimension mismatch", () => { + const error = "Vector dimension mismatch. Expected 1536, got 768" + expect(extractErrorTitle(error, mockT)).toBe("Vector Dimension Mismatch") + }) + + it("should extract title for Qdrant connection failure", () => { + const error = "Failed to connect to Qdrant vector database" + expect(extractErrorTitle(error, mockT)).toBe("Qdrant Connection Failed") + }) + + it("should extract title for workspace requirement", () => { + const error = "Indexing requires an open workspace folder" + expect(extractErrorTitle(error, mockT)).toBe("Workspace Required for Indexing") + }) + }) + + describe("Generic Error Patterns", () => { + it("should extract title from colon-separated format", () => { + const error = "Database Error: Connection lost" + expect(extractErrorTitle(error, mockT)).toBe("Database Error") + }) + + it("should extract title from Error: prefix format", () => { + const error = "Error: Invalid configuration - missing required fields" + expect(extractErrorTitle(error, mockT)).toBe("Invalid configuration") + }) + + it("should extract title from [ERROR] prefix format", () => { + const error = "[ERROR] Configuration not found" + expect(extractErrorTitle(error, mockT)).toBe("Configuration not found") + }) + + it("should use short error message as title", () => { + const error = "Invalid input" + expect(extractErrorTitle(error, mockT)).toBe("Invalid input") + }) + + it("should capitalize first letter of extracted title", () => { + const error = "validation failed" + expect(extractErrorTitle(error, mockT)).toBe("Validation failed") + }) + + it("should remove trailing period from title", () => { + const error = "Operation completed with errors." + expect(extractErrorTitle(error, mockT)).toBe("Operation completed with errors") + }) + }) + + describe("Edge Cases", () => { + it("should return default title for empty string", () => { + expect(extractErrorTitle("", mockT)).toBe("Error") + }) + + it("should return default title for null", () => { + expect(extractErrorTitle(null as any, mockT)).toBe("Error") + }) + + it("should return default title for undefined", () => { + expect(extractErrorTitle(undefined as any, mockT)).toBe("Error") + }) + + it("should return default title for non-string input", () => { + expect(extractErrorTitle(123 as any, mockT)).toBe("Error") + }) + + it("should handle very long error messages", () => { + const longError = + "This is a very long error message that exceeds the maximum length for a title and should fall back to the default error title instead of using the entire message as the title" + expect(extractErrorTitle(longError, mockT)).toBe("Error") + }) + + it("should handle error messages with only whitespace", () => { + expect(extractErrorTitle(" \n\t ", mockT)).toBe("Error") + }) + + it("should convert snake_case error keys to Title Case", () => { + const error = "Something went wrong with invalid_settings_format in the system" + expect(extractErrorTitle(error, mockT)).toBe("Invalid Settings Format") + }) + }) + + describe("Real-world Examples", () => { + it("should handle MCP tool error from actual code", () => { + const error = "Roo tried to use apply_diff with an invalid JSON argument. Retrying..." + expect(extractErrorTitle(error, mockT)).toBe("Invalid Tool Arguments") + }) + + it("should handle file not found error from actual code", () => { + const error = + "File does not exist at path: /Users/test/project/src/app.ts\n\n\nThe specified file could not be found. Please verify the file path and try again.\n" + expect(extractErrorTitle(error, mockT)).toBe("File Does Not Exist") + }) + + it("should handle complex error with multiple colons", () => { + const error = "Error: Failed to process: Invalid JSON: Unexpected token" + expect(extractErrorTitle(error, mockT)).toBe("Failed to process") + }) + + it("should handle error with HTML/XML tags", () => { + const error = "Error reading file: Access denied" + expect(extractErrorTitle(error, mockT)).toBe( + "File Read Error: Access denied", + ) + }) + }) +}) diff --git a/webview-ui/src/utils/errorTitleExtractor.ts b/webview-ui/src/utils/errorTitleExtractor.ts new file mode 100644 index 0000000000..2bacacf951 --- /dev/null +++ b/webview-ui/src/utils/errorTitleExtractor.ts @@ -0,0 +1,267 @@ +import { TFunction } from "i18next" + +/** + * Extracts a meaningful title from an error message. + * This function handles various error formats including MCP errors, + * file operation errors, and other structured error messages. + */ +export function extractErrorTitle(errorContent: string, t: TFunction): string { + // Default fallback title + const defaultTitle = t("chat:error") + + if (!errorContent || typeof errorContent !== "string") { + return defaultTitle + } + + // Clean up the error content + const trimmedContent = errorContent.trim() + + // Define the type for error patterns + type ErrorPattern = { + pattern: RegExp + title?: string + extractTitle?: boolean + prefix?: string + } + + // MCP-specific error patterns with their corresponding titles + const mcpErrorPatterns: ErrorPattern[] = [ + { + pattern: /Invalid MCP settings JSON format.*Please ensure your settings follow the correct JSON format/i, + title: "Invalid MCP Settings Format", + }, + { + pattern: /Invalid MCP settings JSON format.*Please check your settings file for syntax errors/i, + title: "Invalid MCP Settings Syntax", + }, + { + pattern: /Invalid MCP settings format:/i, + title: "Invalid MCP Settings Validation", + }, + { + pattern: /Failed to create or open \.roo\/mcp\.json:/i, + title: "MCP Configuration File Error", + }, + { + pattern: /Failed to update project MCP servers/i, + title: "MCP Server Update Failed", + }, + { + pattern: /Roo tried to use .+ with an invalid JSON argument/i, + title: "Invalid Tool Arguments", + }, + ] + + // File operation error patterns + const fileErrorPatterns: ErrorPattern[] = [ + { + pattern: /^Error reading file:.*?(File not found):/i, + title: "File Not Found", + }, + { + pattern: /^Error reading file:.*?(Permission denied):/i, + title: "Permission Denied", + }, + { + pattern: /^Error reading file:\s*(.+?)(?::|$)/i, + extractTitle: true, + prefix: "File Read Error", + }, + { + pattern: /^File does not exist at path:/i, + title: "File Does Not Exist", + }, + { + pattern: /^Cannot insert content at line \d+ into a non-existent file/i, + title: "Cannot Insert Into Non-Existent File", + }, + { + pattern: /^Failed to parse operations:/i, + title: "Invalid Operations Format", + }, + { + pattern: /^Failed to parse apply_diff XML:/i, + title: "Invalid Diff Format", + }, + ] + + // Tool-specific error patterns + const toolErrorPatterns: ErrorPattern[] = [ + { + pattern: /^Failed to execute command:/i, + title: "Command Execution Failed", + }, + { + pattern: /^Command execution timed out/i, + title: "Command Timeout", + }, + { + pattern: /^Search and replace operation failed:/i, + title: "Search & Replace Failed", + }, + { + pattern: /^Failed to apply diff:/i, + title: "Diff Application Failed", + }, + ] + + // API and service error patterns + const apiErrorPatterns: ErrorPattern[] = [ + { + pattern: /^Authentication failed/i, + title: "Authentication Failed", + }, + { + pattern: /^API rate limit exceeded/i, + title: "Rate Limit Exceeded", + }, + { + pattern: /^API key.*mismatch/i, + title: "API Key Mismatch", + }, + { + pattern: /^Service unavailable/i, + title: "Service Unavailable", + }, + { + pattern: /^Network error/i, + title: "Network Error", + }, + { + pattern: /^Connection failed/i, + title: "Connection Failed", + }, + ] + + // Embeddings and indexing error patterns + const embeddingErrorPatterns: ErrorPattern[] = [ + { + pattern: /^Failed to create embeddings:/i, + title: "Embeddings Creation Failed", + }, + { + pattern: /^Vector dimension mismatch/i, + title: "Vector Dimension Mismatch", + }, + { + pattern: /^Failed to connect to Qdrant/i, + title: "Qdrant Connection Failed", + }, + { + pattern: /^Indexing requires an open workspace/i, + title: "Workspace Required for Indexing", + }, + ] + + // Combine all pattern groups + const allPatterns = [ + ...mcpErrorPatterns, + ...fileErrorPatterns, + ...toolErrorPatterns, + ...apiErrorPatterns, + ...embeddingErrorPatterns, + ] + + // Try to match against specific patterns first + for (const pattern of allPatterns) { + const match = trimmedContent.match(pattern.pattern) + if (match) { + if (pattern.title) { + return pattern.title + } else if (pattern.extractTitle && match[1]) { + // Extract and clean up the title from the match + let extracted = match[1].trim() + // Remove redundant "Error" prefix if present + extracted = extracted.replace(/^Error\s+/i, "").trim() + // Apply prefix if specified + if (pattern.prefix && extracted) { + return `${pattern.prefix}: ${extracted}` + } + // Only use extracted title if it's reasonable length + if (extracted.length > 0 && extracted.length <= 50) { + return extracted + } + } + } + } + + // Generic patterns for common error formats + const genericPatterns = [ + // "Error: Title - rest of message" or "Error: Title: rest" pattern + // This should be checked first to handle "Error:" prefix specially + // Use non-greedy match to stop at first colon or dash + { + regex: /^Error:\s*([^-:]{3,50}?)(?:[-:]|$)/i, + extractTitle: true, + }, + // "[ERROR] Title" pattern + { + regex: /^\[ERROR\]\s*([^:]{3,50})/i, + extractTitle: true, + }, + // Generic "Title: rest of message" pattern (checked last) + { + regex: /^([^:]{3,50}):\s*/, + extractTitle: true, + }, + ] + + // Try generic patterns + for (const pattern of genericPatterns) { + const match = trimmedContent.match(pattern.regex) + if (match && pattern.extractTitle && match[1]) { + let extracted = match[1].trim() + // Remove redundant "Error" prefix + extracted = extracted.replace(/^Error\s+/i, "").trim() + // Capitalize first letter + if (extracted.length > 0) { + extracted = extracted.charAt(0).toUpperCase() + extracted.slice(1) + return extracted + } + } + } + + // If the error message is short enough, use it as the title + // (but clean it up first) + if (trimmedContent.length <= 50) { + // Remove common prefixes + let cleaned = trimmedContent + .replace(/^Error:\s*/i, "") + .replace(/^Failed:\s*/i, "") + .replace(/^Warning:\s*/i, "") + .trim() + + // Capitalize first letter + if (cleaned.length > 0) { + cleaned = cleaned.charAt(0).toUpperCase() + cleaned.slice(1) + // If it ends with a period, remove it for the title + cleaned = cleaned.replace(/\.$/, "") + return cleaned + } + } + + // Check if it's a known error type from the message structure + // This handles cases where the error might be a key from i18n + const knownErrorKeys = [ + "invalid_settings_format", + "invalid_settings_syntax", + "invalid_settings_validation", + "create_json", + "failed_update_project", + "invalidJsonArgument", + ] + + for (const key of knownErrorKeys) { + if (trimmedContent.includes(key)) { + // Convert snake_case to Title Case + const title = key + .split("_") + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(" ") + return title + } + } + + // Default fallback + return defaultTitle +}