fix(web-scraper): re-try local browser launch and do not cache failed launch promise

Author: azasypkin

components/retrack-web-scraper/src/index.ts

@@ -1,23 +1,14 @@
 import { fastify } from 'fastify';
-import type { BrowserServer } from 'playwright-core';
 
 import { registerRoutes } from './api/index.js';
-import type { LocalBrowserConfig } from './config.js';
 import { configure } from './config.js';
-import { stopBrowserServer } from './utilities/browser.js';
-import { launchBrowserServer } from './utilities/browser.js';
+import { createBrowserServerManager } from './utilities/browser.js';
 
 const config = configure();
 if (!config.browser.chromium && !config.browser.firefox) {
   throw new Error('At least one browser (Chromium or Firefox) should be configured.');
 }
 
-const browserServer: {
-  server?: Promise<BrowserServer>;
-  shutdownInProgress?: Promise<void>;
-  shutdownTimer?: NodeJS.Timeout;
-} = {};
-
 const server = fastify({
   bodyLimit: config.server.bodyLimit,
   logger: config.isDev
@@ -26,52 +17,23 @@ const server = fastify({
         transport: { target: 'pino-pretty', options: { translateTime: 'HH:MM:ss Z', ignore: 'pid,hostname' } },
       }
     : { level: config.logLevel },
-}).addHook('onClose', async () => {
-  if (browserServer.server) {
-    await browserServer.server.then((localServer) => stopBrowserServer(logger, localServer)).catch(() => {});
-  }
 });
 
 const logger = server.log;
 logger.debug(`Configuration: ${JSON.stringify(config, null, 2)}.`);
 
+const browserServerManager = createBrowserServerManager(logger);
+server.addHook('onClose', async () => {
+  await browserServerManager.close();
+});
+
 await server.register(import('@fastify/compress'));
 
 registerRoutes({
   server,
   config,
-  isLocalBrowserServerRunning: () => !!browserServer.server,
-  getLocalBrowserServer: async (locaConfig: LocalBrowserConfig) => {
-    if (!browserServer.server) {
-      browserServer.server = (browserServer.shutdownInProgress ?? Promise.resolve()).then(() =>
-        launchBrowserServer(logger, locaConfig),
-      );
-    }
-
-    if (browserServer.shutdownTimer) {
-      clearTimeout(browserServer.shutdownTimer);
-    }
-    browserServer.shutdownTimer = setTimeout(() => {
-      if (!browserServer.server) {
-        return;
-      }
-
-      const browserServerInstance = browserServer.server;
-      browserServer.server = undefined;
-
-      clearTimeout(browserServer.shutdownTimer);
-      browserServer.shutdownTimer = undefined;
-
-      browserServer.shutdownInProgress = browserServerInstance
-        .then((server) => stopBrowserServer(logger, server))
-        .catch(() => {})
-        .finally(() => {
-          browserServer.shutdownInProgress = undefined;
-        });
-    }, locaConfig.ttlSec * 1000);
-
-    return browserServer.server;
-  },
+  isLocalBrowserServerRunning: () => browserServerManager.isRunning(),
+  getLocalBrowserServer: (locaConfig) => browserServerManager.get(locaConfig),
 });
 
 server.listen({ port: config.port, host: '0.0.0.0' }, (err, address) => {

components/retrack-web-scraper/src/tests.ts

@@ -1,2 +1,3 @@
 import './api/status/get.test.js';
 import './api/web_page/execute.test.js';
+import './utilities/browser.test.js';

components/retrack-web-scraper/src/utilities/browser.test.ts

@@ -0,0 +1,89 @@
+import * as assert from 'node:assert/strict';
+import { mock, test } from 'node:test';
+import type { FastifyBaseLogger } from 'fastify';
+import type { BrowserServer } from 'playwright-core';
+
+import type { LocalBrowserConfig } from '../config.js';
+import { createBrowserServerManager } from './browser.js';
+
+function createLoggerMock(): FastifyBaseLogger {
+  const logger = {
+    info: () => {},
+    error: () => {},
+    warn: () => {},
+    debug: () => {},
+    trace: () => {},
+    fatal: () => {},
+    silent: () => {},
+    level: 'silent',
+  } as unknown as FastifyBaseLogger;
+  (logger as unknown as { child: () => FastifyBaseLogger }).child = () => logger;
+  return logger;
+}
+
+function createServerMock(wsEndpoint: string) {
+  return {
+    wsEndpoint: () => wsEndpoint,
+    close: mock.fn(() => Promise.resolve()),
+  } as unknown as BrowserServer;
+}
+
+const LOCAL_CONFIG: LocalBrowserConfig = {
+  protocol: 'playwright',
+  backend: 'chromium',
+  executablePath: '/usr/local/bin/chromium',
+  // Large TTL so the idle shutdown never fires during the test.
+  ttlSec: 3600,
+  headless: true,
+  chromiumSandbox: false,
+};
+
+await test('[browser server manager] launches a single shared server for concurrent callers', async () => {
+  const server = createServerMock('ws://localhost/shared');
+  const launch = mock.fn(() => Promise.resolve(server));
+  const manager = createBrowserServerManager(createLoggerMock(), { launch, stop: () => Promise.resolve() });
+
+  const [first, second] = await Promise.all([manager.get(LOCAL_CONFIG), manager.get(LOCAL_CONFIG)]);
+
+  assert.strictEqual(first, server);
+  assert.strictEqual(second, server);
+  assert.strictEqual(launch.mock.callCount(), 1, 'expected a single launch shared across concurrent callers');
+
+  await manager.close();
+});
+
+await test('[browser server manager] does not cache a failed launch and retries on the next request', async () => {
+  const server = createServerMock('ws://localhost/recovered');
+  let attempts = 0;
+  const launch = mock.fn(() => {
+    // First launch fails (simulating a transient browser crash), the second succeeds.
+    attempts += 1;
+    return attempts === 1 ? Promise.reject(new Error('Failed to launch browser.')) : Promise.resolve(server);
+  });
+  const manager = createBrowserServerManager(createLoggerMock(), { launch, stop: () => Promise.resolve() });
+
+  await assert.rejects(manager.get(LOCAL_CONFIG), /Failed to launch browser/);
+
+  // The previous failure must not be cached, so the manager reports no running server...
+  assert.strictEqual(manager.isRunning(), false, 'a failed launch must not be cached');
+
+  // ...and the next request launches a fresh server successfully instead of replaying the failure.
+  const recovered = await manager.get(LOCAL_CONFIG);
+  assert.strictEqual(recovered, server);
+  assert.strictEqual(launch.mock.callCount(), 2, 'expected the failed launch to be retried on the next request');
+
+  await manager.close();
+});
+
+await test('[browser server manager] close stops the running server and resets state', async () => {
+  const server = createServerMock('ws://localhost/closing');
+  const stop = mock.fn(() => Promise.resolve());
+  const manager = createBrowserServerManager(createLoggerMock(), { launch: () => Promise.resolve(server), stop });
+
+  await manager.get(LOCAL_CONFIG);
+  assert.strictEqual(manager.isRunning(), true);
+
+  await manager.close();
+  assert.strictEqual(stop.mock.callCount(), 1, 'expected the running server to be stopped on close');
+  assert.strictEqual(manager.isRunning(), false);
+});

components/retrack-web-scraper/src/utilities/browser.ts

@@ -1,3 +1,4 @@
+import { setTimeout as delay } from 'node:timers/promises';
 import type { FastifyBaseLogger } from 'fastify';
 import type { BrowserServer, Logger } from 'playwright-core';
 import { Diagnostics } from '../api/diagnostics.js';
@@ -16,6 +17,14 @@ export type BrowserProtocol = 'cdp' | 'playwright';
 // Timeout for connecting to a browser.
 const BROWSER_CONNECT_TIMEOUT_MS = 30000;
 
+// Number of attempts to launch a local browser server before giving up. The browser process can
+// occasionally crash during startup (e.g. a transient `SIGSEGV` under momentary resource pressure
+// when many trackers fire at the same time), and such failures usually clear on an immediate retry.
+const BROWSER_LAUNCH_MAX_ATTEMPTS = 3;
+
+// Delay between consecutive browser launch attempts.
+const BROWSER_LAUNCH_RETRY_DELAY_MS = 500;
+
 /**
  * Connects to a remote browser over Playwright or CDP protocol:
  *   * `browserType.connectOverCDP` is used to connect to a remote Chromium CDP session. In this case, Playwright
@@ -42,20 +51,33 @@ export async function launchBrowserServer(logger: FastifyBaseLogger, config: Loc
 
   const { chromium, firefox } = await import('playwright-core');
   const backend = config.backend === 'chromium' ? chromium : firefox;
-  try {
-    const localServer = await backend.launchServer({
-      executablePath: config.executablePath,
-      headless: config.headless,
-      args: ['--disable-web-security', '--disable-blink-features=AutomationControlled'],
-      ignoreDefaultArgs: ['--enable-automation'],
-      ...(config.backend === 'chromium' ? { channel: 'chromium', chromiumSandbox: config.chromiumSandbox } : {}),
-    });
-    logger.info(`Browser server is running locally at ${localServer.wsEndpoint()}.`);
-    return localServer;
-  } catch (err) {
-    logger.error(`Failed to run browser server locally: ${Diagnostics.errorMessage(err)}`);
-    throw err;
+
+  let lastError: unknown;
+  for (let attempt = 1; attempt <= BROWSER_LAUNCH_MAX_ATTEMPTS; attempt++) {
+    try {
+      const localServer = await backend.launchServer({
+        executablePath: config.executablePath,
+        headless: config.headless,
+        args: ['--disable-web-security', '--disable-blink-features=AutomationControlled'],
+        ignoreDefaultArgs: ['--enable-automation'],
+        ...(config.backend === 'chromium' ? { channel: 'chromium', chromiumSandbox: config.chromiumSandbox } : {}),
+      });
+      logger.info(`Browser server is running locally at ${localServer.wsEndpoint()}.`);
+      return localServer;
+    } catch (err) {
+      lastError = err;
+      logger.error(
+        `Failed to run browser server locally (attempt ${attempt}/${BROWSER_LAUNCH_MAX_ATTEMPTS}): ${Diagnostics.errorMessage(
+          err,
+        )}`,
+      );
+      if (attempt < BROWSER_LAUNCH_MAX_ATTEMPTS) {
+        await delay(BROWSER_LAUNCH_RETRY_DELAY_MS);
+      }
+    }
   }
+
+  throw lastError;
 }
 
 export async function stopBrowserServer(logger: FastifyBaseLogger, server: BrowserServer) {
@@ -68,3 +90,104 @@ export async function stopBrowserServer(logger: FastifyBaseLogger, server: Brows
     logger.error(`Failed to stop local browser server: ${Diagnostics.errorMessage(err)}`);
   }
 }
+
+/**
+ * Dependencies of {@link createBrowserServerManager}. Primarily exists to allow tests to inject
+ * fakes for the launch/stop primitives without spinning up a real browser.
+ */
+export interface BrowserServerManagerDeps {
+  launch?: (logger: FastifyBaseLogger, config: LocalBrowserConfig) => Promise<BrowserServer>;
+  stop?: (logger: FastifyBaseLogger, server: BrowserServer) => Promise<void>;
+}
+
+/**
+ * Manages a single, lazily-launched local browser server that is shared across all in-flight
+ * extraction requests and torn down after an idle TTL.
+ */
+export interface BrowserServerManager {
+  /**
+   * Returns the shared browser server, launching it on first use. Concurrent callers share the
+   * same in-flight launch. If the launch fails, the cached (rejected) launch is discarded so the
+   * next caller retries from scratch instead of replaying the failure.
+   */
+  get: (config: LocalBrowserConfig) => Promise<BrowserServer>;
+  /** Whether a browser server is currently launched (or being launched). */
+  isRunning: () => boolean;
+  /** Stops the shared browser server (if any) and cancels the pending idle shutdown. */
+  close: () => Promise<void>;
+}
+
+export function createBrowserServerManager(
+  logger: FastifyBaseLogger,
+  deps: BrowserServerManagerDeps = {},
+): BrowserServerManager {
+  const launch = deps.launch ?? launchBrowserServer;
+  const stop = deps.stop ?? stopBrowserServer;
+
+  let serverPromise: Promise<BrowserServer> | undefined;
+  let shutdownInProgress: Promise<void> | undefined;
+  let shutdownTimer: NodeJS.Timeout | undefined;
+
+  const clearShutdownTimer = () => {
+    if (shutdownTimer) {
+      clearTimeout(shutdownTimer);
+      shutdownTimer = undefined;
+    }
+  };
+
+  const scheduleShutdown = (ttlSec: number) => {
+    clearShutdownTimer();
+    shutdownTimer = setTimeout(() => {
+      shutdownTimer = undefined;
+
+      const instance = serverPromise;
+      if (!instance) {
+        return;
+      }
+
+      serverPromise = undefined;
+      shutdownInProgress = instance
+        .then((server) => stop(logger, server))
+        .catch(() => {})
+        .finally(() => {
+          shutdownInProgress = undefined;
+        });
+    }, ttlSec * 1000);
+
+    // The idle shutdown timer must not, by itself, keep the process alive.
+    shutdownTimer.unref?.();
+  };
+
+  return {
+    isRunning: () => !!serverPromise,
+    get: (config: LocalBrowserConfig) => {
+      if (!serverPromise) {
+        const launchPromise = (shutdownInProgress ?? Promise.resolve()).then(() => launch(logger, config));
+        serverPromise = launchPromise;
+
+        // A failed launch must not be cached: drop the rejected promise (and any pending idle
+        // shutdown scheduled for it) so the next request attempts a fresh launch instead of
+        // synchronously replaying the cached failure for the whole TTL window.
+        launchPromise.catch(() => {
+          if (serverPromise === launchPromise) {
+            serverPromise = undefined;
+            clearShutdownTimer();
+          }
+        });
+      }
+
+      scheduleShutdown(config.ttlSec);
+
+      return serverPromise;
+    },
+    close: async () => {
+      clearShutdownTimer();
+
+      const instance = serverPromise;
+      serverPromise = undefined;
+      if (instance) {
+        await instance.then((server) => stop(logger, server)).catch(() => {});
+      }
+    },
+  };
+}