refactor: replace defensive try/catches in server.ts with utilities

Replace ~12 try/catch sites with safeUnlink/safeKill calls in shutdown,
emergencyCleanup, killAgent, and log cleanup. Convert empty catches to
selective catches with error code checks. Remove needless welcome page
try/catches (fs.existsSync doesn't need wrapping). Reduces slop-scan
empty-catch locations from 11 to 8 and error-swallowing from 24 to 18.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-04-09 04:41:37 -10:00
parent 2468a25aa1
commit 591cd766be

View File

@@ -38,6 +38,7 @@ import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubsc
import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector'; import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector';
// Bun.spawn used instead of child_process.spawn (compiled bun binaries // Bun.spawn used instead of child_process.spawn (compiled bun binaries
// fail posix_spawn on all executables including /bin/bash) // fail posix_spawn on all executables including /bin/bash)
import { safeUnlink, safeKill } from './error-handling';
import * as fs from 'fs'; import * as fs from 'fs';
import * as net from 'net'; import * as net from 'net';
import * as path from 'path'; import * as path from 'path';
@@ -233,7 +234,9 @@ function findBrowseBin(): string {
path.join(process.env.HOME || '', '.claude', 'skills', 'gstack', 'browse', 'dist', 'browse'), path.join(process.env.HOME || '', '.claude', 'skills', 'gstack', 'browse', 'dist', 'browse'),
]; ];
for (const c of candidates) { for (const c of candidates) {
try { if (fs.existsSync(c)) return c; } catch {} try { if (fs.existsSync(c)) return c; } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
} }
return 'browse'; // fallback to PATH return 'browse'; // fallback to PATH
} }
@@ -265,13 +268,17 @@ function findClaudeBin(): string | null {
const p = proc.stdout.toString().trim(); const p = proc.stdout.toString().trim();
if (p) candidates.unshift(p); if (p) candidates.unshift(p);
} }
} catch {} } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
for (const c of candidates) { for (const c of candidates) {
try { try {
if (!fs.existsSync(c)) continue; if (!fs.existsSync(c)) continue;
// Resolve symlinks — posix_spawn can fail on symlinks in compiled bun binaries // Resolve symlinks — posix_spawn can fail on symlinks in compiled bun binaries
return fs.realpathSync(c); return fs.realpathSync(c);
} catch {} } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
} }
return null; return null;
} }
@@ -465,8 +472,8 @@ function listSessions(): Array<SidebarSession & { chatLines: number }> {
try { try {
const session = JSON.parse(fs.readFileSync(path.join(SESSIONS_DIR, d, 'session.json'), 'utf-8')); const session = JSON.parse(fs.readFileSync(path.join(SESSIONS_DIR, d, 'session.json'), 'utf-8'));
let chatLines = 0; let chatLines = 0;
try { chatLines = fs.readFileSync(path.join(SESSIONS_DIR, d, 'chat.jsonl'), 'utf-8').split('\n').filter(Boolean).length; } catch { try { chatLines = fs.readFileSync(path.join(SESSIONS_DIR, d, 'chat.jsonl'), 'utf-8').split('\n').filter(Boolean).length; } catch (err: any) {
// Expected: no chat file yet if (err?.code !== 'ENOENT') throw err;
} }
return { ...session, chatLines }; return { ...session, chatLines };
} catch { return null; } } catch { return null; }
@@ -602,7 +609,9 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId
try { try {
fs.mkdirSync(gstackDir, { recursive: true, mode: 0o700 }); fs.mkdirSync(gstackDir, { recursive: true, mode: 0o700 });
fs.appendFileSync(agentQueue, entry + '\n'); fs.appendFileSync(agentQueue, entry + '\n');
try { fs.chmodSync(agentQueue, 0o600); } catch {} try { fs.chmodSync(agentQueue, 0o600); } catch (err: any) {
if (err?.code !== 'ENOENT') throw err;
}
} catch (err: any) { } catch (err: any) {
addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: `Failed to queue: ${err.message}` }); addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: `Failed to queue: ${err.message}` });
agentStatus = 'idle'; agentStatus = 'idle';
@@ -617,12 +626,11 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId
function killAgent(targetTabId?: number | null): void { function killAgent(targetTabId?: number | null): void {
if (agentProcess) { if (agentProcess) {
try { agentProcess.kill('SIGTERM'); } catch (err: any) { const pid = agentProcess.pid;
console.warn('[browse] Failed to SIGTERM agent:', err.message); if (pid) {
safeKill(pid, 'SIGTERM');
setTimeout(() => { safeKill(pid, 'SIGKILL'); }, 3000);
} }
setTimeout(() => { try { agentProcess?.kill('SIGKILL'); } catch (err: any) {
console.warn('[browse] Failed to SIGKILL agent:', err.message);
} }, 3000);
} }
// Signal the sidebar-agent worker to cancel via a per-tab cancel file. // Signal the sidebar-agent worker to cancel via a per-tab cancel file.
// Using per-tab files prevents race conditions where one agent's cancel // Using per-tab files prevents race conditions where one agent's cancel
@@ -631,7 +639,12 @@ function killAgent(targetTabId?: number | null): void {
const cancelDir = path.join(process.env.HOME || '/tmp', '.gstack'); const cancelDir = path.join(process.env.HOME || '/tmp', '.gstack');
const tabId = targetTabId ?? agentTabId ?? 0; const tabId = targetTabId ?? agentTabId ?? 0;
const cancelFile = path.join(cancelDir, `sidebar-agent-cancel-${tabId}`); const cancelFile = path.join(cancelDir, `sidebar-agent-cancel-${tabId}`);
try { fs.writeFileSync(cancelFile, Date.now().toString()); } catch {} try {
fs.mkdirSync(cancelDir, { recursive: true });
fs.writeFileSync(cancelFile, Date.now().toString());
} catch (err: any) {
if (err?.code !== 'EACCES' && err?.code !== 'ENOENT') throw err;
}
agentProcess = null; agentProcess = null;
agentStartTime = null; agentStartTime = null;
currentMessage = null; currentMessage = null;
@@ -1175,15 +1188,11 @@ async function shutdown() {
// Clean up Chromium profile locks (prevent SingletonLock on next launch) // Clean up Chromium profile locks (prevent SingletonLock on next launch)
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 (err: any) { safeUnlink(path.join(profileDir, lockFile));
console.debug('[browse] Lock cleanup:', lockFile, err.message);
}
} }
// Clean up state file // Clean up state file
try { fs.unlinkSync(config.stateFile); } catch (err: any) { safeUnlink(config.stateFile);
console.debug('[browse] State file cleanup:', err.message);
}
process.exit(0); process.exit(0);
} }
@@ -1195,9 +1204,7 @@ process.on('SIGINT', shutdown);
// Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check. // Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check.
if (process.platform === 'win32') { if (process.platform === 'win32') {
process.on('exit', () => { process.on('exit', () => {
try { fs.unlinkSync(config.stateFile); } catch { safeUnlink(config.stateFile);
// Best-effort on exit
}
}); });
} }
@@ -1216,13 +1223,9 @@ function emergencyCleanup() {
// Clean Chromium profile locks // Clean Chromium profile locks
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 (err: any) { safeUnlink(path.join(profileDir, lockFile));
console.debug('[browse] Emergency lock cleanup:', lockFile, err.message);
}
}
try { fs.unlinkSync(config.stateFile); } catch (err: any) {
console.debug('[browse] Emergency state cleanup:', err.message);
} }
safeUnlink(config.stateFile);
} }
process.on('uncaughtException', (err) => { process.on('uncaughtException', (err) => {
console.error('[browse] FATAL uncaught exception:', err.message); console.error('[browse] FATAL uncaught exception:', err.message);
@@ -1238,15 +1241,9 @@ process.on('unhandledRejection', (err: any) => {
// ─── Start ───────────────────────────────────────────────────── // ─── Start ─────────────────────────────────────────────────────
async function start() { async function start() {
// Clear old log files // Clear old log files
try { fs.unlinkSync(CONSOLE_LOG_PATH); } catch (err: any) { safeUnlink(CONSOLE_LOG_PATH);
if (err.code !== 'ENOENT') console.debug('[browse] Log cleanup console:', err.message); safeUnlink(NETWORK_LOG_PATH);
} safeUnlink(DIALOG_LOG_PATH);
try { fs.unlinkSync(NETWORK_LOG_PATH); } catch (err: any) {
if (err.code !== 'ENOENT') console.debug('[browse] Log cleanup network:', err.message);
}
try { fs.unlinkSync(DIALOG_LOG_PATH); } catch (err: any) {
if (err.code !== 'ENOENT') console.debug('[browse] Log cleanup dialog:', err.message);
}
const port = await findPort(); const port = await findPort();
@@ -1282,15 +1279,11 @@ async function start() {
const slug = process.env.GSTACK_SLUG || 'unknown'; const slug = process.env.GSTACK_SLUG || 'unknown';
const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp'; const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp';
const projectWelcome = `${homeDir}/.gstack/projects/${slug}/designs/welcome-page-20260331/finalized.html`; const projectWelcome = `${homeDir}/.gstack/projects/${slug}/designs/welcome-page-20260331/finalized.html`;
try { if (require('fs').existsSync(projectWelcome)) return projectWelcome; } catch (err: any) { if (fs.existsSync(projectWelcome)) return projectWelcome;
console.warn('[browse] Error checking project welcome page:', err.message);
}
// Fallback: built-in welcome page from gstack install // Fallback: built-in welcome page from gstack install
const skillRoot = process.env.GSTACK_SKILL_ROOT || `${homeDir}/.claude/skills/gstack`; const skillRoot = process.env.GSTACK_SKILL_ROOT || `${homeDir}/.claude/skills/gstack`;
const builtinWelcome = `${skillRoot}/browse/src/welcome.html`; const builtinWelcome = `${skillRoot}/browse/src/welcome.html`;
try { if (require('fs').existsSync(builtinWelcome)) return builtinWelcome; } catch (err: any) { if (fs.existsSync(builtinWelcome)) return builtinWelcome;
console.warn('[browse] Error checking builtin welcome page:', err.message);
}
return null; return null;
})(); })();
if (welcomePath) { if (welcomePath) {
@@ -1814,8 +1807,9 @@ async function start() {
chatBuffer = []; chatBuffer = [];
chatNextId = 0; chatNextId = 0;
if (sidebarSession) { if (sidebarSession) {
try { fs.writeFileSync(path.join(SESSIONS_DIR, sidebarSession.id, 'chat.jsonl'), '', { mode: 0o600 }); } catch (err: any) { const chatFile = path.join(SESSIONS_DIR, sidebarSession.id, 'chat.jsonl');
console.error('[browse] Failed to clear chat file:', err.message); try { fs.writeFileSync(chatFile, '', { mode: 0o600 }); } catch (err: any) {
if (err?.code !== 'ENOENT') console.error('[browse] Failed to clear chat file:', err.message);
} }
} }
return new Response(JSON.stringify({ ok: true }), { status: 200, headers: { 'Content-Type': 'application/json' } }); return new Response(JSON.stringify({ ok: true }), { status: 200, headers: { 'Content-Type': 'application/json' } });