From 997c98cb1382f574a42de0db6a82d04f85cc5ba9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 11:10:37 +0000 Subject: [PATCH] fix(runtime): close browser after partial session teardown failures Promise.all in shutdown() aborted on the first context.close() error, skipping browser.close() and leaving orphan Chromium processes. Use allSettled with best-effort per-session cleanup and wrap companion SIGTERM/SIGINT handling so app.close() always runs. Co-authored-by: esadrianno --- packages/runtime/src/index.test.ts | 38 ++++++++++++++++++++++++++++++ packages/runtime/src/index.ts | 19 +++++++++++---- services/companion/src/index.ts | 9 ++++--- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/packages/runtime/src/index.test.ts b/packages/runtime/src/index.test.ts index 0b0c708..5a1d1cd 100644 --- a/packages/runtime/src/index.test.ts +++ b/packages/runtime/src/index.test.ts @@ -307,6 +307,44 @@ describe("BrowserRuntime", () => { expect(browserClose).toHaveBeenCalled(); }); + it("shuts down the browser even when a session context.close fails", async () => { + const failingClose = vi.fn().mockRejectedValue(new Error("close-boom")); + const okClose = vi.fn().mockResolvedValue(undefined); + const browserClose = vi.fn().mockResolvedValue(undefined); + const makeContext = (close: typeof failingClose) => ({ + newPage: vi.fn(async () => ({ + goto: vi.fn().mockResolvedValue(undefined), + url: vi.fn(() => "https://page.example"), + title: vi.fn(() => Promise.resolve("title")), + content: vi.fn(() => Promise.resolve("")), + evaluate: vi.fn().mockResolvedValue(undefined), + $$eval: vi.fn().mockResolvedValue([]), + locator: vi.fn(() => ({ + first: () => ({ click: vi.fn(), fill: vi.fn() }), + ariaSnapshot: vi.fn().mockResolvedValue(""), + })), + })), + close, + }); + const browser = { + newContext: vi + .fn() + .mockResolvedValueOnce(makeContext(failingClose)) + .mockResolvedValueOnce(makeContext(okClose)), + close: browserClose, + }; + mockChromiumLaunch.mockResolvedValueOnce(browser); + + const rt = new BrowserRuntime({ headless: true }); + await rt.createSession(); + await rt.createSession(); + await rt.shutdown(); + + expect(failingClose).toHaveBeenCalled(); + expect(okClose).toHaveBeenCalled(); + expect(browserClose).toHaveBeenCalled(); + }); + it("falls back to webkit when Chromium is not installed", async () => { const { browser } = createMockBrowserTree(); mockChromiumLaunch.mockRejectedValueOnce( diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 6e1c945..4c0ec09 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -211,16 +211,25 @@ export class BrowserRuntime { } async shutdown() { - await Promise.all( - [...this.sessions.values()].map(async (session) => { - await session.context.close(); + const sessions = [...this.sessions.values()]; + await Promise.allSettled( + sessions.map(async (session) => { + try { + await session.context.close(); + } catch { + // Best-effort per-session teardown during shutdown. + } }), ); this.sessions.clear(); if (this.browserPromise) { - const browser = await this.browserPromise; - await browser.close(); + try { + const browser = await this.browserPromise; + await browser.close(); + } catch { + // Browser may already be closed. + } this.browserPromise = null; } } diff --git a/services/companion/src/index.ts b/services/companion/src/index.ts index 1797d20..d27d0d4 100644 --- a/services/companion/src/index.ts +++ b/services/companion/src/index.ts @@ -10,9 +10,12 @@ const runtime = new BrowserRuntime({ const { app } = await createCompanionApp({ runtime }); const closeGracefully = async () => { - await runtime.shutdown(); - await app.close(); - process.exit(0); + try { + await runtime.shutdown(); + } finally { + await app.close(); + process.exit(0); + } }; process.on("SIGINT", closeGracefully);