Skip to content

Commit 9e31b48

Browse files
committed
Skip frozen page targets during enumeration (#1230)
1 parent 9a47b65 commit 9e31b48

2 files changed

Lines changed: 120 additions & 5 deletions

File tree

src/McpContext.ts

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,67 @@ interface McpContextOptions {
6060

6161
const DEFAULT_TIMEOUT = 5_000;
6262
const NAVIGATION_TIMEOUT = 10_000;
63+
const RECOVERABLE_PAGE_INITIALIZATION_TIMEOUTS = [
64+
'Network.enable timed out',
65+
'Page.enable timed out',
66+
'Runtime.enable timed out',
67+
];
68+
69+
function isRecoverablePageInitializationError(error: unknown): boolean {
70+
const message = error instanceof Error ? error.message : String(error);
71+
return RECOVERABLE_PAGE_INITIALIZATION_TIMEOUTS.some(timeout => {
72+
return message.includes(timeout);
73+
});
74+
}
75+
76+
async function getPageFromTarget(
77+
target: Target,
78+
logger: Debugger,
79+
operation: string,
80+
skipRecoverableErrors = false,
81+
): Promise<Page | null> {
82+
try {
83+
return await target.page();
84+
} catch (error) {
85+
if (
86+
!skipRecoverableErrors ||
87+
!isRecoverablePageInitializationError(error)
88+
) {
89+
throw error;
90+
}
91+
92+
logger(
93+
`Skipping target during ${operation}: ${target.type()} ${target.url()}`,
94+
error,
95+
);
96+
return null;
97+
}
98+
}
99+
100+
async function enumerateHealthyPagesByTarget(
101+
browser: Browser,
102+
logger: Debugger,
103+
): Promise<Page[]> {
104+
const pages: Page[] = [];
105+
106+
for (const target of browser.targets()) {
107+
if (target.type() !== 'page') {
108+
continue;
109+
}
110+
111+
const page = await getPageFromTarget(
112+
target,
113+
logger,
114+
'fallback page enumeration',
115+
true,
116+
);
117+
if (page && !pages.includes(page)) {
118+
pages.push(page);
119+
}
120+
}
121+
122+
return pages;
123+
}
63124

64125
function getNetworkMultiplierFromString(condition: string | null): number {
65126
const puppeteerCondition =
@@ -579,9 +640,22 @@ export class McpContext implements Context {
579640
isolatedContextNames: Map<Page, string>;
580641
}> {
581642
const defaultCtx = this.browser.defaultBrowserContext();
582-
const allPages = await this.browser.pages(
583-
this.#options.experimentalIncludeAllPages,
584-
);
643+
let allPages: Page[];
644+
try {
645+
allPages = await this.browser.pages(
646+
this.#options.experimentalIncludeAllPages,
647+
);
648+
} catch (error) {
649+
if (!isRecoverablePageInitializationError(error)) {
650+
throw error;
651+
}
652+
653+
this.logger(
654+
'browser.pages() failed with a recoverable page initialization error, falling back to per-target enumeration',
655+
error,
656+
);
657+
allPages = await enumerateHealthyPagesByTarget(this.browser, this.logger);
658+
}
585659

586660
const allTargets = this.browser.targets();
587661
const extensionTargets = allTargets.filter(target => {
@@ -593,7 +667,12 @@ export class McpContext implements Context {
593667

594668
for (const target of extensionTargets) {
595669
// Right now target.page() returns null for popup and side panel pages.
596-
let page = await target.page();
670+
let page = await getPageFromTarget(
671+
target,
672+
this.logger,
673+
'extension target enumeration',
674+
true,
675+
);
597676
if (!page) {
598677
// We need to cache pages instances for targets because target.asPage()
599678
// returns a new page instance every time.

tests/McpContext.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {afterEach, describe, it} from 'node:test';
1010
import sinon from 'sinon';
1111

1212
import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js';
13-
import type {HTTPResponse} from '../src/third_party/index.js';
13+
import type {HTTPResponse, Target} from '../src/third_party/index.js';
1414
import type {TraceResult} from '../src/trace-processing/parse.js';
1515

1616
import {getMockRequest, html, withMcpContext} from './utils.js';
@@ -37,6 +37,42 @@ describe('McpContext', () => {
3737
});
3838
});
3939

40+
it('falls back to healthy targets when browser.pages hits a recoverable timeout', async () => {
41+
await withMcpContext(async (_response, context) => {
42+
const healthyPage = context.getSelectedMcpPage().pptrPage;
43+
const timeoutError = new Error(
44+
"Network.enable timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.",
45+
);
46+
const healthyTarget = {
47+
type: () => {
48+
return 'page';
49+
},
50+
url: () => {
51+
return healthyPage.url();
52+
},
53+
page: sinon.stub().resolves(healthyPage),
54+
} as unknown as Target;
55+
const frozenTarget = {
56+
type: () => {
57+
return 'page';
58+
},
59+
url: () => {
60+
return 'https://discarded.example.test/';
61+
},
62+
page: sinon.stub().rejects(timeoutError),
63+
} as unknown as Target;
64+
65+
sinon.stub(context.browser, 'pages').rejects(timeoutError);
66+
sinon
67+
.stub(context.browser, 'targets')
68+
.returns([healthyTarget, frozenTarget]);
69+
70+
const pages = await context.createPagesSnapshot();
71+
72+
assert.deepStrictEqual(pages, [healthyPage]);
73+
});
74+
});
75+
4076
it('can store and retrieve the latest performance trace', async () => {
4177
await withMcpContext(async (_response, context) => {
4278
const fakeTrace1 = {} as unknown as TraceResult;

0 commit comments

Comments
 (0)