Skip to content

Commit a3b3446

Browse files
author
Merge Resolver
committed
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.
1 parent 8fcd218 commit a3b3446

File tree

3 files changed

+517
-38
lines changed

3 files changed

+517
-38
lines changed

webview-ui/src/components/chat/ChatRow.tsx

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { findMatchingResourceOrTemplate } from "@src/utils/mcp"
2020
import { vscode } from "@src/utils/vscode"
2121
import { removeLeadingNonAlphanumeric } from "@src/utils/removeLeadingNonAlphanumeric"
2222
import { getLanguageFromPath } from "@src/utils/getLanguageFromPath"
23+
import { extractErrorTitle } from "@src/utils/errorTitleExtractor"
2324
import { Button } from "@src/components/ui"
2425

2526
import ChatTextArea from "./ChatTextArea"
@@ -1089,45 +1090,9 @@ export const ChatRowContent = ({
10891090
</div>
10901091
)
10911092
case "error": {
1092-
// Extract error title from the message text
1093-
let errorTitle = t("chat:error")
1093+
// Extract error title from the message text using the comprehensive extractor
10941094
const errorContent = message.text || ""
1095-
1096-
// Common error patterns to extract meaningful titles from
1097-
const patterns = [
1098-
// "Error reading file: File not found: /path/to/file" -> "File Not Found"
1099-
{ regex: /^Error reading file:.*?(File not found):/i, title: "File Not Found" },
1100-
// "Error reading file: Permission denied: /path/to/file" -> "Permission Denied"
1101-
{ regex: /^Error reading file:.*?(Permission denied):/i, title: "Permission Denied" },
1102-
// "Error reading file: <any other error>" -> extract the error part
1103-
{ regex: /^Error reading file:\s*(.+?)(?::|$)/i, extractTitle: true },
1104-
// Generic "Title: rest of message" pattern
1105-
{ regex: /^([^:]+):\s*/, extractTitle: true, maxLength: 50 },
1106-
]
1107-
1108-
// Try each pattern to find a match
1109-
for (const pattern of patterns) {
1110-
const match = errorContent.match(pattern.regex)
1111-
if (match) {
1112-
if (pattern.title) {
1113-
// Use predefined title
1114-
errorTitle = pattern.title
1115-
} else if (pattern.extractTitle && match[1]) {
1116-
// Extract and clean up the title
1117-
let extracted = match[1].trim()
1118-
// Remove redundant "Error" prefix
1119-
extracted = extracted.replace(/^Error\s+/i, "").trim()
1120-
// Only use if it's a reasonable length
1121-
if (
1122-
extracted.length > 0 &&
1123-
(!pattern.maxLength || extracted.length <= pattern.maxLength)
1124-
) {
1125-
errorTitle = extracted
1126-
}
1127-
}
1128-
break
1129-
}
1130-
}
1095+
const errorTitle = extractErrorTitle(errorContent, t)
11311096

11321097
return (
11331098
<div>
Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
import { describe, it, expect, vi } from "vitest"
2+
import { extractErrorTitle } from "../errorTitleExtractor"
3+
4+
// Mock the translation function with proper typing
5+
const mockT = vi.fn((key: string) => {
6+
if (key === "chat:error") return "Error"
7+
return key
8+
}) as any // Cast to any to bypass TFunction type checking in tests
9+
10+
describe("extractErrorTitle", () => {
11+
describe("MCP Error Patterns", () => {
12+
it("should extract title for invalid MCP settings format", () => {
13+
const error =
14+
"Invalid MCP settings JSON format. Please ensure your settings follow the correct JSON format."
15+
expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Format")
16+
})
17+
18+
it("should extract title for invalid MCP settings syntax", () => {
19+
const error = "Invalid MCP settings JSON format. Please check your settings file for syntax errors."
20+
expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Syntax")
21+
})
22+
23+
it("should extract title for MCP settings validation error", () => {
24+
const error = "Invalid MCP settings format: missing required field 'command'"
25+
expect(extractErrorTitle(error, mockT)).toBe("Invalid MCP Settings Validation")
26+
})
27+
28+
it("should extract title for MCP configuration file error", () => {
29+
const error = "Failed to create or open .roo/mcp.json: Permission denied"
30+
expect(extractErrorTitle(error, mockT)).toBe("MCP Configuration File Error")
31+
})
32+
33+
it("should extract title for MCP server update failure", () => {
34+
const error = "Failed to update project MCP servers"
35+
expect(extractErrorTitle(error, mockT)).toBe("MCP Server Update Failed")
36+
})
37+
38+
it("should extract title for invalid tool arguments", () => {
39+
const error = "Roo tried to use apply_diff with an invalid JSON argument. Retrying..."
40+
expect(extractErrorTitle(error, mockT)).toBe("Invalid Tool Arguments")
41+
})
42+
})
43+
44+
describe("File Operation Error Patterns", () => {
45+
it("should extract title for file not found error", () => {
46+
const error = "Error reading file: File not found: /path/to/file.txt"
47+
expect(extractErrorTitle(error, mockT)).toBe("File Not Found")
48+
})
49+
50+
it("should extract title for permission denied error", () => {
51+
const error = "Error reading file: Permission denied: /path/to/file.txt"
52+
expect(extractErrorTitle(error, mockT)).toBe("Permission Denied")
53+
})
54+
55+
it("should extract title for generic file read error", () => {
56+
const error = "Error reading file: Disk full"
57+
expect(extractErrorTitle(error, mockT)).toBe("File Read Error: Disk full")
58+
})
59+
60+
it("should extract title for file does not exist error", () => {
61+
const error = "File does not exist at path: /path/to/file.txt"
62+
expect(extractErrorTitle(error, mockT)).toBe("File Does Not Exist")
63+
})
64+
65+
it("should extract title for insert into non-existent file error", () => {
66+
const error =
67+
"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)."
68+
expect(extractErrorTitle(error, mockT)).toBe("Cannot Insert Into Non-Existent File")
69+
})
70+
71+
it("should extract title for parse operations error", () => {
72+
const error = "Failed to parse operations: Invalid XML format"
73+
expect(extractErrorTitle(error, mockT)).toBe("Invalid Operations Format")
74+
})
75+
76+
it("should extract title for parse diff error", () => {
77+
const error = "Failed to parse apply_diff XML: Unexpected token"
78+
expect(extractErrorTitle(error, mockT)).toBe("Invalid Diff Format")
79+
})
80+
})
81+
82+
describe("Tool-specific Error Patterns", () => {
83+
it("should extract title for command execution failure", () => {
84+
const error = "Failed to execute command: npm install"
85+
expect(extractErrorTitle(error, mockT)).toBe("Command Execution Failed")
86+
})
87+
88+
it("should extract title for command timeout", () => {
89+
const error = "Command execution timed out after 30 seconds"
90+
expect(extractErrorTitle(error, mockT)).toBe("Command Timeout")
91+
})
92+
93+
it("should extract title for search and replace failure", () => {
94+
const error = "Search and replace operation failed: Pattern not found"
95+
expect(extractErrorTitle(error, mockT)).toBe("Search & Replace Failed")
96+
})
97+
98+
it("should extract title for diff application failure", () => {
99+
const error = "Failed to apply diff: Merge conflict"
100+
expect(extractErrorTitle(error, mockT)).toBe("Diff Application Failed")
101+
})
102+
})
103+
104+
describe("API and Service Error Patterns", () => {
105+
it("should extract title for authentication failure", () => {
106+
const error = "Authentication failed. Please check your API key."
107+
expect(extractErrorTitle(error, mockT)).toBe("Authentication Failed")
108+
})
109+
110+
it("should extract title for rate limit error", () => {
111+
const error = "API rate limit exceeded. Please wait before making another request."
112+
expect(extractErrorTitle(error, mockT)).toBe("Rate Limit Exceeded")
113+
})
114+
115+
it("should extract title for API key mismatch", () => {
116+
const error = "API key and subscription plan mismatch"
117+
expect(extractErrorTitle(error, mockT)).toBe("API Key Mismatch")
118+
})
119+
120+
it("should extract title for service unavailable", () => {
121+
const error = "Service unavailable. Please try again later."
122+
expect(extractErrorTitle(error, mockT)).toBe("Service Unavailable")
123+
})
124+
125+
it("should extract title for network error", () => {
126+
const error = "Network error: Unable to connect to server"
127+
expect(extractErrorTitle(error, mockT)).toBe("Network Error")
128+
})
129+
130+
it("should extract title for connection failure", () => {
131+
const error = "Connection failed: Timeout"
132+
expect(extractErrorTitle(error, mockT)).toBe("Connection Failed")
133+
})
134+
})
135+
136+
describe("Embeddings and Indexing Error Patterns", () => {
137+
it("should extract title for embeddings creation failure", () => {
138+
const error = "Failed to create embeddings: Model not available"
139+
expect(extractErrorTitle(error, mockT)).toBe("Embeddings Creation Failed")
140+
})
141+
142+
it("should extract title for vector dimension mismatch", () => {
143+
const error = "Vector dimension mismatch. Expected 1536, got 768"
144+
expect(extractErrorTitle(error, mockT)).toBe("Vector Dimension Mismatch")
145+
})
146+
147+
it("should extract title for Qdrant connection failure", () => {
148+
const error = "Failed to connect to Qdrant vector database"
149+
expect(extractErrorTitle(error, mockT)).toBe("Qdrant Connection Failed")
150+
})
151+
152+
it("should extract title for workspace requirement", () => {
153+
const error = "Indexing requires an open workspace folder"
154+
expect(extractErrorTitle(error, mockT)).toBe("Workspace Required for Indexing")
155+
})
156+
})
157+
158+
describe("Generic Error Patterns", () => {
159+
it("should extract title from colon-separated format", () => {
160+
const error = "Database Error: Connection lost"
161+
expect(extractErrorTitle(error, mockT)).toBe("Database Error")
162+
})
163+
164+
it("should extract title from Error: prefix format", () => {
165+
const error = "Error: Invalid configuration - missing required fields"
166+
expect(extractErrorTitle(error, mockT)).toBe("Invalid configuration")
167+
})
168+
169+
it("should extract title from [ERROR] prefix format", () => {
170+
const error = "[ERROR] Configuration not found"
171+
expect(extractErrorTitle(error, mockT)).toBe("Configuration not found")
172+
})
173+
174+
it("should use short error message as title", () => {
175+
const error = "Invalid input"
176+
expect(extractErrorTitle(error, mockT)).toBe("Invalid input")
177+
})
178+
179+
it("should capitalize first letter of extracted title", () => {
180+
const error = "validation failed"
181+
expect(extractErrorTitle(error, mockT)).toBe("Validation failed")
182+
})
183+
184+
it("should remove trailing period from title", () => {
185+
const error = "Operation completed with errors."
186+
expect(extractErrorTitle(error, mockT)).toBe("Operation completed with errors")
187+
})
188+
})
189+
190+
describe("Edge Cases", () => {
191+
it("should return default title for empty string", () => {
192+
expect(extractErrorTitle("", mockT)).toBe("Error")
193+
})
194+
195+
it("should return default title for null", () => {
196+
expect(extractErrorTitle(null as any, mockT)).toBe("Error")
197+
})
198+
199+
it("should return default title for undefined", () => {
200+
expect(extractErrorTitle(undefined as any, mockT)).toBe("Error")
201+
})
202+
203+
it("should return default title for non-string input", () => {
204+
expect(extractErrorTitle(123 as any, mockT)).toBe("Error")
205+
})
206+
207+
it("should handle very long error messages", () => {
208+
const longError =
209+
"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"
210+
expect(extractErrorTitle(longError, mockT)).toBe("Error")
211+
})
212+
213+
it("should handle error messages with only whitespace", () => {
214+
expect(extractErrorTitle(" \n\t ", mockT)).toBe("Error")
215+
})
216+
217+
it("should convert snake_case error keys to Title Case", () => {
218+
const error = "Something went wrong with invalid_settings_format in the system"
219+
expect(extractErrorTitle(error, mockT)).toBe("Invalid Settings Format")
220+
})
221+
})
222+
223+
describe("Real-world Examples", () => {
224+
it("should handle MCP tool error from actual code", () => {
225+
const error = "Roo tried to use apply_diff with an invalid JSON argument. Retrying..."
226+
expect(extractErrorTitle(error, mockT)).toBe("Invalid Tool Arguments")
227+
})
228+
229+
it("should handle file not found error from actual code", () => {
230+
const error =
231+
"File does not exist at path: /Users/test/project/src/app.ts\n\n<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>"
232+
expect(extractErrorTitle(error, mockT)).toBe("File Does Not Exist")
233+
})
234+
235+
it("should handle complex error with multiple colons", () => {
236+
const error = "Error: Failed to process: Invalid JSON: Unexpected token"
237+
expect(extractErrorTitle(error, mockT)).toBe("Failed to process")
238+
})
239+
240+
it("should handle error with HTML/XML tags", () => {
241+
const error = "Error reading file: <permission_denied>Access denied</permission_denied>"
242+
expect(extractErrorTitle(error, mockT)).toBe(
243+
"File Read Error: <permission_denied>Access denied</permission_denied>",
244+
)
245+
})
246+
})
247+
})

0 commit comments

Comments
 (0)