Skip to content

Commit 502cf47

Browse files
Eric Wheelerhannesrudolph
Eric Wheeler
authored and
hannesrudolph
committed
refactor: replace fs.readFile + JSON.parse with safeReadJson
Replace manual file reading and JSON parsing with the safer safeReadJson utility across multiple files in the codebase. This change: - Provides atomic file access with proper locking to prevent race conditions - Streams file contents efficiently for better memory usage - Improves error handling consistency - Reduces code duplication Fixes: #5331 Signed-off-by: Eric Wheeler <[email protected]>
1 parent cb978cd commit 502cf47

File tree

13 files changed

+105
-102
lines changed

13 files changed

+105
-102
lines changed

‎src/api/providers/fetchers/modelCache.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,13 @@ import * as path from "path"
22
import fs from "fs/promises"
33

44
import NodeCache from "node-cache"
5-
65
import type { ProviderName } from "@roo-code/types"
7-
6+
import { safeReadJson } from "../../../utils/safeReadJson"
87
import { safeWriteJson } from "../../../utils/safeWriteJson"
98

109
import { ContextProxy } from "../../../core/config/ContextProxy"
1110
import { getCacheDirectoryPath } from "../../../utils/storage"
1211
import type { RouterName, ModelRecord } from "../../../shared/api"
13-
import { fileExistsAtPath } from "../../../utils/fs"
1412

1513
import { getOpenRouterModels } from "./openrouter"
1614
import { getVercelAiGatewayModels } from "./vercel-ai-gateway"
@@ -37,8 +35,14 @@ async function readModels(router: RouterName): Promise<ModelRecord | undefined>
3735
const filename = `${router}_models.json`
3836
const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
3937
const filePath = path.join(cacheDir, filename)
40-
const exists = await fileExistsAtPath(filePath)
41-
return exists ? JSON.parse(await fs.readFile(filePath, "utf8")) : undefined
38+
try {
39+
return await safeReadJson(filePath)
40+
} catch (error: any) {
41+
if (error.code === "ENOENT") {
42+
return undefined
43+
}
44+
throw error
45+
}
4246
}
4347

4448
/**

‎src/api/providers/fetchers/modelEndpointCache.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ import * as path from "path"
22
import fs from "fs/promises"
33

44
import NodeCache from "node-cache"
5+
import { safeReadJson } from "../../../utils/safeReadJson"
56
import { safeWriteJson } from "../../../utils/safeWriteJson"
67
import sanitize from "sanitize-filename"
78

89
import { ContextProxy } from "../../../core/config/ContextProxy"
910
import { getCacheDirectoryPath } from "../../../utils/storage"
1011
import { RouterName, ModelRecord } from "../../../shared/api"
11-
import { fileExistsAtPath } from "../../../utils/fs"
1212

1313
import { getOpenRouterModelEndpoints } from "./openrouter"
1414

@@ -26,8 +26,11 @@ async function readModelEndpoints(key: string): Promise<ModelRecord | undefined>
2626
const filename = `${key}_endpoints.json`
2727
const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
2828
const filePath = path.join(cacheDir, filename)
29-
const exists = await fileExistsAtPath(filePath)
30-
return exists ? JSON.parse(await fs.readFile(filePath, "utf8")) : undefined
29+
try {
30+
return await safeReadJson(filePath)
31+
} catch (error) {
32+
return undefined
33+
}
3134
}
3235

3336
export const getModelEndpoints = async ({

‎src/core/config/importExport.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { safeReadJson } from "../../utils/safeReadJson"
12
import { safeWriteJson } from "../../utils/safeWriteJson"
23
import os from "os"
34
import * as path from "path"
@@ -49,7 +50,7 @@ export async function importSettingsFromPath(
4950
const previousProviderProfiles = await providerSettingsManager.export()
5051

5152
const { providerProfiles: newProviderProfiles, globalSettings = {} } = schema.parse(
52-
JSON.parse(await fs.readFile(filePath, "utf-8")),
53+
await safeReadJson(filePath),
5354
)
5455

5556
const providerProfiles = {

‎src/core/context-tracking/FileContextTracker.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1+
import { safeReadJson } from "../../utils/safeReadJson"
12
import { safeWriteJson } from "../../utils/safeWriteJson"
23
import * as path from "path"
34
import * as vscode from "vscode"
45
import { getTaskDirectoryPath } from "../../utils/storage"
56
import { GlobalFileNames } from "../../shared/globalFileNames"
6-
import { fileExistsAtPath } from "../../utils/fs"
7-
import fs from "fs/promises"
87
import { ContextProxy } from "../config/ContextProxy"
98
import type { FileMetadataEntry, RecordSource, TaskMetadata } from "./FileContextTrackerTypes"
109
import { ClineProvider } from "../webview/ClineProvider"
@@ -116,12 +115,14 @@ export class FileContextTracker {
116115
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
117116
const filePath = path.join(taskDir, GlobalFileNames.taskMetadata)
118117
try {
119-
if (await fileExistsAtPath(filePath)) {
120-
return JSON.parse(await fs.readFile(filePath, "utf8"))
121-
}
118+
return await safeReadJson(filePath)
122119
} catch (error) {
123-
console.error("Failed to read task metadata:", error)
120+
if (error.code !== "ENOENT") {
121+
console.error("Failed to read task metadata:", error)
122+
}
124123
}
124+
125+
// On error, return default empty metadata
125126
return { files_in_context: [] }
126127
}
127128

‎src/core/task-persistence/apiMessages.ts‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { safeReadJson } from "../../utils/safeReadJson"
12
import { safeWriteJson } from "../../utils/safeWriteJson"
23
import * as path from "path"
34
import * as fs from "fs/promises"
@@ -21,51 +22,51 @@ export async function readApiMessages({
2122
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
2223
const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory)
2324

24-
if (await fileExistsAtPath(filePath)) {
25-
const fileContent = await fs.readFile(filePath, "utf8")
26-
try {
27-
const parsedData = JSON.parse(fileContent)
28-
if (Array.isArray(parsedData) && parsedData.length === 0) {
29-
console.error(
30-
`[Roo-Debug] readApiMessages: Found API conversation history file, but it's empty (parsed as []). TaskId: ${taskId}, Path: ${filePath}`,
31-
)
32-
}
33-
return parsedData
34-
} catch (error) {
25+
try {
26+
const parsedData = await safeReadJson(filePath)
27+
if (Array.isArray(parsedData) && parsedData.length === 0) {
3528
console.error(
36-
`[Roo-Debug] readApiMessages: Error parsing API conversation history file. TaskId: ${taskId}, Path: ${filePath}, Error: ${error}`,
29+
`[Roo-Debug] readApiMessages: Found API conversation history file, but it's empty (parsed as []). TaskId: ${taskId}, Path: ${filePath}`,
3730
)
38-
throw error
3931
}
40-
} else {
41-
const oldPath = path.join(taskDir, "claude_messages.json")
32+
return parsedData
33+
} catch (error: any) {
34+
if (error.code === "ENOENT") {
35+
// File doesn't exist, try the old path
36+
const oldPath = path.join(taskDir, "claude_messages.json")
4237

43-
if (await fileExistsAtPath(oldPath)) {
44-
const fileContent = await fs.readFile(oldPath, "utf8")
4538
try {
46-
const parsedData = JSON.parse(fileContent)
39+
const parsedData = await safeReadJson(oldPath)
4740
if (Array.isArray(parsedData) && parsedData.length === 0) {
4841
console.error(
4942
`[Roo-Debug] readApiMessages: Found OLD API conversation history file (claude_messages.json), but it's empty (parsed as []). TaskId: ${taskId}, Path: ${oldPath}`,
5043
)
5144
}
5245
await fs.unlink(oldPath)
5346
return parsedData
54-
} catch (error) {
47+
} catch (oldError: any) {
48+
if (oldError.code === "ENOENT") {
49+
// If we reach here, neither the new nor the old history file was found.
50+
console.error(
51+
`[Roo-Debug] readApiMessages: API conversation history file not found for taskId: ${taskId}. Expected at: ${filePath}`,
52+
)
53+
return []
54+
}
55+
56+
// For any other error with the old file, log and rethrow
5557
console.error(
56-
`[Roo-Debug] readApiMessages: Error parsing OLD API conversation history file (claude_messages.json). TaskId: ${taskId}, Path: ${oldPath}, Error: ${error}`,
58+
`[Roo-Debug] readApiMessages: Error reading OLD API conversation history file (claude_messages.json). TaskId: ${taskId}, Path: ${oldPath}, Error: ${oldError}`,
5759
)
58-
// DO NOT unlink oldPath if parsing failed, throw error instead.
59-
throw error
60+
throw oldError
6061
}
62+
} else {
63+
// For any other error with the main file, log and rethrow
64+
console.error(
65+
`[Roo-Debug] readApiMessages: Error reading API conversation history file. TaskId: ${taskId}, Path: ${filePath}, Error: ${error}`,
66+
)
67+
throw error
6168
}
6269
}
63-
64-
// If we reach here, neither the new nor the old history file was found.
65-
console.error(
66-
`[Roo-Debug] readApiMessages: API conversation history file not found for taskId: ${taskId}. Expected at: ${filePath}`,
67-
)
68-
return []
6970
}
7071

7172
export async function saveApiMessages({

‎src/core/task-persistence/taskMessages.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1+
import { safeReadJson } from "../../utils/safeReadJson"
12
import { safeWriteJson } from "../../utils/safeWriteJson"
23
import * as path from "path"
3-
import * as fs from "fs/promises"
44

55
import type { ClineMessage } from "@roo-code/types"
66

7-
import { fileExistsAtPath } from "../../utils/fs"
8-
97
import { GlobalFileNames } from "../../shared/globalFileNames"
108
import { getTaskDirectoryPath } from "../../utils/storage"
119

@@ -20,13 +18,15 @@ export async function readTaskMessages({
2018
}: ReadTaskMessagesOptions): Promise<ClineMessage[]> {
2119
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
2220
const filePath = path.join(taskDir, GlobalFileNames.uiMessages)
23-
const fileExists = await fileExistsAtPath(filePath)
2421

25-
if (fileExists) {
26-
return JSON.parse(await fs.readFile(filePath, "utf8"))
22+
try {
23+
return await safeReadJson(filePath)
24+
} catch (error) {
25+
if (error.code !== "ENOENT") {
26+
console.error("Failed to read task messages:", error)
27+
}
28+
return []
2729
}
28-
29-
return []
3030
}
3131

3232
export type SaveTaskMessagesOptions = {

‎src/core/webview/ClineProvider.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ import { t } from "../../i18n"
8282
import { buildApiHandler } from "../../api"
8383
import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../api/providers/fetchers/lmstudio"
8484

85+
import { safeReadJson } from "../../utils/safeReadJson"
8586
import { ContextProxy } from "../config/ContextProxy"
8687
import { ProviderSettingsManager } from "../config/ProviderSettingsManager"
8788
import { CustomModesManager } from "../config/CustomModesManager"
@@ -1457,10 +1458,9 @@ export class ClineProvider
14571458
const taskDirPath = await getTaskDirectoryPath(globalStoragePath, id)
14581459
const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
14591460
const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages)
1460-
const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
14611461

1462-
if (fileExists) {
1463-
const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
1462+
try {
1463+
const apiConversationHistory = await safeReadJson(apiConversationHistoryFilePath)
14641464

14651465
return {
14661466
historyItem,
@@ -1469,6 +1469,10 @@ export class ClineProvider
14691469
uiMessagesFilePath,
14701470
apiConversationHistory,
14711471
}
1472+
} catch (error) {
1473+
if (error.code !== "ENOENT") {
1474+
console.error(`Failed to read API conversation history for task ${id}:`, error)
1475+
}
14721476
}
14731477
}
14741478

‎src/integrations/misc/extract-text.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { isBinaryFile } from "isbinaryfile"
77
import { extractTextFromXLSX } from "./extract-text-from-xlsx"
88
import { countFileLines } from "./line-counter"
99
import { readLines } from "./read-lines"
10+
import { safeReadJson } from "../../utils/safeReadJson"
1011

1112
async function extractTextFromPDF(filePath: string): Promise<string> {
1213
const dataBuffer = await fs.readFile(filePath)
@@ -20,8 +21,7 @@ async function extractTextFromDOCX(filePath: string): Promise<string> {
2021
}
2122

2223
async function extractTextFromIPYNB(filePath: string): Promise<string> {
23-
const data = await fs.readFile(filePath, "utf8")
24-
const notebook = JSON.parse(data)
24+
const notebook = await safeReadJson(filePath)
2525
let extractedText = ""
2626

2727
for (const cell of notebook.cells) {

‎src/services/code-index/cache-manager.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as vscode from "vscode"
22
import { createHash } from "crypto"
33
import { ICacheManager } from "./interfaces/cache"
44
import debounce from "lodash.debounce"
5+
import { safeReadJson } from "../../utils/safeReadJson"
56
import { safeWriteJson } from "../../utils/safeWriteJson"
67
import { TelemetryService } from "@roo-code/telemetry"
78
import { TelemetryEventName } from "@roo-code/types"
@@ -37,8 +38,7 @@ export class CacheManager implements ICacheManager {
3738
*/
3839
async initialize(): Promise<void> {
3940
try {
40-
const cacheData = await vscode.workspace.fs.readFile(this.cachePath)
41-
this.fileHashes = JSON.parse(cacheData.toString())
41+
this.fileHashes = await safeReadJson(this.cachePath.fsPath)
4242
} catch (error) {
4343
this.fileHashes = {}
4444
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {

‎src/services/marketplace/MarketplaceManager.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type { CustomModesManager } from "../../core/config/CustomModesManager"
1515

1616
import { RemoteConfigLoader } from "./RemoteConfigLoader"
1717
import { SimpleInstaller } from "./SimpleInstaller"
18+
import { safeReadJson } from "../../utils/safeReadJson"
1819

1920
export interface MarketplaceItemsResponse {
2021
organizationMcps: MarketplaceItem[]
@@ -272,8 +273,7 @@ export class MarketplaceManager {
272273
// Check MCPs in .roo/mcp.json
273274
const projectMcpPath = path.join(workspaceFolder.uri.fsPath, ".roo", "mcp.json")
274275
try {
275-
const content = await fs.readFile(projectMcpPath, "utf-8")
276-
const data = JSON.parse(content)
276+
const data = await safeReadJson(projectMcpPath)
277277
if (data?.mcpServers && typeof data.mcpServers === "object") {
278278
for (const serverName of Object.keys(data.mcpServers)) {
279279
metadata[serverName] = {
@@ -317,8 +317,7 @@ export class MarketplaceManager {
317317
// Check global MCPs
318318
const globalMcpPath = path.join(globalSettingsPath, GlobalFileNames.mcpSettings)
319319
try {
320-
const content = await fs.readFile(globalMcpPath, "utf-8")
321-
const data = JSON.parse(content)
320+
const data = await safeReadJson(globalMcpPath)
322321
if (data?.mcpServers && typeof data.mcpServers === "object") {
323322
for (const serverName of Object.keys(data.mcpServers)) {
324323
metadata[serverName] = {

0 commit comments

Comments
 (0)