fix: settle popup and download event handles without deadlock

src/browser/run/playwright-transport.ts

@@ -352,14 +352,18 @@ export class PlaywrightTransport {
   readonly #connection: DispatcherConnection;
   readonly #root: RootDispatcher;
   readonly #deliver: (message: string) => void;
+  readonly #onDeliveryError: ((error: Error) => void) | undefined;
+  readonly #inbound: string[] = [];
   #registerPageImpl: (page: Page) => void;
   #cancellation: Promise<void> | undefined;
   #disposed = false;
+  #flushScheduled = false;
   #browserWaitMs = 0;
 
   constructor(
     input: BrowserRunSessionScope,
     deliver: (message: string) => void,
+    onDeliveryError?: (error: Error) => void,
   ) {
     if (
       !input.browser.contexts().includes(input.context)
@@ -381,10 +385,9 @@ export class PlaywrightTransport {
     this.#registerPageImpl = page => { implementation(page); };
     for (const page of input.pages()) this.registerPage(page);
     this.#deliver = deliver;
+    this.#onDeliveryError = onDeliveryError;
     this.#connection = new server.DispatcherConnection();
-    this.#connection.onmessage = message => {
-      if (!this.#disposed) this.#deliver(JSON.stringify(message));
-    };
+    this.#connection.onmessage = message => this.handleServerMessage(message);
     this.#root = new server.RootDispatcher(
       this.#connection,
       async (scope, { sdkLanguage }) => new server.PlaywrightDispatcher(
@@ -438,6 +441,12 @@ export class PlaywrightTransport {
     this.#registerPageImpl(page);
   }
 
+  private handleServerMessage(message: Record<string, unknown>): void {
+    if (this.#disposed) return;
+    this.#inbound.push(JSON.stringify(message));
+    this.#scheduleFlush();
+  }
+
   cancel(error: Error): Promise<void> {
     if (this.#disposed) return Promise.resolve();
     this.#cancellation ??= this.#root.stopPendingOperations(error).catch(() => undefined);
@@ -448,23 +457,52 @@ export class PlaywrightTransport {
     if (this.#disposed) return;
     await this.cancel(error);
     this.#disposed = true;
+    this.#inbound.length = 0;
     this.#connection.onmessage = () => undefined;
     this.#root._dispose();
   }
 
+  #scheduleFlush(): void {
+    if (this.#flushScheduled) return;
+    this.#flushScheduled = true;
+    queueMicrotask(() => {
+      this.#flushScheduled = false;
+      this.#flushInbound();
+    });
+  }
+
+  #flushInbound(): void {
+    while (!this.#disposed && this.#inbound.length > 0) {
+      try {
+        this.#deliver(this.#inbound.shift()!);
+      } catch (error) {
+        this.#failDelivery(error);
+        return;
+      }
+    }
+  }
+
+  #failDelivery(error: unknown): void {
+    this.#onDeliveryError?.(error instanceof Error ? error : new Error(String(error)));
+  }
+
   #unsupported(id: unknown, api: string): void {
     queueMicrotask(() => {
       if (this.#disposed) return;
-      this.#deliver(JSON.stringify({
-        id,
-        error: {
+      try {
+        this.#deliver(JSON.stringify({
+          id,
           error: {
-            name: 'BrowserRunError',
-            message: `BROWSER_RUN_API_UNSUPPORTED: ${unsupportedApiMessage(api)}`,
-            stack: '',
+            error: {
+              name: 'BrowserRunError',
+              message: `BROWSER_RUN_API_UNSUPPORTED: ${unsupportedApiMessage(api)}`,
+              stack: '',
+            },
           },
-        },
-      }));
+        }));
+      } catch (error) {
+        this.#failDelivery(error);
+      }
     });
   }
 }

src/browser/run/runner.test.ts

@@ -21,7 +21,7 @@ import {
 } from 'playwright-core';
 import { LocalBrowserRunArtifactSink } from './artifacts.js';
 import { MemorySnapshotBaselineStore } from '../snapshot/index.js';
-import { unsupportedApiMessage } from './playwright-transport.js';
+import { PlaywrightTransport, unsupportedApiMessage } from './playwright-transport.js';
 import { QuickJSHost } from './quickjs-host.js';
 import { DOWNLOAD_WAIT_TIMEOUT_HINT, POPUP_WAIT_TIMEOUT_HINT, runBrowserProgram } from './runner.js';
 
@@ -355,7 +355,12 @@ afterAll(async () => {
     });
   });
 
-  it('waits for popups and exposes context pages', async () => {
+  it('resolves waitForEvent("popup") into a readable Page before the outer run timeout', async () => {
+    await context.route('https://popup.example.test/**', route => route.fulfill({
+      contentType: 'text/html',
+      body: '<!doctype html><title>Popup Title</title><p>Popup Body</p>',
+    }));
+    await page.setContent('<a href="https://popup.example.test/" target="_blank">Popup</a>');
     const registered: Page[] = [];
     const output = await runBrowserProgram({
       ...sessionScope(),
@@ -369,17 +374,23 @@ afterAll(async () => {
         return () => context.off('page', registeredListener);
       },
     }, `
-      const popupPromise = page.waitForEvent('popup');
+      const popupPromise = page.waitForEvent('popup', { timeout: 5000 });
       await page.getByRole('link', { name: 'Popup' }).click();
       const popup = await popupPromise;
       return {
         popupUrl: popup.url(),
+        title: await popup.title(),
+        body: await popup.locator('body').innerText(),
         pages: context.pages().length,
-        contexts: browser.contexts().length,
       };
-    `);
+    `, { timeoutMs: 15_000 });
 
-    expect(output.result).toEqual({ popupUrl: 'about:blank', pages: 2, contexts: 1 });
+    expect(output.result).toEqual({
+      popupUrl: 'https://popup.example.test/',
+      title: 'Popup Title',
+      body: 'Popup Body',
+      pages: 2,
+    });
     expect(registered).toEqual([context.pages()[1]]);
   });
 
@@ -671,14 +682,19 @@ afterAll(async () => {
     }
   });
 
-  it('waits for downloads', async () => {
+  it('resolves waitForEvent("download") and saves the file before the outer run timeout', async () => {
     const output = await run(`
-      const downloadPromise = page.waitForEvent('download');
+      const downloadPromise = page.waitForEvent('download', { timeout: 5000 });
       await page.getByRole('link', { name: 'Download' }).click();
-      return (await downloadPromise).suggestedFilename();
-    `);
+      const download = await downloadPromise;
+      await download.saveAs(download.suggestedFilename());
+      return { filename: download.suggestedFilename() };
+    `, { timeoutMs: 15_000 });
 
-    expect(output.result).toBe('hello.txt');
+    expect(output.result).toEqual({ filename: 'hello.txt' });
+    expect(output.artifacts).toEqual([
+      expect.objectContaining({ filename: 'hello.txt' }),
+    ]);
   });
 
   it('captures a generated blob object-URL download as an artifact', async () => {
@@ -711,6 +727,33 @@ afterAll(async () => {
     ]);
   });
 
+  it.each(['popup', 'download'] as const)('honors waitForEvent("%s") timeout when no event fires', async (event) => {
+    const started = Date.now();
+    await expect(run(`
+      await page.waitForEvent(${JSON.stringify(event)}, { timeout: 80 });
+    `, { timeoutMs: 5_000 })).rejects.toMatchObject({
+      code: 'BROWSER_RUN_TIMEOUT',
+      message: expect.stringContaining(`Timeout 80ms exceeded while waiting for event "${event}"`),
+    });
+    expect(Date.now() - started).toBeLessThan(1_000);
+  });
+
+  it('routes a throwing deferred delivery into onDeliveryError', async () => {
+    const seen: Error[] = [];
+    const transport = new PlaywrightTransport(sessionScope(), () => {
+      throw new Error('delivery failed');
+    }, (error) => { seen.push(error); });
+    try {
+      (transport as unknown as {
+        handleServerMessage(message: Record<string, unknown>): void;
+      }).handleServerMessage({ id: 1, method: 'unused' });
+      await new Promise(resolve => setTimeout(resolve, 0));
+      expect(seen.map(error => error.message)).toContain('delivery failed');
+    } finally {
+      await transport.dispose();
+    }
+  });
+
   it('tells the caller not to recompute bytes when a download wait times out', async () => {
     await expect(run(`
       await page.waitForEvent('download');

src/browser/run/runner.ts

@@ -305,10 +305,20 @@ export async function runBrowserProgram(
   const baselineStore = options.snapshotBaselineStore ?? new MemorySnapshotBaselineStore();
   if (!snapshotDiffEnabled) baselineStore.clear(input.pageId);
   let host!: QuickJSHost;
+  let transport!: PlaywrightTransport;
   const artifactSink = input.artifactSink ?? new LocalBrowserRunArtifactSink();
-  const transport = new PlaywrightTransport(input, message => (
-    host.deliverTransport(message)
-  ));
+  const pendingDelivery: string[] = [];
+  transport = new PlaywrightTransport(input, message => {
+    if (!host) {
+      pendingDelivery.push(message);
+      return;
+    }
+    host.deliverTransport(message);
+  }, (error) => {
+    if (!host) return;
+    host.cancelPending(error);
+    void transport.cancel(error);
+  });
   const quickjsBootStartedAt = Date.now();
   try {
     host = await QuickJSHost.create({
@@ -354,6 +364,8 @@ export async function runBrowserProgram(
       },
     });
     host.installHostCall();
+    for (const message of pendingDelivery) host.deliverTransport(message);
+    pendingDelivery.length = 0;
   } catch (error) {
     timings.quickjs_boot_ms = Math.max(0, Date.now() - quickjsBootStartedAt);
     await transport.dispose(error instanceof Error ? error : new Error(String(error)));