mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
refactor: selective catches in write-commands, cdp-inspector, meta-commands, snapshot
Convert ~27 empty/obscuring catches to selective error handling across 4 browse source files. CDP ops check for closed/Target/detached messages, DOM ops check TypeError/DOMException, filesystem ops check ENOENT/EACCES, JSON parsing checks SyntaxError. Remove dead code in cdp-inspector where try/catch wrapped synchronous no-ops. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
+25
-34
@@ -98,8 +98,9 @@ async function getOrCreateSession(page: Page): Promise<any> {
|
||||
try {
|
||||
await session.send('DOM.getDocument', { depth: 0 });
|
||||
return session;
|
||||
} catch {
|
||||
// Session is stale — recreate
|
||||
} catch (err: any) {
|
||||
// Session is stale — recreate (CDP disconnects throw on closed/Target errors)
|
||||
if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('detached')) throw err;
|
||||
cdpSessions.delete(page);
|
||||
initializedPages.delete(page);
|
||||
}
|
||||
@@ -117,7 +118,9 @@ async function getOrCreateSession(page: Page): Promise<any> {
|
||||
page.once('framenavigated', () => {
|
||||
try {
|
||||
session.detach().catch(() => {});
|
||||
} catch {}
|
||||
} catch (err: any) {
|
||||
if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('detached')) throw err;
|
||||
}
|
||||
cdpSessions.delete(page);
|
||||
initializedPages.delete(page);
|
||||
});
|
||||
@@ -258,8 +261,9 @@ export async function inspectElement(
|
||||
left: border[0] - margin[0],
|
||||
},
|
||||
};
|
||||
} catch {
|
||||
// Element may not have a box model (e.g., display:none)
|
||||
} catch (err: any) {
|
||||
// Element may not have a box model (e.g., display:none) — CDP returns "Could not compute box model"
|
||||
if (!err?.message?.includes('box model') && !err?.message?.includes('Could not compute')) throw err;
|
||||
}
|
||||
|
||||
// Get matched styles
|
||||
@@ -315,10 +319,8 @@ export async function inspectElement(
|
||||
|
||||
if (rule.styleSheetId) {
|
||||
styleSheetId = rule.styleSheetId;
|
||||
try {
|
||||
// Try to resolve stylesheet URL
|
||||
source = rule.origin === 'regular' ? (rule.styleSheetId || 'stylesheet') : rule.origin;
|
||||
} catch {}
|
||||
// Resolve stylesheet source name
|
||||
source = rule.origin === 'regular' ? (rule.styleSheetId || 'stylesheet') : rule.origin;
|
||||
}
|
||||
|
||||
if (rule.style?.range) {
|
||||
@@ -328,15 +330,7 @@ export async function inspectElement(
|
||||
}
|
||||
|
||||
// Try to get a friendly source name from stylesheet
|
||||
if (styleSheetId) {
|
||||
try {
|
||||
// Stylesheet URL might be embedded in the rule data
|
||||
// CDP provides sourceURL in some cases
|
||||
if (rule.style?.cssText) {
|
||||
// Parse source from the styleSheetId metadata
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
// (styleSheetId metadata is available via CDP — see stylesheet URL resolution below)
|
||||
|
||||
// Get media query if present
|
||||
let media: string | undefined;
|
||||
@@ -433,15 +427,9 @@ export async function inspectElement(
|
||||
}
|
||||
|
||||
// Resolve stylesheet URLs for better source info
|
||||
for (const rule of matchedRules) {
|
||||
if (rule.styleSheetId && rule.source !== 'inline') {
|
||||
try {
|
||||
const sheetMeta = await session.send('CSS.getStyleSheetText', { styleSheetId: rule.styleSheetId }).catch(() => null);
|
||||
// Try to get the stylesheet header for URL info
|
||||
// The styleSheetId itself is opaque, but we can try to get source URL
|
||||
} catch {}
|
||||
}
|
||||
}
|
||||
// Note: CSS.getStyleSheetText is called per-rule but result is unused — the styleSheetId
|
||||
// is opaque and CDP doesn't expose a direct URL lookup. Left as a placeholder for future
|
||||
// enhancement (e.g., CSS.styleSheetAdded event tracking).
|
||||
|
||||
return {
|
||||
selector,
|
||||
@@ -531,8 +519,9 @@ export async function modifyStyle(
|
||||
method = 'setStyleTexts';
|
||||
source = `${targetRule.source}:${targetRule.sourceLine}`;
|
||||
sourceLine = targetRule.sourceLine;
|
||||
} catch {
|
||||
// Fall back to inline
|
||||
} catch (err: any) {
|
||||
// Fall back to inline — setStyleTexts fails on immutable stylesheets or stale ranges
|
||||
if (!err?.message?.includes('style') && !err?.message?.includes('range') && !err?.message?.includes('closed') && !err?.message?.includes('Target')) throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -591,8 +580,9 @@ export async function undoModification(page: Page, index?: number): Promise<void
|
||||
await modifyStyle(page, mod.selector, mod.property, mod.oldValue);
|
||||
// Remove the undo modification from history (it's a restore, not a new mod)
|
||||
modificationHistory.pop();
|
||||
} catch {
|
||||
// Fall back to inline restore
|
||||
} catch (err: any) {
|
||||
// Fall back to inline restore — CDP may have disconnected or stylesheet changed
|
||||
if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('style') && !err?.message?.includes('not found') && !err?.message?.includes('Element')) throw err;
|
||||
await page.evaluate(
|
||||
([sel, prop, val]) => {
|
||||
const el = document.querySelector(sel);
|
||||
@@ -652,8 +642,9 @@ export async function resetModifications(page: Page): Promise<void> {
|
||||
},
|
||||
[mod.selector, mod.property, mod.oldValue]
|
||||
);
|
||||
} catch {
|
||||
// Best effort
|
||||
} catch (err: any) {
|
||||
// Best effort — page may have navigated or element may be gone
|
||||
if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('Execution context')) throw err;
|
||||
}
|
||||
}
|
||||
modificationHistory.length = 0;
|
||||
@@ -757,7 +748,7 @@ export function detachSession(page?: Page): void {
|
||||
if (page) {
|
||||
const session = cdpSessions.get(page);
|
||||
if (session) {
|
||||
try { session.detach().catch(() => {}); } catch {}
|
||||
try { session.detach().catch(() => {}); } catch (err: any) { if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('detached')) throw err; }
|
||||
cdpSessions.delete(page);
|
||||
initializedPages.delete(page);
|
||||
}
|
||||
|
||||
+16
-10
@@ -248,8 +248,9 @@ export async function handleMetaCommand(
|
||||
try {
|
||||
commands = JSON.parse(jsonStr);
|
||||
if (!Array.isArray(commands)) throw new Error('not array');
|
||||
} catch {
|
||||
} catch (err: any) {
|
||||
// Fallback: pipe-delimited format "goto url | click @e5 | snapshot -ic"
|
||||
if (!(err instanceof SyntaxError) && err?.message !== 'not array') throw err;
|
||||
commands = jsonStr.split(' | ')
|
||||
.filter(seg => seg.trim().length > 0)
|
||||
.map(seg => tokenizePipeSegment(seg.trim()));
|
||||
@@ -291,7 +292,7 @@ export async function handleMetaCommand(
|
||||
} else {
|
||||
// Parse error from JSON result
|
||||
let errMsg = cr.result;
|
||||
try { errMsg = JSON.parse(cr.result).error || cr.result; } catch {}
|
||||
try { errMsg = JSON.parse(cr.result).error || cr.result; } catch (err: any) { if (!(err instanceof SyntaxError)) throw err; }
|
||||
results.push(`[${name}] ERROR: ${errMsg}`);
|
||||
}
|
||||
lastWasWrite = WRITE_COMMANDS.has(name);
|
||||
@@ -431,8 +432,9 @@ export async function handleMetaCommand(
|
||||
execSync(`osascript -e 'tell application "${appName}" to activate'`, { stdio: 'pipe', timeout: 3000 });
|
||||
activated = true;
|
||||
break;
|
||||
} catch {
|
||||
// Try next browser
|
||||
} catch (err: any) {
|
||||
// Try next browser — osascript fails if app not found or AppleScript errors
|
||||
if (err?.status === undefined && !err?.message?.includes('Command failed')) throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -448,8 +450,9 @@ export async function handleMetaCommand(
|
||||
await resolved.locator.scrollIntoViewIfNeeded({ timeout: 5000 });
|
||||
return `Browser activated. Scrolled ${args[0]} into view.`;
|
||||
}
|
||||
} catch {
|
||||
// Ref not found — still activated the browser
|
||||
} catch (err: any) {
|
||||
// Ref not found or element gone — still activated the browser
|
||||
if (!err?.message?.includes('not found') && !err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('timeout')) throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -491,7 +494,9 @@ export async function handleMetaCommand(
|
||||
let gitRoot: string;
|
||||
try {
|
||||
gitRoot = execSync('git rev-parse --show-toplevel', { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }).trim();
|
||||
} catch {
|
||||
} catch (err: any) {
|
||||
// execSync throws with exit status on non-git directories
|
||||
if (err?.status === undefined && !err?.message?.includes('Command failed')) throw err;
|
||||
return 'Not in a git repository — cannot locate inbox.';
|
||||
}
|
||||
|
||||
@@ -514,8 +519,9 @@ export async function handleMetaCommand(
|
||||
url: data.page?.url || 'unknown',
|
||||
userMessage: data.userMessage || '',
|
||||
});
|
||||
} catch {
|
||||
// Skip malformed files
|
||||
} catch (err: any) {
|
||||
// Skip malformed JSON or unreadable files
|
||||
if (!(err instanceof SyntaxError) && err?.code !== 'ENOENT' && err?.code !== 'EACCES') throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -537,7 +543,7 @@ export async function handleMetaCommand(
|
||||
// Handle --clear flag
|
||||
if (args.includes('--clear')) {
|
||||
for (const file of files) {
|
||||
try { fs.unlinkSync(path.join(inboxDir, file)); } catch {}
|
||||
try { fs.unlinkSync(path.join(inboxDir, file)); } catch (err: any) { if (err?.code !== 'ENOENT') throw err; }
|
||||
}
|
||||
lines.push(`Cleared ${files.length} message${files.length === 1 ? '' : 's'}.`);
|
||||
}
|
||||
|
||||
+15
-8
@@ -331,7 +331,9 @@ export async function handleSnapshot(
|
||||
output.push(`@${ref} [${elem.reason}] "${elem.text}"`);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
} catch (err: any) {
|
||||
// Cursor scan fails on pages with strict CSP or when page has navigated
|
||||
if (!err?.message?.includes('Execution context') && !err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('Content Security')) throw err;
|
||||
output.push('');
|
||||
output.push('(cursor scan failed — CSP restriction)');
|
||||
}
|
||||
@@ -355,7 +357,7 @@ export async function handleSnapshot(
|
||||
const nodeFs = require('fs') as typeof import('fs');
|
||||
const absolute = nodePath.resolve(screenshotPath);
|
||||
const safeDirs = [TEMP_DIR, process.cwd()].map((d: string) => {
|
||||
try { return nodeFs.realpathSync(d); } catch { return d; }
|
||||
try { return nodeFs.realpathSync(d); } catch (err: any) { if (err?.code !== 'ENOENT') throw err; return d; }
|
||||
});
|
||||
let realPath: string;
|
||||
try {
|
||||
@@ -365,7 +367,8 @@ export async function handleSnapshot(
|
||||
try {
|
||||
const dir = nodeFs.realpathSync(nodePath.dirname(absolute));
|
||||
realPath = nodePath.join(dir, nodePath.basename(absolute));
|
||||
} catch {
|
||||
} catch (err2: any) {
|
||||
if (err2?.code !== 'ENOENT') throw err2;
|
||||
realPath = absolute;
|
||||
}
|
||||
} else {
|
||||
@@ -385,8 +388,9 @@ export async function handleSnapshot(
|
||||
if (box) {
|
||||
boxes.push({ ref: `@${ref}`, box });
|
||||
}
|
||||
} catch {
|
||||
// Element may be offscreen or hidden — skip
|
||||
} catch (err: any) {
|
||||
// Element may be offscreen, hidden, or page navigated — skip
|
||||
if (!err?.message?.includes('Timeout') && !err?.message?.includes('timeout') && !err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('Execution context')) throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -418,13 +422,16 @@ export async function handleSnapshot(
|
||||
|
||||
output.push('');
|
||||
output.push(`[annotated screenshot: ${screenshotPath}]`);
|
||||
} catch {
|
||||
// Remove overlays even on screenshot failure
|
||||
} catch (err: any) {
|
||||
// Remove overlays even on screenshot failure — but only swallow page/browser errors
|
||||
if (!err?.message?.includes('closed') && !err?.message?.includes('Target') && !err?.message?.includes('Execution context') && !err?.message?.includes('screenshot')) throw err;
|
||||
try {
|
||||
await page.evaluate(() => {
|
||||
document.querySelectorAll('.__browse_annotation__').forEach(el => el.remove());
|
||||
});
|
||||
} catch {}
|
||||
} catch (err2: any) {
|
||||
if (!err2?.message?.includes('closed') && !err2?.message?.includes('Target') && !err2?.message?.includes('Execution context')) throw err2;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -399,7 +399,7 @@ export async function handleWriteCommand(
|
||||
if (!fs.existsSync(fp)) throw new Error(`File not found: ${fp}`);
|
||||
if (path.isAbsolute(fp)) {
|
||||
let resolvedFp: string;
|
||||
try { resolvedFp = fs.realpathSync(path.resolve(fp)); } catch { resolvedFp = path.resolve(fp); }
|
||||
try { resolvedFp = fs.realpathSync(path.resolve(fp)); } catch (err: any) { if (err?.code !== 'ENOENT') throw err; resolvedFp = path.resolve(fp); }
|
||||
if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolvedFp, dir))) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
@@ -455,7 +455,7 @@ export async function handleWriteCommand(
|
||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||
const raw = fs.readFileSync(filePath, 'utf-8');
|
||||
let cookies: any[];
|
||||
try { cookies = JSON.parse(raw); } catch { throw new Error(`Invalid JSON in ${filePath}`); }
|
||||
try { cookies = JSON.parse(raw); } catch (err: any) { throw new Error(`Invalid JSON in ${filePath}: ${err?.message || err}`); }
|
||||
if (!Array.isArray(cookies)) throw new Error('Cookie file must contain a JSON array');
|
||||
|
||||
// Auto-fill domain from current page URL when missing (consistent with cookie command)
|
||||
@@ -520,8 +520,9 @@ export async function handleWriteCommand(
|
||||
const pickerUrl = `http://127.0.0.1:${port}/cookie-picker?code=${code}`;
|
||||
try {
|
||||
Bun.spawn(['open', pickerUrl], { stdout: 'ignore', stderr: 'ignore' });
|
||||
} catch {
|
||||
// open may fail silently — URL is in the message below
|
||||
} catch (err: any) {
|
||||
// open may fail on non-macOS or if 'open' binary is missing — URL is in the message below
|
||||
if (err?.code !== 'ENOENT' && !err?.message?.includes('spawn')) throw err;
|
||||
}
|
||||
|
||||
return `Cookie picker opened at http://127.0.0.1:${port}/cookie-picker\nDetected browsers: ${browsers.map(b => b.name).join(', ')}\nSelect domains to import, then close the picker when done.`;
|
||||
@@ -606,7 +607,10 @@ export async function handleWriteCommand(
|
||||
(el as HTMLElement).style.setProperty('display', 'none', 'important');
|
||||
removed++;
|
||||
});
|
||||
} catch {}
|
||||
} catch (err: any) {
|
||||
// querySelectorAll throws DOMException on invalid CSS selectors — skip those
|
||||
if (!(err instanceof DOMException)) throw err;
|
||||
}
|
||||
}
|
||||
return removed;
|
||||
}, selectors);
|
||||
@@ -815,7 +819,9 @@ export async function handleWriteCommand(
|
||||
document.querySelectorAll(sel).forEach(el => {
|
||||
(el as HTMLElement).style.display = 'none';
|
||||
});
|
||||
} catch {}
|
||||
} catch (err: any) {
|
||||
if (!(err instanceof DOMException)) throw err;
|
||||
}
|
||||
}
|
||||
// Also hide fixed/sticky (except nav)
|
||||
for (const el of document.querySelectorAll('*')) {
|
||||
@@ -838,7 +844,9 @@ export async function handleWriteCommand(
|
||||
document.querySelectorAll(sel).forEach(el => {
|
||||
(el as HTMLElement).style.display = 'none';
|
||||
});
|
||||
} catch {}
|
||||
} catch (err: any) {
|
||||
if (!(err instanceof DOMException)) throw err;
|
||||
}
|
||||
}
|
||||
}, hideSelectors);
|
||||
}
|
||||
@@ -950,13 +958,13 @@ export async function handleWriteCommand(
|
||||
reader.onerror = () => reject('Failed to read blob');
|
||||
reader.readAsDataURL(blob);
|
||||
});
|
||||
} catch {
|
||||
return 'ERROR:EXPIRED';
|
||||
} catch (err: any) {
|
||||
return `ERROR:EXPIRED:${err?.message || 'unknown'}`;
|
||||
}
|
||||
}, url);
|
||||
|
||||
if (dataUrl === 'ERROR:TOO_LARGE') throw new Error('Blob too large (>100MB). Use a different approach.');
|
||||
if (dataUrl === 'ERROR:EXPIRED') throw new Error('Blob URL expired or inaccessible.');
|
||||
if (dataUrl.startsWith('ERROR:EXPIRED')) throw new Error(`Blob URL expired or inaccessible: ${dataUrl.slice('ERROR:EXPIRED:'.length)}`);
|
||||
|
||||
const match = dataUrl.match(/^data:([^;]+);base64,(.+)$/);
|
||||
if (!match) throw new Error('Failed to decode blob data');
|
||||
|
||||
Reference in New Issue
Block a user