Skip to content

Commit e8f9de1

Browse files
EhabYEhabY
committed
Review comments
1 parent 128754a commit e8f9de1

File tree

1 file changed

+74
-68
lines changed

1 file changed

+74
-68
lines changed

‎src/remote/sshProcess.ts‎

Lines changed: 74 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -78,42 +78,64 @@ export class SshProcessMonitor implements vscode.Disposable {
7878
private lastStaleSearchTime = 0;
7979

8080
/**
81-
* Cleans up network info files older than the specified age.
81+
* Helper to clean up files in a directory.
82+
* Stats files in parallel, applies selection criteria, then deletes in parallel.
8283
*/
83-
private static async cleanupOldNetworkFiles(
84-
networkInfoPath: string,
85-
maxAgeMs: number,
84+
private static async cleanupFiles(
85+
dir: string,
86+
fileType: string,
8687
logger: Logger,
88+
options: {
89+
filter: (name: string) => boolean;
90+
select: (
91+
files: Array<{ name: string; mtime: number }>,
92+
now: number,
93+
) => Array<{ name: string }>;
94+
},
8795
): Promise<void> {
8896
try {
89-
const files = await fs.readdir(networkInfoPath);
9097
const now = Date.now();
98+
const files = await fs.readdir(dir);
9199

92-
const deletedFiles: string[] = [];
93-
for (const file of files) {
94-
if (!file.endsWith(".json")) {
95-
continue;
96-
}
100+
// Gather file stats in parallel
101+
const withStats = await Promise.all(
102+
files.filter(options.filter).map(async (name) => {
103+
try {
104+
const stats = await fs.stat(path.join(dir, name));
105+
return { name, mtime: stats.mtime.getTime() };
106+
} catch (error) {
107+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
108+
logger.debug(`Failed to stat ${fileType} ${name}`, error);
109+
}
110+
return null;
111+
}
112+
}),
113+
);
97114

98-
const filePath = path.join(networkInfoPath, file);
99-
try {
100-
const stats = await fs.stat(filePath);
101-
const ageMs = now - stats.mtime.getTime();
115+
const toDelete = options.select(
116+
withStats.filter((f) => f !== null),
117+
now,
118+
);
102119

103-
if (ageMs > maxAgeMs) {
104-
await fs.unlink(filePath);
105-
deletedFiles.push(file);
106-
}
107-
} catch (error) {
108-
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
109-
logger.debug(`Failed to clean up network info file ${file}`, error);
120+
// Delete files in parallel
121+
const results = await Promise.all(
122+
toDelete.map(async (file) => {
123+
try {
124+
await fs.unlink(path.join(dir, file.name));
125+
return file.name;
126+
} catch (error) {
127+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
128+
logger.debug(`Failed to delete ${fileType} ${file.name}`, error);
129+
}
130+
return null;
110131
}
111-
}
112-
}
132+
}),
133+
);
113134

135+
const deletedFiles = results.filter((name) => name !== null);
114136
if (deletedFiles.length > 0) {
115137
logger.debug(
116-
`Cleaned up ${deletedFiles.length} old network info file(s): ${deletedFiles.join(", ")}`,
138+
`Cleaned up ${deletedFiles.length} ${fileType}(s): ${deletedFiles.join(", ")}`,
117139
);
118140
}
119141
} catch {
@@ -122,58 +144,42 @@ export class SshProcessMonitor implements vscode.Disposable {
122144
}
123145

124146
/**
125-
* Cleans up old proxy log files when there are too many.
126-
* Deletes oldest stale files (by mtime) until count <= maxFilesToKeep.
147+
* Cleans up network info files older than the specified age.
148+
*/
149+
private static async cleanupOldNetworkFiles(
150+
networkInfoPath: string,
151+
maxAgeMs: number,
152+
logger: Logger,
153+
): Promise<void> {
154+
await SshProcessMonitor.cleanupFiles(
155+
networkInfoPath,
156+
"network info file",
157+
logger,
158+
{
159+
filter: (name) => name.endsWith(".json"),
160+
select: (files, now) => files.filter((f) => now - f.mtime > maxAgeMs),
161+
},
162+
);
163+
}
164+
165+
/**
166+
* Cleans up old proxy log files that exceed the retention limit.
167+
* Only deletes files older than maxAgeMs when file count exceeds maxFilesToKeep.
127168
*/
128169
private static async cleanupOldLogFiles(
129170
logDir: string,
130171
maxFilesToKeep: number,
131172
maxAgeMs: number,
132173
logger: Logger,
133174
): Promise<void> {
134-
try {
135-
const now = Date.now();
136-
const files = await fs.readdir(logDir);
137-
138-
const withStats = await Promise.all(
175+
await SshProcessMonitor.cleanupFiles(logDir, "log file", logger, {
176+
filter: (name) => name.startsWith("coder-ssh") && name.endsWith(".log"),
177+
select: (files, now) =>
139178
files
140-
.filter((f) => f.startsWith("coder-ssh") && f.endsWith(".log"))
141-
.map(async (name) => {
142-
try {
143-
const stats = await fs.stat(path.join(logDir, name));
144-
return { name, mtime: stats.mtime.getTime() };
145-
} catch {
146-
return null;
147-
}
148-
}),
149-
);
150-
151-
const toDelete = withStats
152-
.filter((f) => f !== null)
153-
.sort((a, b) => a.mtime - b.mtime) // oldest first
154-
.slice(0, -maxFilesToKeep) // keep the newest maxFilesToKeep
155-
.filter((f) => now - f.mtime > maxAgeMs); // only delete stale files
156-
157-
const deletedFiles: string[] = [];
158-
for (const file of toDelete) {
159-
try {
160-
await fs.unlink(path.join(logDir, file.name));
161-
deletedFiles.push(file.name);
162-
} catch (error) {
163-
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
164-
logger.debug(`Failed to clean up log file ${file.name}`, error);
165-
}
166-
}
167-
}
168-
169-
if (deletedFiles.length > 0) {
170-
logger.debug(
171-
`Cleaned up ${deletedFiles.length} old log file(s): ${deletedFiles.join(", ")}`,
172-
);
173-
}
174-
} catch {
175-
// Directory may not exist yet, ignore
176-
}
179+
.toSorted((a, b) => a.mtime - b.mtime) // oldest first
180+
.slice(0, -maxFilesToKeep) // keep the newest maxFilesToKeep
181+
.filter((f) => now - f.mtime > maxAgeMs), // only delete stale files
182+
});
177183
}
178184

179185
private constructor(options: SshProcessMonitorOptions) {

0 commit comments

Comments
 (0)