fix: skip unresponsive page targets

Author: bertrand319

src/McpContext.ts

@@ -59,6 +59,25 @@ interface McpContextOptions {
 const DEFAULT_TIMEOUT = 5_000;
 const NAVIGATION_TIMEOUT = 10_000;
 
+async function resolveWithTimeout<T>(
+  promise: Promise<T>,
+  timeoutMs: number,
+): Promise<T | undefined> {
+  let timeout: ReturnType<typeof setTimeout> | undefined;
+  const timeoutPromise = new Promise<undefined>(resolve => {
+    timeout = setTimeout(() => {
+      resolve(undefined);
+    }, timeoutMs);
+  });
+  try {
+    return await Promise.race([promise, timeoutPromise]);
+  } finally {
+    if (timeout !== undefined) {
+      clearTimeout(timeout);
+    }
+  }
+}
+
 export class McpContext implements Context {
   browser: Browser;
   logger: Debugger;
@@ -564,9 +583,20 @@ export class McpContext implements Context {
     isolatedContextNames: Map<Page, string>;
   }> {
     const defaultCtx = this.browser.defaultBrowserContext();
-    const allPages = await this.browser.pages(
-      this.#options.experimentalIncludeAllPages,
-    );
+    const pagePromises: Array<Promise<Page | null>> = [];
+    for (const target of this.browser.targets()) {
+      if (!this.#isPageTarget(target)) {
+        continue;
+      }
+      pagePromises.push(this.#resolveTargetPage(target));
+    }
+
+    const allPages: Page[] = [];
+    for (const page of await Promise.all(pagePromises)) {
+      if (page && !allPages.includes(page)) {
+        allPages.push(page);
+      }
+    }
 
     const allTargets = this.browser.targets();
     const extensionTargets = allTargets.filter(target => {
@@ -578,17 +608,15 @@ export class McpContext implements Context {
 
     for (const target of extensionTargets) {
       // Right now target.page() returns null for popup and side panel pages.
-      let page = await target.page();
+      let page = await this.#resolveTargetPage(target);
       if (!page) {
         // We need to cache pages instances for targets because target.asPage()
         // returns a new page instance every time.
         page = this.#extensionPages.get(target) ?? null;
         if (!page) {
-          try {
-            page = await target.asPage();
+          page = await this.#resolveTargetPage(target, true);
+          if (page) {
             this.#extensionPages.set(target, page);
-          } catch (e) {
-            this.logger('Failed to get page for extension target', e);
           }
         }
       }
@@ -628,6 +656,42 @@ export class McpContext implements Context {
     return {pages: allPages, isolatedContextNames};
   }
 
+  #isPageTarget(target: Target): boolean {
+    const type = target.type();
+    if (type === 'page') {
+      return true;
+    }
+    if (!this.#options.experimentalIncludeAllPages) {
+      return false;
+    }
+    return type === 'background_page' || type === 'webview' || type === 'other';
+  }
+
+  async #resolveTargetPage(
+    target: Target,
+    force = false,
+  ): Promise<Page | null> {
+    try {
+      const pagePromise: Promise<Page | null> = force
+        ? target.asPage()
+        : target.page();
+      const page = await resolveWithTimeout(pagePromise, DEFAULT_TIMEOUT);
+      if (page === undefined) {
+        this.logger(
+          `Timed out getting page for target ${target.type()} ${target.url()}`,
+        );
+        return null;
+      }
+      return page;
+    } catch (error) {
+      this.logger(
+        `Failed to get page for target ${target.type()} ${target.url()}`,
+        error,
+      );
+      return null;
+    }
+  }
+
   async detectOpenDevToolsWindows() {
     this.logger('Detecting open DevTools windows');
     const {pages} = await this.#getAllPages();

tests/McpContext.test.ts

@@ -9,15 +9,63 @@ import path from 'node:path';
 import {afterEach, describe, it} from 'node:test';
 import {pathToFileURL} from 'node:url';
 
+import {TargetType} from 'puppeteer-core';
 import sinon from 'sinon';
 
 import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js';
 import {TextSnapshot} from '../src/TextSnapshot.js';
-import type {HTTPResponse} from '../src/third_party/index.js';
+import type {
+  Browser,
+  BrowserContext,
+  CDPSession,
+  HTTPResponse,
+  Page,
+  Target,
+  WebWorker,
+} from '../src/third_party/index.js';
 import type {TraceResult} from '../src/trace-processing/parse.js';
 
 import {getMockRequest, html, withMcpContext} from './utils.js';
 
+function createHangingPageTarget(
+  browser: Browser,
+  browserContext: BrowserContext,
+): Target {
+  return {
+    page(): Promise<Page | null> {
+      return new Promise(() => {
+        // Intentionally never resolves to mimic a stalled target.
+      });
+    },
+    asPage(): Promise<Page> {
+      return new Promise(() => {
+        // Intentionally never resolves to mimic a stalled target.
+      });
+    },
+    url(): string {
+      return 'https://example.com/hanging';
+    },
+    createCDPSession(): Promise<CDPSession> {
+      return Promise.reject(new Error('Not implemented'));
+    },
+    type(): TargetType {
+      return TargetType.PAGE;
+    },
+    browser(): Browser {
+      return browser;
+    },
+    browserContext(): BrowserContext {
+      return browserContext;
+    },
+    opener(): Target | undefined {
+      return;
+    },
+    worker(): Promise<WebWorker | null> {
+      return Promise.resolve(null);
+    },
+  };
+}
+
 describe('McpContext', () => {
   afterEach(() => {
     sinon.restore();
@@ -100,6 +148,29 @@ describe('McpContext', () => {
       },
     );
   });
+
+  it('skips page targets that do not resolve while creating a pages snapshot', async () => {
+    await withMcpContext(async (_response, context) => {
+      const page = context.getSelectedPptrPage();
+      const targets = context.browser.targets();
+      const hangingTarget = createHangingPageTarget(
+        context.browser,
+        page.browserContext(),
+      );
+      sinon
+        .stub(context.browser, 'targets')
+        .returns([...targets, hangingTarget]);
+      sinon.stub(context, 'detectOpenDevToolsWindows').resolves();
+      const clock = sinon.useFakeTimers();
+
+      const pagesPromise = context.createPagesSnapshot();
+      await clock.tickAsync(5_000);
+      const pages = await pagesPromise;
+
+      assert.ok(pages.includes(page));
+    });
+  });
+
   it('resolves uid from a non-selected page snapshot', async () => {
     await withMcpContext(async (_response, context) => {
       // Page 1: set content and snapshot