refactor: extract isProcessAlive and replace try/catches in cli.ts

Move isProcessAlive to shared error-handling module. Replace ~20
try/catch sites with safeUnlink/safeKill in killServer, connect,
disconnect, and cleanup flows. Convert empty catches to selective
catches. Reduces slop-scan empty-catch from 22 to 2 locations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-04-09 04:41:43 -10:00
parent 591cd766be
commit 11e56e9a10
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
1 changed files with 34 additions and 45 deletions

View File

@ -11,6 +11,7 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import { safeUnlink, safeKill, isProcessAlive } from './error-handling';
import { resolveConfig, ensureStateDir, readVersionHash } from './config'; import { resolveConfig, ensureStateDir, readVersionHash } from './config';
const config = resolveConfig(); const config = resolveConfig();
@ -103,27 +104,7 @@ function readState(): ServerState | null {
} }
} }
function isProcessAlive(pid: number): boolean { // isProcessAlive is imported from ./error-handling
if (IS_WINDOWS) {
// Bun's compiled binary can't signal Windows PIDs (always throws ESRCH).
// Use tasklist as a fallback. Only for one-shot calls — too slow for polling loops.
try {
const result = Bun.spawnSync(
['tasklist', '/FI', `PID eq ${pid}`, '/NH', '/FO', 'CSV'],
{ stdout: 'pipe', stderr: 'pipe', timeout: 3000 }
);
return result.stdout.toString().includes(`"${pid}"`);
} catch {
return false;
}
}
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
/** /**
* HTTP health check definitive proof the server is alive and responsive. * HTTP health check definitive proof the server is alive and responsive.
@ -153,7 +134,9 @@ async function killServer(pid: number): Promise<void> {
['taskkill', '/PID', String(pid), '/T', '/F'], ['taskkill', '/PID', String(pid), '/T', '/F'],
{ stdout: 'pipe', stderr: 'pipe', timeout: 5000 } { stdout: 'pipe', stderr: 'pipe', timeout: 5000 }
); );
} catch {} } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
const deadline = Date.now() + 2000; const deadline = Date.now() + 2000;
while (Date.now() < deadline && isProcessAlive(pid)) { while (Date.now() < deadline && isProcessAlive(pid)) {
await Bun.sleep(100); await Bun.sleep(100);
@ -161,7 +144,7 @@ async function killServer(pid: number): Promise<void> {
return; return;
} }
try { process.kill(pid, 'SIGTERM'); } catch { return; } safeKill(pid, 'SIGTERM');
// Wait up to 2s for graceful shutdown // Wait up to 2s for graceful shutdown
const deadline = Date.now() + 2000; const deadline = Date.now() + 2000;
@ -171,7 +154,7 @@ async function killServer(pid: number): Promise<void> {
// Force kill if still alive // Force kill if still alive
if (isProcessAlive(pid)) { if (isProcessAlive(pid)) {
try { process.kill(pid, 'SIGKILL'); } catch {} safeKill(pid, 'SIGKILL');
} }
} }
@ -197,10 +180,10 @@ function cleanupLegacyState(): void {
}); });
const cmd = check.stdout.toString().trim(); const cmd = check.stdout.toString().trim();
if (cmd.includes('bun') || cmd.includes('server.ts')) { if (cmd.includes('bun') || cmd.includes('server.ts')) {
try { process.kill(data.pid, 'SIGTERM'); } catch {} safeKill(data.pid, 'SIGTERM');
} }
} }
fs.unlinkSync(fullPath); safeUnlink(fullPath);
} catch { } catch {
// Best effort — skip files we can't parse or clean up // Best effort — skip files we can't parse or clean up
} }
@ -210,7 +193,7 @@ function cleanupLegacyState(): void {
f.startsWith('browse-console') || f.startsWith('browse-network') || f.startsWith('browse-dialog') f.startsWith('browse-console') || f.startsWith('browse-network') || f.startsWith('browse-dialog')
); );
for (const file of logFiles) { for (const file of logFiles) {
try { fs.unlinkSync(`/tmp/${file}`); } catch {} safeUnlink(`/tmp/${file}`);
} }
} catch { } catch {
// /tmp read failed — skip legacy cleanup // /tmp read failed — skip legacy cleanup
@ -222,8 +205,8 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
ensureStateDir(config); ensureStateDir(config);
// Clean up stale state file and error log // Clean up stale state file and error log
try { fs.unlinkSync(config.stateFile); } catch {} safeUnlink(config.stateFile);
try { fs.unlinkSync(path.join(config.stateDir, 'browse-startup-error.log')); } catch {} safeUnlink(path.join(config.stateDir, 'browse-startup-error.log'));
let proc: any = null; let proc: any = null;
@ -297,7 +280,7 @@ function acquireServerLock(): (() => void) | null {
const fd = fs.openSync(lockPath, 'wx'); const fd = fs.openSync(lockPath, 'wx');
fs.writeSync(fd, `${process.pid}\n`); fs.writeSync(fd, `${process.pid}\n`);
fs.closeSync(fd); fs.closeSync(fd);
return () => { try { fs.unlinkSync(lockPath); } catch {} }; return () => { safeUnlink(lockPath); };
} catch { } catch {
// Lock already held — check if the holder is still alive // Lock already held — check if the holder is still alive
try { try {
@ -469,7 +452,9 @@ function isNgrokAvailable(): boolean {
try { try {
const content = fs.readFileSync(conf, 'utf-8'); const content = fs.readFileSync(conf, 'utf-8');
if (content.includes('authtoken:')) return true; if (content.includes('authtoken:')) return true;
} catch {} } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
} }
return false; return false;
@ -797,10 +782,10 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
// Kill ANY existing server (SIGTERM → wait 2s → SIGKILL) // Kill ANY existing server (SIGTERM → wait 2s → SIGKILL)
if (existingState && isProcessAlive(existingState.pid)) { if (existingState && isProcessAlive(existingState.pid)) {
try { process.kill(existingState.pid, 'SIGTERM'); } catch {} safeKill(existingState.pid, 'SIGTERM');
await new Promise(resolve => setTimeout(resolve, 2000)); await new Promise(resolve => setTimeout(resolve, 2000));
if (isProcessAlive(existingState.pid)) { if (isProcessAlive(existingState.pid)) {
try { process.kill(existingState.pid, 'SIGKILL'); } catch {} safeKill(existingState.pid, 'SIGKILL');
await new Promise(resolve => setTimeout(resolve, 1000)); await new Promise(resolve => setTimeout(resolve, 1000));
} }
} }
@ -814,24 +799,24 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
const lockTarget = fs.readlinkSync(singletonLock); // e.g. "hostname-12345" const lockTarget = fs.readlinkSync(singletonLock); // e.g. "hostname-12345"
const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10); const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10);
if (orphanPid && isProcessAlive(orphanPid)) { if (orphanPid && isProcessAlive(orphanPid)) {
try { process.kill(orphanPid, 'SIGTERM'); } catch {} safeKill(orphanPid, 'SIGTERM');
await new Promise(resolve => setTimeout(resolve, 1000)); await new Promise(resolve => setTimeout(resolve, 1000));
if (isProcessAlive(orphanPid)) { if (isProcessAlive(orphanPid)) {
try { process.kill(orphanPid, 'SIGKILL'); } catch {} safeKill(orphanPid, 'SIGKILL');
await new Promise(resolve => setTimeout(resolve, 500)); await new Promise(resolve => setTimeout(resolve, 500));
} }
} }
} catch { } catch (err: any) {
// No lock symlink or not readable — nothing to kill if (err?.code !== 'ENOENT' && err?.code !== 'EINVAL') throw err;
} }
// Clean up Chromium profile locks (can persist after crashes) // Clean up Chromium profile locks (can persist after crashes)
for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) {
try { fs.unlinkSync(path.join(profileDir, lockFile)); } catch {} safeUnlink(path.join(profileDir, lockFile));
} }
// Delete stale state file // Delete stale state file
try { fs.unlinkSync(config.stateFile); } catch {} safeUnlink(config.stateFile);
console.log('Launching headed Chromium with extension + sidebar agent...'); console.log('Launching headed Chromium with extension + sidebar agent...');
try { try {
@ -877,7 +862,9 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
try { try {
fs.mkdirSync(path.dirname(agentQueue), { recursive: true, mode: 0o700 }); fs.mkdirSync(path.dirname(agentQueue), { recursive: true, mode: 0o700 });
fs.writeFileSync(agentQueue, '', { mode: 0o600 }); fs.writeFileSync(agentQueue, '', { mode: 0o600 });
} catch {} } catch (err: any) {
if (err?.code !== 'EACCES') throw err;
}
// Resolve browse binary path the same way — execPath-relative // Resolve browse binary path the same way — execPath-relative
let browseBin = path.resolve(__dirname, '..', 'dist', 'browse'); let browseBin = path.resolve(__dirname, '..', 'dist', 'browse');
@ -891,7 +878,9 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
try { try {
const { spawnSync } = require('child_process'); const { spawnSync } = require('child_process');
spawnSync('pkill', ['-f', 'sidebar-agent\\.ts'], { stdio: 'ignore', timeout: 3000 }); spawnSync('pkill', ['-f', 'sidebar-agent\\.ts'], { stdio: 'ignore', timeout: 3000 });
} catch {} } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
const agentProc = Bun.spawn(['bun', 'run', agentScript], { const agentProc = Bun.spawn(['bun', 'run', agentScript], {
cwd: config.projectDir, cwd: config.projectDir,
@ -947,18 +936,18 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
} }
// Force kill + cleanup // Force kill + cleanup
if (isProcessAlive(existingState.pid)) { if (isProcessAlive(existingState.pid)) {
try { process.kill(existingState.pid, 'SIGTERM'); } catch {} safeKill(existingState.pid, 'SIGTERM');
await new Promise(resolve => setTimeout(resolve, 2000)); await new Promise(resolve => setTimeout(resolve, 2000));
if (isProcessAlive(existingState.pid)) { if (isProcessAlive(existingState.pid)) {
try { process.kill(existingState.pid, 'SIGKILL'); } catch {} safeKill(existingState.pid, 'SIGKILL');
} }
} }
// Clean profile locks and state file // Clean profile locks and state file
const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile');
for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) {
try { fs.unlinkSync(path.join(profileDir, lockFile)); } catch {} safeUnlink(path.join(profileDir, lockFile));
} }
try { fs.unlinkSync(config.stateFile); } catch {} safeUnlink(config.stateFile);
console.log('Disconnected (server was unresponsive — force cleaned).'); console.log('Disconnected (server was unresponsive — force cleaned).');
process.exit(0); process.exit(0);
} }