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
This commit is contained in:
Roo Code 2025-07-30 00:04:26 +00:00
parent ecb09af007
commit 6796e1a285
2 changed files with 32 additions and 11 deletions

View file

@ -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) {

View file

@ -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>")
})
})
})