Skip to content

fix: relaunch Chromium on disconnect and fail health check when it is gone - #17

Open
HarshMN2345 wants to merge 9 commits into
mainfrom
fix/browser-relaunch-on-disconnect
Open

fix: relaunch Chromium on disconnect and fail health check when it is gone#17
HarshMN2345 wants to merge 9 commits into
mainfrom
fix/browser-relaunch-on-disconnect

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Aug 24, 2026

Copy link
Copy Markdown
Member

Problem

Chromium is launched once at module load and never re-checked:

export const browser = await chromium.launch({ ... });

When it dies, the Bun server keeps listening. Every request after that fails with Target page, context or browser has been closed, and it stays that way until something unrelated restarts the container.

/v1/health already reports the right thing — browser.isConnected() — but returns it with HTTP 200, so no probe can act on it.

Observed in production: sustained failures at roughly this rate for days, each failing in well under a second, with zero pod restarts. It stopped only when a rollout happened to restart the pods.

Change

  • getBrowser() resolves the browser and relaunches it when the instance is disconnected. Concurrent callers share a single in-flight relaunch, and the guard is cleared on failure so a failed relaunch is retried on the next request.
  • /v1/health returns 503 when disconnected, so an httpGet liveness probe can restart the pod.
  • screenshots, reports and test resolve through getBrowser() instead of importing the singleton.

Verification

Ran the service and killed Chromium out from under it.

Step Before After
Health, browser alive 200 pass 200 pass
Kill Chromium → health 200 fail 503 fail
Screenshot request 400 "context: Target page, context or browser has been closed" 200, valid PNG
Health after 200 fail, stays broken 200 pass, self-healed

The "before" column reproduces the production error string exactly.

bun run type-check and bun run lint pass.

Tests

tests/recovery/browser.test.ts, running against the real image as its own CI step:

  1. Recovery — SIGKILL headless-shell (what the OOM killer sends in production), assert /v1/health returns 503, a capture succeeds (relaunch), and health returns 200 again.
  2. Shared relaunch — kill Chromium, fire five concurrent captures, assert all succeed and the container logs show exactly one relaunch. Verified the assertion has teeth: with the sharing guard removed, five concurrent launches wedged the service past the test timeout.

No mocks, no test-only surface in the image. The suite runs separately from the other e2e tests because killing the browser mid-flight fails whatever else is using it, and skips when no container is resolvable.

Note for deployment

The 503 only has an effect once the liveness probe actually calls /v1/health. The chart currently uses tcpSocket, which only checks that Bun is listening — that needs a separate change to httpGet on this path, plus a readiness probe so a pod with a dead browser leaves the Service.

… 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.
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

Docker Image Stats

Metric Value
Image Size 494MB
Memory Usage 156.6MiB
Cold Start Time 1.04s
Screenshot Time 2.82s

Screenshot benchmark: Average of 3 runs on https://appwrite.io

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds on-demand Chromium recovery and makes browser disconnection visible through the health endpoint.

  • Shares one in-flight relaunch among concurrent browser requests and permits retries after launch failures.
  • Routes screenshot, report, and test operations through the recoverable browser accessor.
  • Returns HTTP 503 from the health endpoint while Chromium is disconnected.
  • Adds container-level recovery and concurrent-relaunch tests to CI.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
src/config/browser.ts Introduces a shared, retryable Chromium relaunch promise and exposes the current browser instance for health checks.
src/routes/health.ts Changes disconnected browser health responses from HTTP 200 to HTTP 503 while retaining the existing JSON status.
src/routes/screenshots.ts Resolves the shared browser through the recovery accessor before creating each screenshot context.
src/routes/reports.ts Resolves the shared browser through the recovery accessor before creating each Lighthouse context.
src/routes/test.ts Uses the recovery accessor before creating the diagnostic route's browser context.
tests/recovery/browser.test.ts Adds real-container coverage for recovery after SIGKILL and for sharing one relaunch among concurrent requests.
.github/workflows/test.yml Adds the browser recovery suite as a separate CI step.
package.json Adds a dedicated command for running the recovery tests.

Reviews (9): Last reviewed commit: "test: poll for the disconnect instead of..." | Re-trigger Greptile

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.
Comment thread src/config/browser.ts Outdated
Comment thread src/config/browser.ts Outdated
…uard

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.
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.
/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.
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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants