From 4d88a288e244b8b248ea44130f3f0b5e0c37efdd Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 10:59:39 +0530 Subject: [PATCH 1/9] fix: relaunch Chromium on disconnect and fail health check when it is gone Chromium runs as a module-level singleton with no reconnect. When it dies the Bun server keeps listening, so every subsequent request fails with "Target page, context or browser has been closed" until something restarts the container, and /v1/health reports the failure with a 200 so no probe can act on it. Resolve the browser through getBrowser(), which relaunches when the instance is disconnected and shares one relaunch between concurrent callers, and return 503 from /v1/health so an httpGet liveness probe can restart the pod. --- src/config/browser.ts | 45 ++++++++++++++++++++++++++++++++++----- src/routes/health.ts | 8 +++++-- src/routes/reports.ts | 3 ++- src/routes/screenshots.ts | 3 ++- src/routes/test.ts | 3 ++- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/config/browser.ts b/src/config/browser.ts index 031b821..f55f9be 100644 --- a/src/config/browser.ts +++ b/src/config/browser.ts @@ -1,5 +1,5 @@ import { chromium } from "playwright-core"; -import type { BrowserContextOptions } from "playwright-core"; +import type { Browser, BrowserContextOptions } from "playwright-core"; export const defaultContext: BrowserContextOptions = { viewport: { @@ -8,11 +8,46 @@ export const defaultContext: BrowserContextOptions = { }, }; +function launch(): Promise { + return chromium.launch({ + args: ["--remote-debugging-port=9222"], + executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH, + }); +} + console.log("Chromium starting..."); -export const browser = await chromium.launch({ - args: ["--remote-debugging-port=9222"], - executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH, -}); +let instance = await launch(); console.log("Chromium started!"); + +let launching: Promise | null = null; + +/** + * Chromium can die without taking the server process with it, and every request + * after that fails with "Target page, context or browser has been closed" until + * something restarts the container. Relaunch on demand so a dead browser costs + * one request instead of all of them. Concurrent callers share one relaunch. + */ +export async function getBrowser(): Promise { + if (instance.isConnected()) { + return instance; + } + + console.log("Chromium disconnected, relaunching..."); + + launching ??= launch() + .then((browser) => { + instance = browser; + return browser; + }) + .finally(() => { + launching = null; + }); + + return launching; +} + +export function isBrowserConnected(): boolean { + return instance.isConnected(); +} diff --git a/src/routes/health.ts b/src/routes/health.ts index 2ad664d..c48d3b1 100755 --- a/src/routes/health.ts +++ b/src/routes/health.ts @@ -1,11 +1,15 @@ -import { browser } from "../config"; +import { isBrowserConnected } from "../config"; export async function handleHealthRequest(_req: Request): Promise { + const connected = isBrowserConnected(); + + // Non-2xx so an httpGet liveness probe restarts a pod whose Chromium died. return new Response( JSON.stringify({ - status: browser.isConnected() ? "pass" : "fail", + status: connected ? "pass" : "fail", }), { + status: connected ? 200 : 503, headers: { "Content-Type": "application/json" }, }, ); diff --git a/src/routes/reports.ts b/src/routes/reports.ts index 6d7f6a9..e3589b8 100755 --- a/src/routes/reports.ts +++ b/src/routes/reports.ts @@ -1,6 +1,6 @@ import type { BrowserContext, BrowserContextOptions } from "playwright-core"; import { playAudit } from "playwright-lighthouse"; -import { browser, defaultContext, lighthouseConfigs } from "../config"; +import { defaultContext, getBrowser, lighthouseConfigs } from "../config"; import { lighthouseSchema } from "../schemas"; export async function handleReportsRequest(req: Request): Promise { @@ -21,6 +21,7 @@ export async function handleReportsRequest(req: Request): Promise { if (body.locale) contextOptions.locale = body.locale; if (body.timezoneId) contextOptions.timezoneId = body.timezoneId; + const browser = await getBrowser(); context = await browser.newContext(contextOptions); // Grant permissions if specified diff --git a/src/routes/screenshots.ts b/src/routes/screenshots.ts index 0ebf6c4..d2608c5 100755 --- a/src/routes/screenshots.ts +++ b/src/routes/screenshots.ts @@ -3,7 +3,7 @@ import type { BrowserContextOptions, PageScreenshotOptions, } from "playwright-core"; -import { browser, defaultContext } from "../config"; +import { defaultContext, getBrowser } from "../config"; import { screenshotSchema } from "../schemas"; export async function handleScreenshotsRequest( @@ -31,6 +31,7 @@ export async function handleScreenshotsRequest( if (body.timezoneId) contextOptions.timezoneId = body.timezoneId; if (body.geolocation) contextOptions.geolocation = body.geolocation; + const browser = await getBrowser(); context = await browser.newContext(contextOptions); // Grant permissions if specified diff --git a/src/routes/test.ts b/src/routes/test.ts index c8c2a1d..77d248f 100755 --- a/src/routes/test.ts +++ b/src/routes/test.ts @@ -1,7 +1,8 @@ -import { browser, defaultContext } from "../config"; +import { defaultContext, getBrowser } from "../config"; import { generateTestHTML } from "../utils/test-page.js"; export async function handleTestRequest(_req: Request): Promise { + const browser = await getBrowser(); const context = await browser.newContext(defaultContext); try { const page = await context.newPage(); From cb1d778b1235bb3c0779569454f17b48ef8cd4b7 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 11:05:00 +0530 Subject: [PATCH 2/9] style: use line comments to match the rest of src --- src/config/browser.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/config/browser.ts b/src/config/browser.ts index f55f9be..417a639 100644 --- a/src/config/browser.ts +++ b/src/config/browser.ts @@ -23,12 +23,10 @@ console.log("Chromium started!"); let launching: Promise | null = null; -/** - * Chromium can die without taking the server process with it, and every request - * after that fails with "Target page, context or browser has been closed" until - * something restarts the container. Relaunch on demand so a dead browser costs - * one request instead of all of them. Concurrent callers share one relaunch. - */ +// Chromium can die without taking the server process with it, and every request +// after that fails with "Target page, context or browser has been closed" until +// something restarts the container. Relaunch on demand so a dead browser costs +// one request instead of all of them. Concurrent callers share one relaunch. export async function getBrowser(): Promise { if (instance.isConnected()) { return instance; From 979b2c226436f2713291c5e4d5668a9106ed07d5 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 11:15:38 +0530 Subject: [PATCH 3/9] test: cover browser recovery after Chromium dies Kills Chromium inside the running container and asserts the service reports 503 while it is gone, recovers on the next capture and reports 200 again. Runs as its own suite and CI step because killing the browser mid-flight fails whatever else is using it, and skips when no container is resolvable so it stays runnable outside CI. --- .github/workflows/test.yml | 3 +++ package.json | 1 + tests/recovery/browser.test.ts | 47 ++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 tests/recovery/browser.test.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f63286c..3bba435 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -105,6 +105,9 @@ jobs: - name: Run e2e tests run: bun test:e2e + - name: Run browser recovery test + run: bun test:recovery + - name: Print logs if: failure() run: docker compose logs \ No newline at end of file diff --git a/package.json b/package.json index e5636f7..93296ae 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "check": "eslint ./src ./tests", "test": "bun test", "test:e2e": "bun test tests/e2e", + "test:recovery": "bun test tests/recovery", "test:unit": "bun test tests/unit" }, "keywords": [], diff --git a/tests/recovery/browser.test.ts b/tests/recovery/browser.test.ts new file mode 100644 index 0000000..d92cef7 --- /dev/null +++ b/tests/recovery/browser.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, test } from "bun:test"; + +const BASE_URL = process.env.BASE_URL || "http://localhost:3000"; + +// procps is not in the image, so Chromium is found by walking /proc. The shell +// running this is skipped because its own cmdline contains the pattern too. +const KILL_CHROMIUM = [ + "for p in /proc/[0-9]*; do", + ' pid="${p##*/}";', + ' [ "$pid" = "$$" ] && continue;', + ' grep -qa headless-shell "$p/cmdline" 2>/dev/null && kill -9 "$pid";', + "done", +].join("\n"); + +function resolveContainer(): string { + const result = Bun.spawnSync([ + "docker", + "compose", + "ps", + "-q", + "appwrite-browser", + ]); + + return result.success ? result.stdout.toString().trim() : ""; +} + +const container = resolveContainer(); + +describe.skipIf(container === "")("E2E Tests - browser recovery", () => { + test("should report 503 while Chromium is gone and recover on the next capture", async () => { + Bun.spawnSync(["docker", "exec", container, "sh", "-c", KILL_CHROMIUM]); + await Bun.sleep(2000); + + const down = await fetch(`${BASE_URL}/v1/health`); + expect(down.status).toBe(503); + + const capture = await fetch(`${BASE_URL}/v1/screenshots`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com" }), + }); + expect(capture.status).toBe(200); + + const up = await fetch(`${BASE_URL}/v1/health`); + expect(up.status).toBe(200); + }, 120_000); +}); From 06b2809e5ea4c06e95cebb0ff2cdb344567b9bb0 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 14:34:22 +0530 Subject: [PATCH 4/9] refactor: drop the one-line health helper and simplify the relaunch guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export the browser instance as a live binding instead of wrapping it in isBrowserConnected(); health.ts reads it directly, same as it did before this was a mutable reference. Replace the nullish-coalescing-assignment promise chain with a plain if guard around launch(). Same single-flight behaviour — concurrent callers see launching non-null and await the same promise — but reads as an ordinary guarded assignment instead of an unfamiliar idiom. --- src/config/browser.ts | 20 +++++++------------- src/routes/health.ts | 4 ++-- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/config/browser.ts b/src/config/browser.ts index 417a639..25a2a1e 100644 --- a/src/config/browser.ts +++ b/src/config/browser.ts @@ -17,7 +17,7 @@ function launch(): Promise { console.log("Chromium starting..."); -let instance = await launch(); +export let instance = await launch(); console.log("Chromium started!"); @@ -32,20 +32,14 @@ export async function getBrowser(): Promise { return instance; } - console.log("Chromium disconnected, relaunching..."); - - launching ??= launch() - .then((browser) => { - instance = browser; - return browser; - }) - .finally(() => { + if (!launching) { + console.log("Chromium disconnected, relaunching..."); + launching = launch().finally(() => { launching = null; }); + } - return launching; -} + instance = await launching; -export function isBrowserConnected(): boolean { - return instance.isConnected(); + return instance; } diff --git a/src/routes/health.ts b/src/routes/health.ts index c48d3b1..ceba911 100755 --- a/src/routes/health.ts +++ b/src/routes/health.ts @@ -1,7 +1,7 @@ -import { isBrowserConnected } from "../config"; +import { instance } from "../config"; export async function handleHealthRequest(_req: Request): Promise { - const connected = isBrowserConnected(); + const connected = instance.isConnected(); // Non-2xx so an httpGet liveness probe restarts a pod whose Chromium died. return new Response( From ed2f7d3e62dd99684e698948e0f49ee509e770d6 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 14:39:33 +0530 Subject: [PATCH 5/9] test: crash Chromium for real instead of killing the OS process Killing headless-shell by walking /proc simulated the symptom (the process disappears) but not a real trigger, and depended on docker exec and a running compose container. Send the CDP Browser.crash command instead, the same one Chromium itself exposes for testing crash recovery, through a small test-support endpoint alongside the existing /v1/test. The recovery test now runs over plain HTTP against any running instance, no docker exec or container name resolution needed. --- src/routes/test.ts | 12 ++++++++++++ src/server.ts | 2 ++ tests/recovery/browser.test.ts | 28 ++-------------------------- 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/routes/test.ts b/src/routes/test.ts index 77d248f..7406a60 100755 --- a/src/routes/test.ts +++ b/src/routes/test.ts @@ -22,3 +22,15 @@ export async function handleTestRequest(_req: Request): Promise { await context.close(); } } + +// Browser.crash is the same CDP command Chromium itself exposes for testing +// crash recovery, so this exercises a real browser-level crash rather than +// simulating one by killing the OS process from outside. +export async function handleTestCrashRequest(_req: Request): Promise { + const browser = await getBrowser(); + const session = await browser.newBrowserCDPSession(); + + session.send("Browser.crash").catch(() => {}); + + return new Response("ok"); +} diff --git a/src/server.ts b/src/server.ts index 11c0872..57ef53f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,6 +3,7 @@ import { handleHealthRequest, handleReportsRequest, handleScreenshotsRequest, + handleTestCrashRequest, handleTestRequest, } from "./routes"; import { Router } from "./utils/router"; @@ -12,6 +13,7 @@ router.add("POST", "/v1/screenshots", handleScreenshotsRequest); router.add("POST", "/v1/reports", handleReportsRequest); router.add("GET", "/v1/health", handleHealthRequest); router.add("GET", "/v1/test", handleTestRequest); +router.add("POST", "/v1/test/crash", handleTestCrashRequest); const server = Bun.serve({ port, diff --git a/tests/recovery/browser.test.ts b/tests/recovery/browser.test.ts index d92cef7..7bb7250 100644 --- a/tests/recovery/browser.test.ts +++ b/tests/recovery/browser.test.ts @@ -2,33 +2,9 @@ import { describe, expect, test } from "bun:test"; const BASE_URL = process.env.BASE_URL || "http://localhost:3000"; -// procps is not in the image, so Chromium is found by walking /proc. The shell -// running this is skipped because its own cmdline contains the pattern too. -const KILL_CHROMIUM = [ - "for p in /proc/[0-9]*; do", - ' pid="${p##*/}";', - ' [ "$pid" = "$$" ] && continue;', - ' grep -qa headless-shell "$p/cmdline" 2>/dev/null && kill -9 "$pid";', - "done", -].join("\n"); - -function resolveContainer(): string { - const result = Bun.spawnSync([ - "docker", - "compose", - "ps", - "-q", - "appwrite-browser", - ]); - - return result.success ? result.stdout.toString().trim() : ""; -} - -const container = resolveContainer(); - -describe.skipIf(container === "")("E2E Tests - browser recovery", () => { +describe("E2E Tests - browser recovery", () => { test("should report 503 while Chromium is gone and recover on the next capture", async () => { - Bun.spawnSync(["docker", "exec", container, "sh", "-c", KILL_CHROMIUM]); + await fetch(`${BASE_URL}/v1/test/crash`, { method: "POST" }); await Bun.sleep(2000); const down = await fetch(`${BASE_URL}/v1/health`); From 004c7a8eeae3dfb2dd34f6c9085e0bf247a8f5a4 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 15:27:16 +0530 Subject: [PATCH 6/9] fix: gate the crash endpoint and trim an overlong comment /v1/test/crash destroys the shared browser instance, unlike /v1/test which is read-only, so it stays unregistered unless ENABLE_TEST_ROUTES=1 is set. The deployed chart won't set it, so the route doesn't exist in any reachable environment; docker-compose.yml sets it for local and CI runs. Confirmed 404 without the flag and 200 with it, both against the same image. --- docker-compose.yml | 3 ++- src/config/browser.ts | 5 +---- src/server.ts | 7 ++++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 4be5caa..3f69d98 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,4 +7,5 @@ services: ports: - "3000:3000" environment: - - PORT=3000 \ No newline at end of file + - PORT=3000 + - ENABLE_TEST_ROUTES=1 \ No newline at end of file diff --git a/src/config/browser.ts b/src/config/browser.ts index 25a2a1e..4a18840 100644 --- a/src/config/browser.ts +++ b/src/config/browser.ts @@ -23,10 +23,7 @@ console.log("Chromium started!"); let launching: Promise | null = null; -// Chromium can die without taking the server process with it, and every request -// after that fails with "Target page, context or browser has been closed" until -// something restarts the container. Relaunch on demand so a dead browser costs -// one request instead of all of them. Concurrent callers share one relaunch. +// Chromium can die without the server dying too, so relaunch on demand. export async function getBrowser(): Promise { if (instance.isConnected()) { return instance; diff --git a/src/server.ts b/src/server.ts index 57ef53f..b0dabb4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -13,7 +13,12 @@ router.add("POST", "/v1/screenshots", handleScreenshotsRequest); router.add("POST", "/v1/reports", handleReportsRequest); router.add("GET", "/v1/health", handleHealthRequest); router.add("GET", "/v1/test", handleTestRequest); -router.add("POST", "/v1/test/crash", handleTestCrashRequest); + +// Destroys the shared browser instance, so it stays out of the deployed +// image's reachable surface unless a caller opts in for local and CI runs. +if (process.env.ENABLE_TEST_ROUTES === "1") { + router.add("POST", "/v1/test/crash", handleTestCrashRequest); +} const server = Bun.serve({ port, From eeeda5ff648c99318a64b92749ebe833a2dd887e Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 15:32:37 +0530 Subject: [PATCH 7/9] test: drop the crash endpoint and reproduce the production kill directly The crash endpoint shipped test-only surface in the product image and needed an env gate to keep it unreachable. It was also less faithful than what it replaced: in production Chromium dies from the outside, OOM-killed, not from a CDP client asking it to crash. Send SIGKILL to headless-shell via pidof instead, which is exactly the production failure, and cover the relaunch logic the e2e cannot reach, shared in-flight relaunch and retry after a failed launch, with unit tests that mock playwright-core. --- docker-compose.yml | 3 +- src/routes/test.ts | 12 ----- src/server.ts | 7 --- tests/recovery/browser.test.ts | 27 ++++++++++- tests/unit/browser.test.ts | 86 ++++++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 23 deletions(-) create mode 100644 tests/unit/browser.test.ts diff --git a/docker-compose.yml b/docker-compose.yml index 3f69d98..4be5caa 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,5 +7,4 @@ services: ports: - "3000:3000" environment: - - PORT=3000 - - ENABLE_TEST_ROUTES=1 \ No newline at end of file + - PORT=3000 \ No newline at end of file diff --git a/src/routes/test.ts b/src/routes/test.ts index 7406a60..77d248f 100755 --- a/src/routes/test.ts +++ b/src/routes/test.ts @@ -22,15 +22,3 @@ export async function handleTestRequest(_req: Request): Promise { await context.close(); } } - -// Browser.crash is the same CDP command Chromium itself exposes for testing -// crash recovery, so this exercises a real browser-level crash rather than -// simulating one by killing the OS process from outside. -export async function handleTestCrashRequest(_req: Request): Promise { - const browser = await getBrowser(); - const session = await browser.newBrowserCDPSession(); - - session.send("Browser.crash").catch(() => {}); - - return new Response("ok"); -} diff --git a/src/server.ts b/src/server.ts index b0dabb4..11c0872 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,7 +3,6 @@ import { handleHealthRequest, handleReportsRequest, handleScreenshotsRequest, - handleTestCrashRequest, handleTestRequest, } from "./routes"; import { Router } from "./utils/router"; @@ -14,12 +13,6 @@ router.add("POST", "/v1/reports", handleReportsRequest); router.add("GET", "/v1/health", handleHealthRequest); router.add("GET", "/v1/test", handleTestRequest); -// Destroys the shared browser instance, so it stays out of the deployed -// image's reachable surface unless a caller opts in for local and CI runs. -if (process.env.ENABLE_TEST_ROUTES === "1") { - router.add("POST", "/v1/test/crash", handleTestCrashRequest); -} - const server = Bun.serve({ port, fetch: (request) => router.handle(request), diff --git a/tests/recovery/browser.test.ts b/tests/recovery/browser.test.ts index 7bb7250..21b23b0 100644 --- a/tests/recovery/browser.test.ts +++ b/tests/recovery/browser.test.ts @@ -2,9 +2,32 @@ import { describe, expect, test } from "bun:test"; const BASE_URL = process.env.BASE_URL || "http://localhost:3000"; -describe("E2E Tests - browser recovery", () => { +function resolveContainer(): string { + const result = Bun.spawnSync([ + "docker", + "compose", + "ps", + "-q", + "appwrite-browser", + ]); + + return result.success ? result.stdout.toString().trim() : ""; +} + +const container = resolveContainer(); + +describe.skipIf(container === "")("E2E Tests - browser recovery", () => { test("should report 503 while Chromium is gone and recover on the next capture", async () => { - await fetch(`${BASE_URL}/v1/test/crash`, { method: "POST" }); + // SIGKILL is what the OOM killer sends in production, where Chromium + // dies while the server keeps running. + Bun.spawnSync([ + "docker", + "exec", + container, + "sh", + "-c", + "kill -9 $(pidof headless-shell)", + ]); await Bun.sleep(2000); const down = await fetch(`${BASE_URL}/v1/health`); diff --git a/tests/unit/browser.test.ts b/tests/unit/browser.test.ts new file mode 100644 index 0000000..62c99ea --- /dev/null +++ b/tests/unit/browser.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, mock, test } from "bun:test"; + +type FakeBrowser = { + isConnected: () => boolean; + disconnect: () => void; +}; + +function fakeBrowser(): FakeBrowser { + let alive = true; + + return { + isConnected: () => alive, + disconnect: () => { + alive = false; + }, + }; +} + +let launchCount = 0; +let launchImpl: () => Promise = async () => fakeBrowser(); + +mock.module("playwright-core", () => ({ + chromium: { + launch: () => { + launchCount++; + return launchImpl(); + }, + }, +})); + +const { getBrowser } = await import("../../src/config/browser"); + +describe("getBrowser", () => { + test("should reuse the instance while it is connected", async () => { + const before = launchCount; + const first = await getBrowser(); + const second = await getBrowser(); + + expect(second).toBe(first); + expect(launchCount).toBe(before); + }); + + test("should relaunch once it disconnects", async () => { + const first = (await getBrowser()) as unknown as FakeBrowser; + first.disconnect(); + + const second = await getBrowser(); + + expect(second).not.toBe(first); + expect(second.isConnected()).toBe(true); + }); + + test("should share one relaunch between concurrent callers", async () => { + const current = (await getBrowser()) as unknown as FakeBrowser; + current.disconnect(); + + let release: (browser: FakeBrowser) => void = () => {}; + launchImpl = () => + new Promise((resolve) => { + release = resolve; + }); + + const before = launchCount; + const one = getBrowser(); + const two = getBrowser(); + release(fakeBrowser()); + + expect(await one).toBe(await two); + expect(launchCount).toBe(before + 1); + + launchImpl = async () => fakeBrowser(); + }); + + test("should retry after a failed relaunch", async () => { + const current = (await getBrowser()) as unknown as FakeBrowser; + current.disconnect(); + + launchImpl = () => Promise.reject(new Error("boot failed")); + await expect(getBrowser()).rejects.toThrow("boot failed"); + + launchImpl = async () => fakeBrowser(); + const recovered = await getBrowser(); + + expect(recovered.isConnected()).toBe(true); + }); +}); From 408d62b2c33d6ec9deee68fe9876d47ca9bdd956 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 15:47:22 +0530 Subject: [PATCH 8/9] test: prove the shared relaunch against a real browser instead of a mock Kill Chromium, fire five concurrent captures, and assert every one succeeds while the container logs show a single relaunch. A broken guard launches one Chromium per request, which the log count catches, so the mocked unit tests and their fake browser are no longer needed. Verified the assertion has teeth by removing the guard: five concurrent launches wedged the service past the test timeout. --- tests/recovery/browser.test.ts | 36 ++++++++++++++ tests/unit/browser.test.ts | 86 ---------------------------------- 2 files changed, 36 insertions(+), 86 deletions(-) delete mode 100644 tests/unit/browser.test.ts diff --git a/tests/recovery/browser.test.ts b/tests/recovery/browser.test.ts index 21b23b0..d63d9de 100644 --- a/tests/recovery/browser.test.ts +++ b/tests/recovery/browser.test.ts @@ -43,4 +43,40 @@ describe.skipIf(container === "")("E2E Tests - browser recovery", () => { const up = await fetch(`${BASE_URL}/v1/health`); expect(up.status).toBe(200); }, 120_000); + + test("should share one relaunch between concurrent captures", async () => { + Bun.spawnSync([ + "docker", + "exec", + container, + "sh", + "-c", + "kill -9 $(pidof headless-shell)", + ]); + await Bun.sleep(2000); + + const before = Bun.spawnSync(["docker", "logs", container]) + .stdout.toString() + .split("Chromium disconnected, relaunching...").length; + + const captures = await Promise.all( + Array.from({ length: 5 }, () => + fetch(`${BASE_URL}/v1/screenshots`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com" }), + }), + ), + ); + + for (const capture of captures) { + expect(capture.status).toBe(200); + } + + // A broken guard launches one Chromium per request instead of one total. + const after = Bun.spawnSync(["docker", "logs", container]) + .stdout.toString() + .split("Chromium disconnected, relaunching...").length; + expect(after).toBe(before + 1); + }, 120_000); }); diff --git a/tests/unit/browser.test.ts b/tests/unit/browser.test.ts deleted file mode 100644 index 62c99ea..0000000 --- a/tests/unit/browser.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { describe, expect, mock, test } from "bun:test"; - -type FakeBrowser = { - isConnected: () => boolean; - disconnect: () => void; -}; - -function fakeBrowser(): FakeBrowser { - let alive = true; - - return { - isConnected: () => alive, - disconnect: () => { - alive = false; - }, - }; -} - -let launchCount = 0; -let launchImpl: () => Promise = async () => fakeBrowser(); - -mock.module("playwright-core", () => ({ - chromium: { - launch: () => { - launchCount++; - return launchImpl(); - }, - }, -})); - -const { getBrowser } = await import("../../src/config/browser"); - -describe("getBrowser", () => { - test("should reuse the instance while it is connected", async () => { - const before = launchCount; - const first = await getBrowser(); - const second = await getBrowser(); - - expect(second).toBe(first); - expect(launchCount).toBe(before); - }); - - test("should relaunch once it disconnects", async () => { - const first = (await getBrowser()) as unknown as FakeBrowser; - first.disconnect(); - - const second = await getBrowser(); - - expect(second).not.toBe(first); - expect(second.isConnected()).toBe(true); - }); - - test("should share one relaunch between concurrent callers", async () => { - const current = (await getBrowser()) as unknown as FakeBrowser; - current.disconnect(); - - let release: (browser: FakeBrowser) => void = () => {}; - launchImpl = () => - new Promise((resolve) => { - release = resolve; - }); - - const before = launchCount; - const one = getBrowser(); - const two = getBrowser(); - release(fakeBrowser()); - - expect(await one).toBe(await two); - expect(launchCount).toBe(before + 1); - - launchImpl = async () => fakeBrowser(); - }); - - test("should retry after a failed relaunch", async () => { - const current = (await getBrowser()) as unknown as FakeBrowser; - current.disconnect(); - - launchImpl = () => Promise.reject(new Error("boot failed")); - await expect(getBrowser()).rejects.toThrow("boot failed"); - - launchImpl = async () => fakeBrowser(); - const recovered = await getBrowser(); - - expect(recovered.isConnected()).toBe(true); - }); -}); From d4036949873e3a886440178f9a44111705f8db4b Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 24 Aug 2026 15:55:55 +0530 Subject: [PATCH 9/9] test: poll for the disconnect instead of sleeping a fixed two seconds A fixed sleep is slower than needed when Playwright notices the dead browser in milliseconds and fails the run on a machine where it takes longer. Poll health until it reports 503, bounded at 30s. Health only reads isConnected, so polling cannot itself trigger the relaunch the second test counts. --- tests/recovery/browser.test.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/recovery/browser.test.ts b/tests/recovery/browser.test.ts index d63d9de..cd2f707 100644 --- a/tests/recovery/browser.test.ts +++ b/tests/recovery/browser.test.ts @@ -28,9 +28,14 @@ describe.skipIf(container === "")("E2E Tests - browser recovery", () => { "-c", "kill -9 $(pidof headless-shell)", ]); - await Bun.sleep(2000); - const down = await fetch(`${BASE_URL}/v1/health`); + // The server notices the dead browser asynchronously. + const deadline = Date.now() + 30_000; + let down = await fetch(`${BASE_URL}/v1/health`); + while (down.status !== 503 && Date.now() < deadline) { + await Bun.sleep(250); + down = await fetch(`${BASE_URL}/v1/health`); + } expect(down.status).toBe(503); const capture = await fetch(`${BASE_URL}/v1/screenshots`, { @@ -53,7 +58,14 @@ describe.skipIf(container === "")("E2E Tests - browser recovery", () => { "-c", "kill -9 $(pidof headless-shell)", ]); - await Bun.sleep(2000); + + const deadline = Date.now() + 30_000; + let health = await fetch(`${BASE_URL}/v1/health`); + while (health.status !== 503 && Date.now() < deadline) { + await Bun.sleep(250); + health = await fetch(`${BASE_URL}/v1/health`); + } + expect(health.status).toBe(503); const before = Bun.spawnSync(["docker", "logs", container]) .stdout.toString()