fix(browser): stop expired work and release deadline resources

src/browser/PlaywrightExecutionBoundary.test.ts

@@ -1,3 +1,5 @@
+import { getEventListeners } from "node:events";
+
 import { afterEach, describe, expect, it, vi } from "vitest";
 
 import { withPlaywrightExecutionBoundary } from "./PlaywrightExecutionBoundary.js";
@@ -24,6 +26,56 @@ describe("withPlaywrightExecutionBoundary", () => {
     await rejection;
   });
 
+  it("uses the owning policy's timeout error when its deadline expires", async () => {
+    vi.useFakeTimers();
+    const timeout = new AnalysisTimeoutError("capture_browser_scenario", 25, {
+      userMessage: "Fixed service-worker policy deadline.",
+    });
+    const result = withPlaywrightExecutionBoundary(
+      () => new Promise<never>(() => undefined),
+      25,
+      undefined,
+      () => timeout,
+    );
+    const rejection = expect(result).rejects.toBe(timeout);
+    await vi.advanceTimersByTimeAsync(25);
+    await rejection;
+  });
+
+  it("does not admit work or attach cancellation after synchronous expiry", async () => {
+    vi.spyOn(performance, "now")
+      .mockReturnValueOnce(1_000)
+      .mockReturnValueOnce(1_001);
+    const controller = new AbortController();
+    const addListener = vi.spyOn(controller.signal, "addEventListener");
+    const work = vi.fn(async () => "unreachable");
+
+    await expect(
+      withPlaywrightExecutionBoundary(work, 0.5, controller.signal),
+    ).rejects.toBeInstanceOf(AnalysisTimeoutError);
+    expect(work).not.toHaveBeenCalled();
+    expect(addListener).not.toHaveBeenCalled();
+  });
+
+  it("cleans up when admitted work throws synchronously", async () => {
+    vi.useFakeTimers();
+    const cause = { opaque: true };
+    const controller = new AbortController();
+    const work = vi.fn((): Promise<never> => {
+      throw cause;
+    });
+    const pending = withPlaywrightExecutionBoundary(
+      work,
+      1_000,
+      controller.signal,
+    );
+
+    await expect(pending).rejects.toBe(cause);
+    expect(work).toHaveBeenCalledOnce();
+    expect(getEventListeners(controller.signal, "abort")).toHaveLength(0);
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
   it("rejects when the caller cancels active work", async () => {
     const controller = new AbortController();
     const result = withPlaywrightExecutionBoundary(

src/browser/PlaywrightExecutionBoundary.ts

@@ -12,9 +12,11 @@ export const withPlaywrightExecutionBoundary = <Value>(
   work: () => Promise<Value>,
   timeoutMs: number | undefined,
   signal?: AbortSignal,
+  timeoutError: () => AnalysisTimeoutError = () =>
+    new AnalysisTimeoutError(OPERATION, timeoutMs ?? 0),
 ): Promise<Value> => {
   if (timeoutMs !== undefined && timeoutMs <= 0)
-    return Promise.reject(new AnalysisTimeoutError(OPERATION, timeoutMs));
+    return Promise.reject(timeoutError());
   if (signal?.aborted === true)
     return Promise.reject(
       new AnalysisCancelledError(OPERATION, { cause: signal.reason }),
@@ -29,7 +31,7 @@ export const withPlaywrightExecutionBoundary = <Value>(
       if (deadline === undefined) return;
       const remaining = deadline - performance.now();
       if (remaining <= 0) {
-        fail(new AnalysisTimeoutError(OPERATION, timeoutMs ?? 0));
+        fail(timeoutError());
         return;
       }
       timer = setTimeout(scheduleTimeout, Math.min(remaining, 2_147_483_647));
@@ -55,11 +57,16 @@ export const withPlaywrightExecutionBoundary = <Value>(
       fail(new AnalysisCancelledError(OPERATION, { cause: signal?.reason }));
     };
     scheduleTimeout();
+    if (settled) return;
     signal?.addEventListener("abort", onAbort, { once: true });
     if (signal?.aborted === true) {
       onAbort();
       return;
     }
-    void work().then(succeed, fail);
+    try {
+      void work().then(succeed, fail);
+    } catch (cause: unknown) {
+      fail(cause);
+    }
   });
 };

src/browser/PlaywrightScenarioServiceWorkers.ts

@@ -8,10 +8,7 @@ import {
   CDP_DISCOVERY_RESPONSE_TIMEOUT_MS,
   selectedCdpCommandTimeoutMs,
 } from "../domain/browserDeadline.js";
-import {
-  AnalysisCancelledError,
-  AnalysisTimeoutError,
-} from "../domain/analysisErrorCore.js";
+import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js";
 import { browserObservationEndpointSchema } from "../domain/browserObservation.js";
 import { BrowserObservationError } from "../domain/browserObservationError.js";
 import { CdpConnection } from "./CdpConnection.js";
@@ -44,47 +41,6 @@ const serviceWorkerPolicyCheckTimeout = (): AnalysisTimeoutError => {
   });
 };
 
-const withServiceWorkerPolicyDeadline = <Value>(
-  work: () => Promise<Value>,
-  signal: AbortSignal,
-): Promise<Value> => {
-  if (signal.aborted)
-    return Promise.reject(
-      new AnalysisCancelledError(OPERATION, { cause: signal.reason }),
-    );
-  return new Promise<Value>((resolve, reject) => {
-    let settled = false;
-    const timer = setTimeout(() => {
-      fail(serviceWorkerPolicyCheckTimeout());
-    }, SERVICE_WORKER_POLICY_CHECK_TIMEOUT_MS);
-    timer.unref();
-    const cleanup = (): void => {
-      clearTimeout(timer);
-      signal.removeEventListener("abort", onAbort);
-    };
-    const succeed = (value: Value): void => {
-      if (settled) return;
-      settled = true;
-      cleanup();
-      resolve(value);
-    };
-    const fail = (error: unknown): void => {
-      if (settled) return;
-      settled = true;
-      cleanup();
-      reject(error);
-    };
-    const onAbort = (): void => {
-      fail(new AnalysisCancelledError(OPERATION, { cause: signal.reason }));
-    };
-    signal.addEventListener("abort", onAbort, { once: true });
-    if (signal.aborted) {
-      onAbort();
-      return;
-    }
-    void work().then(succeed, fail);
-  });
-};
 const scriptResponse = z.object({ identifier: z.string() });
 const restorationResponse = z.object({
   result: z.object({ value: z.literal("policy_owned").nullable() }),
@@ -136,9 +92,11 @@ export class PlaywrightScenarioServiceWorkers {
   }
 
   private async install(): Promise<void> {
-    await withServiceWorkerPolicyDeadline(
+    await withPlaywrightExecutionBoundary(
       () => this.assertBlocked(),
+      SERVICE_WORKER_POLICY_CHECK_TIMEOUT_MS,
       this.#setup.signal,
+      serviceWorkerPolicyCheckTimeout,
     );
     // Discovery stays on its fixed deadline. The following handshake and
     // Target.attachToTarget each use command_timeout_ms.

tests/boundary/browser/cdpTargetDeadlines.test.ts

@@ -401,6 +401,8 @@ it("stops a stalled blocked-service-worker check before discovery", async () =>
     "allowed-page",
     commandTimeoutMs,
   );
+  // Keep the monotonic deadline and its explicitly expired timer in sync.
+  const clock = vi.spyOn(performance, "now").mockReturnValue(1_000);
   const pending = workers.initialize();
   try {
     await vi.waitFor(() =>
@@ -411,6 +413,7 @@ it("stops a stalled blocked-service-worker check before discovery", async () =>
     expect(browser.httpRequests).toHaveLength(0);
     expect(browser.commands).toHaveLength(0);
     expect(activeTimers(commandTimeoutMs)).toHaveLength(0);
+    clock.mockReturnValue(1_000 + SERVICE_WORKER_POLICY_CHECK_TIMEOUT_MS);
     expireTimer(SERVICE_WORKER_POLICY_CHECK_TIMEOUT_MS);
     const failure = await pending.catch((cause: unknown) => cause);
     expect(failure).toBeInstanceOf(AnalysisTimeoutError);
@@ -424,6 +427,7 @@ it("stops a stalled blocked-service-worker check before discovery", async () =>
     expect(browser.httpRequests).toHaveLength(0);
     expect(activeTimers(commandTimeoutMs)).toHaveLength(0);
   } finally {
+    clock.mockRestore();
     const closing = workers.close().catch(() => undefined);
     await browser.close();
     await closing;