From 591cd766be5027d02230c8167457bffc19926bd8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 9 Apr 2026 04:41:37 -1000 Subject: [PATCH] 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) --- browse/src/server.ts | 82 ++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/browse/src/server.ts b/browse/src/server.ts index 37a2b531..4f97afb9 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -38,6 +38,7 @@ import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubsc import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector'; // Bun.spawn used instead of child_process.spawn (compiled bun binaries // fail posix_spawn on all executables including /bin/bash) +import { safeUnlink, safeKill } from './error-handling'; import * as fs from 'fs'; import * as net from 'net'; import * as path from 'path'; @@ -233,7 +234,9 @@ function findBrowseBin(): string { path.join(process.env.HOME || '', '.claude', 'skills', 'gstack', 'browse', 'dist', 'browse'), ]; 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 } @@ -265,13 +268,17 @@ function findClaudeBin(): string | null { const p = proc.stdout.toString().trim(); if (p) candidates.unshift(p); } - } catch {} + } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; + } for (const c of candidates) { try { if (!fs.existsSync(c)) continue; // Resolve symlinks — posix_spawn can fail on symlinks in compiled bun binaries return fs.realpathSync(c); - } catch {} + } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; + } } return null; } @@ -465,8 +472,8 @@ function listSessions(): Array { try { const session = JSON.parse(fs.readFileSync(path.join(SESSIONS_DIR, d, 'session.json'), 'utf-8')); let chatLines = 0; - try { chatLines = fs.readFileSync(path.join(SESSIONS_DIR, d, 'chat.jsonl'), 'utf-8').split('\n').filter(Boolean).length; } catch { - // Expected: no chat file yet + try { chatLines = fs.readFileSync(path.join(SESSIONS_DIR, d, 'chat.jsonl'), 'utf-8').split('\n').filter(Boolean).length; } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; } return { ...session, chatLines }; } catch { return null; } @@ -602,7 +609,9 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId try { fs.mkdirSync(gstackDir, { recursive: true, mode: 0o700 }); 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) { addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: `Failed to queue: ${err.message}` }); agentStatus = 'idle'; @@ -617,12 +626,11 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId function killAgent(targetTabId?: number | null): void { if (agentProcess) { - try { agentProcess.kill('SIGTERM'); } catch (err: any) { - console.warn('[browse] Failed to SIGTERM agent:', err.message); + const pid = agentProcess.pid; + 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. // 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 tabId = targetTabId ?? agentTabId ?? 0; 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; agentStartTime = null; currentMessage = null; @@ -1175,15 +1188,11 @@ async function shutdown() { // Clean up Chromium profile locks (prevent SingletonLock on next launch) const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { - try { fs.unlinkSync(path.join(profileDir, lockFile)); } catch (err: any) { - console.debug('[browse] Lock cleanup:', lockFile, err.message); - } + safeUnlink(path.join(profileDir, lockFile)); } // Clean up state file - try { fs.unlinkSync(config.stateFile); } catch (err: any) { - console.debug('[browse] State file cleanup:', err.message); - } + safeUnlink(config.stateFile); 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. if (process.platform === 'win32') { process.on('exit', () => { - try { fs.unlinkSync(config.stateFile); } catch { - // Best-effort on exit - } + safeUnlink(config.stateFile); }); } @@ -1216,13 +1223,9 @@ function emergencyCleanup() { // Clean Chromium profile locks const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { - try { fs.unlinkSync(path.join(profileDir, lockFile)); } catch (err: any) { - 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(path.join(profileDir, lockFile)); } + safeUnlink(config.stateFile); } process.on('uncaughtException', (err) => { console.error('[browse] FATAL uncaught exception:', err.message); @@ -1238,15 +1241,9 @@ process.on('unhandledRejection', (err: any) => { // ─── Start ───────────────────────────────────────────────────── async function start() { // Clear old log files - try { fs.unlinkSync(CONSOLE_LOG_PATH); } catch (err: any) { - if (err.code !== 'ENOENT') console.debug('[browse] Log cleanup console:', err.message); - } - 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); - } + safeUnlink(CONSOLE_LOG_PATH); + safeUnlink(NETWORK_LOG_PATH); + safeUnlink(DIALOG_LOG_PATH); const port = await findPort(); @@ -1282,15 +1279,11 @@ async function start() { const slug = process.env.GSTACK_SLUG || 'unknown'; const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp'; const projectWelcome = `${homeDir}/.gstack/projects/${slug}/designs/welcome-page-20260331/finalized.html`; - try { if (require('fs').existsSync(projectWelcome)) return projectWelcome; } catch (err: any) { - console.warn('[browse] Error checking project welcome page:', err.message); - } + if (fs.existsSync(projectWelcome)) return projectWelcome; // Fallback: built-in welcome page from gstack install const skillRoot = process.env.GSTACK_SKILL_ROOT || `${homeDir}/.claude/skills/gstack`; const builtinWelcome = `${skillRoot}/browse/src/welcome.html`; - try { if (require('fs').existsSync(builtinWelcome)) return builtinWelcome; } catch (err: any) { - console.warn('[browse] Error checking builtin welcome page:', err.message); - } + if (fs.existsSync(builtinWelcome)) return builtinWelcome; return null; })(); if (welcomePath) { @@ -1814,8 +1807,9 @@ async function start() { chatBuffer = []; chatNextId = 0; if (sidebarSession) { - try { fs.writeFileSync(path.join(SESSIONS_DIR, sidebarSession.id, 'chat.jsonl'), '', { mode: 0o600 }); } catch (err: any) { - console.error('[browse] Failed to clear chat file:', err.message); + const chatFile = path.join(SESSIONS_DIR, sidebarSession.id, 'chat.jsonl'); + 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' } });