mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 11:45:20 +02:00
fix: onDisconnect async rejection leaves process running
The disconnect handler calls this.onDisconnect() without awaiting it, but server.ts wires the callback to shutdown(2) — which is async. If that promise rejects, the rejection drops on the floor as an unhandled rejection, the browser is already disconnected, and the server keeps running indefinitely with no browser attached. Add a sync try/catch for throws and a .catch() chain for promise rejections. Both fall back to process.exit(2) so a dead browser never leaves a live server. Also widen the callback type from `() => void` to `() => void | Promise<void>` to match the actual runtime shape of the wired shutdown(2) call. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -75,7 +75,8 @@ export class BrowserManager {
|
||||
// Called when the headed browser disconnects without intentional teardown
|
||||
// (user closed the window). Wired up by server.ts to run full cleanup
|
||||
// (sidebar-agent, state file, profile locks) before exiting with code 2.
|
||||
public onDisconnect: (() => void) | null = null;
|
||||
// Returns void or a Promise; rejections are caught and fall back to exit(2).
|
||||
public onDisconnect: (() => void | Promise<void>) | null = null;
|
||||
|
||||
getConnectionMode(): 'launched' | 'headed' { return this.connectionMode; }
|
||||
|
||||
@@ -475,15 +476,27 @@ export class BrowserManager {
|
||||
// Browser disconnect handler — exit code 2 distinguishes from crashes (1).
|
||||
// Calls onDisconnect() to trigger full shutdown (kill sidebar-agent, save
|
||||
// session, clean profile locks + state file) before exit. Falls back to
|
||||
// direct process.exit(2) if no callback is wired up.
|
||||
// direct process.exit(2) if no callback is wired up, or if the callback
|
||||
// throws/rejects — never leave the process running with a dead browser.
|
||||
if (this.browser) {
|
||||
this.browser.on('disconnected', () => {
|
||||
if (this.intentionalDisconnect) return;
|
||||
console.error('[browse] Real browser disconnected (user closed or crashed).');
|
||||
console.error('[browse] Run `$B connect` to reconnect.');
|
||||
if (this.onDisconnect) {
|
||||
this.onDisconnect();
|
||||
} else {
|
||||
if (!this.onDisconnect) {
|
||||
process.exit(2);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const result = this.onDisconnect();
|
||||
if (result && typeof (result as Promise<void>).catch === 'function') {
|
||||
(result as Promise<void>).catch((err) => {
|
||||
console.error('[browse] onDisconnect rejected:', err);
|
||||
process.exit(2);
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('[browse] onDisconnect threw:', err);
|
||||
process.exit(2);
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user