fix(showcase): route browser-pool capacity loss to error/warn + guard reinit

Widen PoolLogger to optional warn/error and route capacity-loss events
(recycle-relaunch-failed, recovery-failed, recycle-failed, new reinit-empty)
to error and per-attempt failures (relaunch-failed, reinit-failed,
close-failed) to warn so Sentry sees real outages instead of buried info.
Add a reiniting concurrent-entry guard so two acquires on an empty pool
launch at most poolSize browsers.

Author: jpr5

showcase/harness/src/probes/helpers/browser-pool.test.ts

@@ -128,6 +128,41 @@ function makeFakeLogger(): {
   };
 }
 
+interface LeveledLog extends CapturedLog {
+  level: "info" | "warn" | "error";
+}
+
+/**
+ * Fake logger that captures the SEVERITY each event was emitted at. Unlike
+ * `makeFakeLogger` (info-only), this exposes `warn`/`error` so routing-by-
+ * severity is observable: a capacity-loss event logged via `logger.error(...)`
+ * must show up with `level: "error"`, never `level: "info"`.
+ */
+function makeLeveledLogger(): {
+  logger: {
+    info: (msg: string, meta?: Record<string, unknown>) => void;
+    warn: (msg: string, meta?: Record<string, unknown>) => void;
+    error: (msg: string, meta?: Record<string, unknown>) => void;
+  };
+  events: LeveledLog[];
+} {
+  const events: LeveledLog[] = [];
+  return {
+    logger: {
+      info(msg, meta) {
+        events.push({ level: "info", event: msg, meta });
+      },
+      warn(msg, meta) {
+        events.push({ level: "warn", event: msg, meta });
+      },
+      error(msg, meta) {
+        events.push({ level: "error", event: msg, meta });
+      },
+    },
+    events,
+  };
+}
+
 /**
  * Flush pending microtasks `times` times so post-recycle state settles
  * enough to observe without tearing the pool down. It does NOT advance real
@@ -700,6 +735,135 @@ describe("BrowserPool dead-instance detection", () => {
     await Promise.allSettled([a, b]);
   });
 
+  it("routes capacity-loss events to error and per-attempt failures to warn", async () => {
+    // Single-slot pool. The browser crashes and EVERY immediate relaunch
+    // attempt fails (calls 2..4 — RELAUNCH_MAX_ATTEMPTS=3), so each per-attempt
+    // close/relaunch failure must log at warn while the terminal capacity-loss
+    // event (`recycle-relaunch-failed`) must log at error.
+    const { launchBrowser, launched } = makeFakeLauncher({
+      failAtCalls: [2, 3, 4],
+    });
+    const { logger, events } = makeLeveledLogger();
+    const pool = new BrowserPool(1, 100, logger, launchBrowser);
+    await pool.init();
+    expect(launched).toHaveLength(1);
+
+    launched[0]!.__crash();
+    await waitFor(() => pool.stats().size === 0, 3_000);
+
+    // Terminal capacity-loss after all retries → error.
+    const recycleFailed = events.find(
+      (e) => e.event === "browser-pool.recycle-relaunch-failed",
+    );
+    expect(recycleFailed).toBeDefined();
+    expect(recycleFailed?.level).toBe("error");
+
+    // The capacity-loss event must NOT also have been emitted at info.
+    expect(
+      events.some(
+        (e) =>
+          e.event === "browser-pool.recycle-relaunch-failed" &&
+          e.level === "info",
+      ),
+    ).toBe(false);
+
+    await pool.shutdown();
+  });
+
+  it("routes per-attempt relaunch-failed (lazy recovery) to warn", async () => {
+    // Single-slot pool. Crash + all immediate retries fail (calls 2..4) parks
+    // the slot. A first acquire's lazy relaunchPendingSlots attempt (call 5)
+    // ALSO fails → `relaunch-failed` must log at warn (per-attempt, not a
+    // terminal capacity loss).
+    const { launchBrowser, launched } = makeFakeLauncher({
+      failAtCalls: [2, 3, 4, 5],
+    });
+    const { logger, events } = makeLeveledLogger();
+    const pool = new BrowserPool(1, 100, logger, launchBrowser);
+    await pool.init();
+
+    launched[0]!.__crash();
+    await waitFor(() => pool.stats().size === 0, 3_000);
+
+    // First acquire drives a relaunchPendingSlots attempt (call 5) which fails.
+    await expect(pool.acquire(50)).rejects.toThrow(
+      "BrowserPool acquire timeout",
+    );
+
+    const relaunchFailed = events.find(
+      (e) => e.event === "browser-pool.relaunch-failed",
+    );
+    expect(relaunchFailed).toBeDefined();
+    expect(relaunchFailed?.level).toBe("warn");
+
+    await pool.shutdown();
+  });
+
+  it("emits an error when reinit ends with an empty pool (every relaunch failed)", async () => {
+    // 1-slot pool. After init (call 1), every subsequent launch fails. The
+    // backstop reinit() can only run when this.slots is truly empty, so we
+    // construct a never-launched pool: init throws nothing (call 1 succeeds),
+    // but to reach the empty-pool reinit path we use a 1-slot pool whose ONLY
+    // slot was never created because init's launch failed. Use failAt=1 so
+    // init launches nothing, leaving this.slots empty; the next acquire drives
+    // reinit, whose launch (call 2) also fails, ending the pool empty → error.
+    const { launchBrowser } = makeFakeLauncher({ failAtCalls: [1, 2, 3] });
+    const { logger, events } = makeLeveledLogger();
+    const pool = new BrowserPool(1, 100, logger, launchBrowser);
+    // init's only launch (call 1) throws — init sets `launchBrowser` BEFORE the
+    // launch loop, so after catching the throw `this.slots` is empty but the
+    // pool can still reinit. (init does not swallow launch errors by design.)
+    await expect(pool.init()).rejects.toThrow("simulated launch failure");
+    expect(pool.stats().size).toBe(0);
+
+    // acquire drives the empty-pool reinit backstop; its launch (call 2) fails,
+    // so reinitInner's loop ends with this.slots.length === 0 → error emit.
+    await expect(pool.acquire(50)).rejects.toThrow(
+      "BrowserPool acquire timeout",
+    );
+
+    const reinitEmpty = events.find(
+      (e) => e.event === "browser-pool.reinit-empty",
+    );
+    expect(reinitEmpty).toBeDefined();
+    expect(reinitEmpty?.level).toBe("error");
+
+    await pool.shutdown();
+  });
+
+  it("does not overshoot poolSize when two concurrent acquires hit an empty pool", async () => {
+    // Empty pool (init launches nothing). Two concurrent acquire() calls both
+    // see this.slots.length === 0. WITHOUT a reiniting guard, BOTH drive
+    // reinit() and each launches up to poolSize browsers — overshoot. WITH the
+    // guard, the second acquire skips reinit and falls through to the waiter
+    // queue, so at most poolSize browsers are launched.
+    const poolSize = 2;
+    // init = call 1 fails (so this.slots stays empty); reinit launches succeed.
+    const { launchBrowser, launched } = makeFakeLauncher({ failAtCalls: [1] });
+    const { logger } = makeLeveledLogger();
+    const pool = new BrowserPool(poolSize, 100, logger, launchBrowser);
+    // init's first launch (call 1) throws, leaving this.slots empty but with
+    // `launchBrowser` already set so reinit can run. init does not swallow.
+    await expect(pool.init()).rejects.toThrow("simulated launch failure");
+    expect(pool.stats().size).toBe(0);
+
+    const a = pool.acquire(5_000);
+    const b = pool.acquire(5_000);
+
+    const first = await Promise.race([a, b]);
+    expect(first).toBeDefined();
+    expect((first as unknown as FakeBrowser).isConnected()).toBe(true);
+
+    await drainMicrotasks(30);
+
+    // The guard ensures only ONE reinit ran: at most poolSize browsers exist.
+    expect(launched.length).toBeLessThanOrEqual(poolSize);
+    expect(pool.stats().size).toBeLessThanOrEqual(poolSize);
+
+    await pool.shutdown();
+    await Promise.allSettled([a, b]);
+  });
+
   it("shutdown() does not loop the disconnect handler when closing slot browsers", async () => {
     const { launchBrowser, launched } = makeFakeLauncher();
     const { logger, events } = makeFakeLogger();

showcase/harness/src/probes/helpers/browser-pool.ts

@@ -42,11 +42,17 @@ export interface BrowserPoolStats {
 
 /**
  * Minimal logger surface the pool uses for lifecycle events. Matches the
- * harness-wide `Logger` interface but only the `info` method is required.
- * Optional so existing callers (tests, legacy boot paths) don't break.
+ * harness-wide `Logger` interface but only the `info` method is required;
+ * `warn`/`error` are OPTIONAL so existing callers (tests, legacy boot paths)
+ * that inject a bare-`info` fake still type-check. The concrete harness logger
+ * implements all three (warn/error route to stderr → Sentry), so capacity-loss
+ * events emitted via `error` reach the alert pipeline instead of being buried
+ * at info. Call the optional methods safely: `this.logger?.warn?.(...)`.
  */
 interface PoolLogger {
   info(msg: string, meta?: Record<string, unknown>): void;
+  warn?(msg: string, meta?: Record<string, unknown>): void;
+  error?(msg: string, meta?: Record<string, unknown>): void;
 }
 
 /**
@@ -73,6 +79,15 @@ export class BrowserPool {
   // the same slot — which would leak one process and/or publish the slot to
   // `available` twice.
   private relaunchingSlots = new Set<Slot>();
+  // Re-entry guard for the empty-pool reinit backstop, mirroring the
+  // relaunchingSlots/recyclingSlots idiom. Two concurrent acquire() calls
+  // against a truly-empty pool (this.slots.length === 0) would each drive
+  // reinit() and each launch up to poolSize browsers — overshooting capacity.
+  // Setting this synchronously before reinit's first await makes the second
+  // acquire skip reinit and fall through to the waiter queue, where the first
+  // reinit's handOff serves it. Cleared in a finally so a thrown reinit can't
+  // wedge the guard set forever.
+  private reiniting = false;
   private isShutdown = false;
   private readonly logger?: PoolLogger;
   private readonly injectedLaunchBrowser?: LaunchBrowser;
@@ -156,7 +171,7 @@ export class BrowserPool {
     try {
       await browser.close();
     } catch (err) {
-      this.logger?.info("browser-pool.close-failed", {
+      this.logger?.warn?.("browser-pool.close-failed", {
         slotIndex,
         error: err instanceof Error ? err.message : String(err),
       });
@@ -178,7 +193,7 @@ export class BrowserPool {
     // contained — a recovery launch failing is non-fatal to the pool.
     return promise
       .catch((err: unknown) => {
-        this.logger?.info("browser-pool.recovery-failed", {
+        this.logger?.error?.("browser-pool.recovery-failed", {
           error: err instanceof Error ? err.message : String(err),
         });
       })
@@ -200,10 +215,20 @@ export class BrowserPool {
    */
   private async reinit(): Promise<void> {
     if (this.isShutdown || this.launchBrowser === undefined) return;
-    // Track the whole reinit so a shutdown racing this launch loop drains it
-    // before closing slots, rather than leaking a browser that resolves after
-    // shutdown already awaited the in-flight set.
-    await this.track(this.reinitInner());
+    // Set the concurrent-entry guard SYNCHRONOUSLY before the first `await`
+    // (before `this.track(...)`) so a second acquire reaching its empty-pool
+    // check while this reinit is in flight skips reinit and parks a waiter
+    // instead of launching its own duplicate set of browsers. Cleared in the
+    // `finally` so a thrown reinit can't leave the guard latched forever.
+    this.reiniting = true;
+    try {
+      // Track the whole reinit so a shutdown racing this launch loop drains it
+      // before closing slots, rather than leaking a browser that resolves after
+      // shutdown already awaited the in-flight set.
+      await this.track(this.reinitInner());
+    } finally {
+      this.reiniting = false;
+    }
   }
 
   private async reinitInner(): Promise<void> {
@@ -229,12 +254,25 @@ export class BrowserPool {
         this.handOff(slot, browser);
       } catch (err) {
         // Best effort — keep trying the remaining slots. If none succeed
-        // the pool stays empty and the next acquire retries reinit.
-        this.logger?.info("browser-pool.reinit-failed", {
+        // the pool stays empty and the next acquire retries reinit. Per-attempt
+        // failure is a warn: a single slot failing to relaunch is recoverable
+        // (other slots may succeed, or a later acquire retries).
+        this.logger?.warn?.("browser-pool.reinit-failed", {
           error: err instanceof Error ? err.message : String(err),
         });
       }
     }
+
+    // The genuine empty-pool capacity-loss signal: reinit ran but every launch
+    // failed, so the pool ends with zero slots. Unlike the per-attempt warn
+    // above, this is the terminal "pool ended empty" outage and must reach the
+    // error/Sentry pipeline. Per-attempt reinit-failed stays warn (recoverable);
+    // this distinct end-of-loop emit fires once when the backstop drained empty.
+    if (this.slots.length === 0) {
+      this.logger?.error?.("browser-pool.reinit-empty", {
+        poolSize: this.poolSize,
+      });
+    }
   }
 
   /**
@@ -309,8 +347,10 @@ export class BrowserPool {
           slotIndex: this.slots.indexOf(slot),
         });
       } catch (err) {
-        // Still failing — leave it pending for the next acquire to retry.
-        this.logger?.info("browser-pool.relaunch-failed", {
+        // Still failing — leave it pending for the next acquire to retry. This
+        // is a per-attempt failure (the slot stays parked and recoverable), so
+        // warn rather than error.
+        this.logger?.warn?.("browser-pool.relaunch-failed", {
           slotIndex: this.slots.indexOf(slot),
           error: err instanceof Error ? err.message : String(err),
         });
@@ -333,16 +373,21 @@ export class BrowserPool {
     // `relaunchPendingSlots()`, not here. (`stats().size` may read 0 in that
     // state, but `size` does not gate this backstop — `this.slots.length`
     // does.)
-    if (this.slots.length === 0) {
+    if (this.slots.length === 0 && !this.reiniting) {
       await this.reinit();
-    } else {
+    } else if (this.slots.length > 0) {
       // Lazily relaunch any slot whose last relaunch failed. This is the
       // recovery path for a transient launch failure: the slot was kept
       // (capacity preserved) but parked as `relaunchPending`; the next
       // acquire re-attempts the launch before falling through to the
       // normal available-slot scan.
       await this.relaunchPendingSlots();
     }
+    // The remaining case — `slots.length === 0 && this.reiniting` — is the
+    // concurrent-acquire skip: another acquire's reinit is already launching
+    // up to poolSize browsers, so this call does NOT launch its own duplicate
+    // set. It falls through to the waiter-queue path below; the in-flight
+    // reinit's `handOff` serves this waiter as soon as a fresh browser lands.
 
     // Skip zombie slots whose browser has disconnected but whose disconnect
     // handler hasn't yet completed the recycle. Without this loop, a single
@@ -601,7 +646,10 @@ export class BrowserPool {
       // launch (see relaunchPendingSlots), and the empty-pool backstop in
       // acquire() re-initializes if every slot ends up pending/lost.
       slot.relaunchPending = true;
-      this.logger?.info("browser-pool.recycle-relaunch-failed", {
+      // Capacity loss after exhausting ALL immediate retries — the slot is now
+      // parked dead and only a later lazy acquire can recover it. This is a
+      // genuine capacity-loss signal that must reach error/Sentry, not info.
+      this.logger?.error?.("browser-pool.recycle-relaunch-failed", {
         slotIndex: this.slots.indexOf(slot),
         attempts: RELAUNCH_MAX_ATTEMPTS,
         poolSize: this.slots.length,
@@ -623,7 +671,7 @@ export class BrowserPool {
     // never surface as an unhandled rejection.
     recyclePromise
       .catch((err: unknown) => {
-        this.logger?.info("browser-pool.recycle-failed", {
+        this.logger?.error?.("browser-pool.recycle-failed", {
           slotIndex: this.slots.indexOf(slot),
           error: err instanceof Error ? err.message : String(err),
         });