fix: address review feedback for nested try-catch pattern and add test coverage

- Remove nested try-catch pattern in src/core/mentions/index.ts for consistency
- Add test coverage for RangeError handling in readFileTool
- Ensure graceful error handling when isBinaryFile throws RangeError

Author: roomote

src/core/mentions/index.ts

@@ -267,18 +267,18 @@ async function getFileOrFolderContent(
 						const absoluteFilePath = path.resolve(absPath, entry.name)
 						fileContentPromises.push(
 							(async () => {
+								let isBinary = false
+								try {
+									isBinary = await isBinaryFile(absoluteFilePath)
+								} catch (error) {
+									// If isBinaryFile throws an error (e.g., RangeError), treat as binary
+									console.warn(`Error checking if file is binary for ${absoluteFilePath}:`, error)
+									isBinary = true
+								}
+								if (isBinary) {
+									return undefined
+								}
 								try {
-									let isBinary = false
-									try {
-										isBinary = await isBinaryFile(absoluteFilePath)
-									} catch (error) {
-										// If isBinaryFile throws an error (e.g., RangeError), treat as binary
-										console.warn(`Error checking if file is binary for ${absoluteFilePath}:`, error)
-										isBinary = true
-									}
-									if (isBinary) {
-										return undefined
-									}
 									const content = await extractTextFromFile(absoluteFilePath, maxReadFileLine)
 									return `<file_content path="${filePath.toPosix()}">\n${content}\n</file_content>`
 								} catch (error) {

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

@@ -518,5 +518,26 @@ describe("read_file tool XML output structure", () => {
 				`<files>\n<file><path>${testFilePath}</path><error>Access to ${testFilePath} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.</error></file>\n</files>`,
 			)
 		})
+
+		it("should handle RangeError from isBinaryFile gracefully", async () => {
+			// Setup - mock isBinaryFile to throw RangeError
+			mockedIsBinaryFile.mockRejectedValue(new RangeError("Invalid array length"))
+			mockedCountFileLines.mockResolvedValue(5)
+
+			// Execute - the main goal is to verify the error doesn't crash the application
+			const result = await executeReadFileTool(
+				{},
+				{
+					totalLines: 5,
+				},
+			)
+
+			// Verify that the file is processed (the error is handled gracefully)
+			expect(result).toContain(`<file><path>${testFilePath}</path>`)
+
+			// Verify that we get a valid XML response (not an error)
+			expect(result).toMatch(/<files>.*<\/files>/s)
+			expect(result).not.toContain("<error>")
+		})
 	})
 })