fix: address PR review feedback

- Remove unusual semicolon prefix in readFileTool.ts and use conventional approach
- Add test coverage for RangeError handling in extract-text.ts
- Console warning messages are already consistent across files
- Error handling patterns are already consistent (single try-catch)
This commit is contained in:
Roo Code 2025-07-29 22:43:39 +00:00
parent 17b08730dc
commit ecb09af007
2 changed files with 21 additions and 1 deletions

View file

@ -437,7 +437,9 @@ export async function readFileTool(
let isBinary: boolean
try {
;[totalLines, isBinary] = await Promise.all([countFileLines(fullPath), isBinaryFile(fullPath)])
const results = await Promise.all([countFileLines(fullPath), isBinaryFile(fullPath)])
totalLines = results[0]
isBinary = results[1]
} catch (error) {
// If isBinaryFile throws an error (e.g., RangeError), treat the file as binary
console.warn(`Error checking if file is binary for ${relPath}:`, error)

View file

@ -25,6 +25,8 @@ describe("extractTextFromFile - Large File Handling", () => {
// Set default mock behavior
mockedFs.access.mockResolvedValue(undefined)
mockedIsBinaryFile.mockResolvedValue(false)
// Mock console.warn
vi.spyOn(console, "warn").mockImplementation(() => {})
})
it("should truncate files that exceed maxReadFileLine limit", async () => {
@ -218,4 +220,20 @@ describe("extractTextFromFile - Large File Handling", () => {
"File not found: /test/nonexistent.ts",
)
})
it("should handle RangeError from isBinaryFile and treat file as binary", async () => {
// Setup - mock isBinaryFile to throw RangeError
mockedIsBinaryFile.mockRejectedValue(new RangeError("Invalid array length"))
// Execute and expect it to throw since file is treated as binary
await expect(extractTextFromFile("/test/problematic-file.bin", 100)).rejects.toThrow(
"Cannot read text for file type: .bin",
)
// Verify that the warning was logged
expect(console.warn).toHaveBeenCalledWith(
"Error checking if file is binary for /test/problematic-file.bin:",
expect.any(RangeError),
)
})
})