Skip to content

Commit 6c69928

Browse files
committed
fix(cli,browser): robust browser state management and error handling
• Fix CLI silently swallowing launch errors when using --headed/--proxy flags by checking response.success, not just network errors • Fix isLaunched() returning true for disconnected browsers by checking browser.isConnected() and cleaning up stale state • Add browser.on('disconnected') handler to proactively reset state when browser crashes or is closed externally Before: Closing browser window caused "Browser not launched. Call launch first" error on next command, requiring manual close to recover. After: Browser state is properly tracked; auto-relaunch works reliably. Fixes zombie daemon state issue on Windows (and likely other platforms).
1 parent 399fd7a commit 6c69928

3 files changed

Lines changed: 82 additions & 7 deletions

File tree

cli/src/main.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,10 +286,19 @@ fn main() {
286286
.insert("proxy".to_string(), proxy_obj);
287287
}
288288

289-
if let Err(e) = send_command(launch_cmd, &flags.session) {
290-
if !flags.json {
291-
eprintln!("{} Could not configure browser: {}", color::warning_indicator(), e);
289+
let err = match send_command(launch_cmd, &flags.session) {
290+
Ok(resp) if resp.success => None,
291+
Ok(resp) => Some(resp.error.unwrap_or_else(|| "Browser launch failed".to_string())),
292+
Err(e) => Some(e.to_string()),
293+
};
294+
295+
if let Some(msg) = err {
296+
if flags.json {
297+
println!(r#"{{"success":false,"error":"{}"}}"#, msg);
298+
} else {
299+
eprintln!("{} {}", color::error_indicator(), msg);
292300
}
301+
exit(1);
293302
}
294303
}
295304

src/browser.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,8 @@ describe('BrowserManager', () => {
404404
on: vi.fn(),
405405
},
406406
],
407-
close: vi.fn(),
407+
on: vi.fn(),
408+
close: vi.fn().mockResolvedValue(undefined),
408409
};
409410
const spy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any);
410411

src/browser.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,44 @@ export class BrowserManager {
9898
private recordingTempDir: string = '';
9999

100100
/**
101-
* Check if browser is launched
101+
* Check if browser is launched and usable
102+
* Returns false if browser was launched but has since disconnected or has no pages
102103
*/
103104
isLaunched(): boolean {
104-
return this.browser !== null || this.isPersistentContext;
105+
// For persistent context (extensions), check if we have usable pages
106+
if (this.isPersistentContext) {
107+
// Filter out any closed pages (handles crash before 'close' event fires)
108+
this.pages = this.pages.filter((page) => !page.isClosed());
109+
if (this.pages.length === 0) {
110+
// All pages closed - clean up stale state
111+
this.contexts = [];
112+
this.isPersistentContext = false;
113+
return false;
114+
}
115+
return true;
116+
}
117+
118+
// For regular browser, check connection status and pages
119+
if (this.browser === null) {
120+
return false;
121+
}
122+
123+
// Check if browser is still connected (handles crash/close scenarios)
124+
if (!this.browser.isConnected()) {
125+
// Browser disconnected - clean up stale state
126+
this.browser = null;
127+
this.pages = [];
128+
this.contexts = [];
129+
this.cdpPort = null;
130+
return false;
131+
}
132+
133+
// Browser is connected but has no pages - not usable
134+
if (this.pages.length === 0) {
135+
return false;
136+
}
137+
138+
return true;
105139
}
106140

107141
/**
@@ -694,12 +728,32 @@ export class BrowserManager {
694728
}
695729
);
696730
this.isPersistentContext = true;
731+
732+
// Handle context close (crash, manual close, etc.) for persistent contexts
733+
// Note: BrowserContext uses 'close' event, not 'disconnected' like Browser
734+
context.on('close', () => {
735+
this.browser = null;
736+
this.pages = [];
737+
this.contexts = [];
738+
this.cdpPort = null;
739+
this.isPersistentContext = false;
740+
});
697741
} else {
698742
this.browser = await launcher.launch({
699743
headless: options.headless ?? true,
700744
executablePath: options.executablePath,
701745
});
702746
this.cdpPort = null;
747+
748+
// Handle browser disconnect (crash, manual close, etc.)
749+
this.browser.on('disconnected', () => {
750+
this.browser = null;
751+
this.pages = [];
752+
this.contexts = [];
753+
this.cdpPort = null;
754+
this.isPersistentContext = false;
755+
});
756+
703757
context = await this.browser.newContext({
704758
viewport,
705759
extraHTTPHeaders: options.headers,
@@ -749,6 +803,15 @@ export class BrowserManager {
749803
this.browser = browser;
750804
this.cdpPort = cdpPort;
751805

806+
// Handle browser disconnect (crash, manual close, etc.)
807+
this.browser.on('disconnected', () => {
808+
this.browser = null;
809+
this.pages = [];
810+
this.contexts = [];
811+
this.cdpPort = null;
812+
this.isPersistentContext = false;
813+
});
814+
752815
for (const context of contexts) {
753816
this.contexts.push(context);
754817
this.setupContextTracking(context);
@@ -762,7 +825,9 @@ export class BrowserManager {
762825
this.activePageIndex = 0;
763826
} catch (error) {
764827
// Clean up browser connection if validation or setup failed
765-
await browser.close().catch(() => {});
828+
if (browser) {
829+
await browser.close().catch(() => {});
830+
}
766831
throw error;
767832
}
768833
}

0 commit comments

Comments
 (0)