Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Commit a0aeb97

Browse files
OrKoNOrKoN
authored
fix: respect custom timeouts in navigate tools (ChromeDevTools#865)
Closes ChromeDevTools#863
1 parent c0435a2 commit a0aeb97

5 files changed

Lines changed: 109 additions & 65 deletions

File tree

‎src/McpContext.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,10 @@ export class McpContext implements Context {
727727
return new WaitForHelper(page, cpuMultiplier, networkMultiplier);
728728
}
729729

730-
waitForEventsAfterAction(action: () => Promise<unknown>): Promise<void> {
730+
waitForEventsAfterAction(
731+
action: () => Promise<unknown>,
732+
options?: {timeout?: number},
733+
): Promise<void> {
731734
const page = this.getSelectedPage();
732735
const cpuMultiplier = this.getCpuThrottlingRate();
733736
const networkMultiplier = getNetworkMultiplierFromString(
@@ -738,7 +741,7 @@ export class McpContext implements Context {
738741
cpuMultiplier,
739742
networkMultiplier,
740743
);
741-
return waitForHelper.waitForEventsAfterAction(action);
744+
return waitForHelper.waitForEventsAfterAction(action, options);
742745
}
743746

744747
getNetworkRequestStableId(request: HTTPRequest): number {

‎src/WaitForHelper.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,13 @@ export class WaitForHelper {
125125

126126
async waitForEventsAfterAction(
127127
action: () => Promise<unknown>,
128+
options?: {timeout?: number},
128129
): Promise<void> {
129130
const navigationFinished = this.waitForNavigationStarted()
130131
.then(navigationStated => {
131132
if (navigationStated) {
132133
return this.#page.waitForNavigation({
133-
timeout: this.#navigationTimeout,
134+
timeout: options?.timeout ?? this.#navigationTimeout,
134135
signal: this.#abortController.signal,
135136
});
136137
}

‎src/tools/ToolDefinition.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,10 @@ export type Context = Readonly<{
131131
data: Uint8Array<ArrayBufferLike>,
132132
filename: string,
133133
): Promise<{filename: string}>;
134-
waitForEventsAfterAction(action: () => Promise<unknown>): Promise<void>;
134+
waitForEventsAfterAction(
135+
action: () => Promise<unknown>,
136+
options?: {timeout?: number},
137+
): Promise<void>;
135138
waitForTextOnPage(text: string, timeout?: number): Promise<Element>;
136139
getDevToolsData(): Promise<DevToolsData>;
137140
/**

‎src/tools/pages.ts‎

Lines changed: 69 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,14 @@ export const newPage = defineTool({
9898
handler: async (request, response, context) => {
9999
const page = await context.newPage(request.params.background);
100100

101-
await context.waitForEventsAfterAction(async () => {
102-
await page.goto(request.params.url, {
103-
timeout: request.params.timeout,
104-
});
105-
});
101+
await context.waitForEventsAfterAction(
102+
async () => {
103+
await page.goto(request.params.url, {
104+
timeout: request.params.timeout,
105+
});
106+
},
107+
{timeout: request.params.timeout},
108+
);
106109

107110
response.setIncludePages(true);
108111
},
@@ -181,62 +184,67 @@ export const navigatePage = defineTool({
181184
page.on('dialog', dialogHandler);
182185

183186
try {
184-
await context.waitForEventsAfterAction(async () => {
185-
switch (request.params.type) {
186-
case 'url':
187-
if (!request.params.url) {
188-
throw new Error('A URL is required for navigation of type=url.');
189-
}
190-
try {
191-
await page.goto(request.params.url, options);
192-
response.appendResponseLine(
193-
`Successfully navigated to ${request.params.url}.`,
194-
);
195-
} catch (error) {
196-
response.appendResponseLine(
197-
`Unable to navigate in the selected page: ${error.message}.`,
198-
);
199-
}
200-
break;
201-
case 'back':
202-
try {
203-
await page.goBack(options);
204-
response.appendResponseLine(
205-
`Successfully navigated back to ${page.url()}.`,
206-
);
207-
} catch (error) {
208-
response.appendResponseLine(
209-
`Unable to navigate back in the selected page: ${error.message}.`,
210-
);
211-
}
212-
break;
213-
case 'forward':
214-
try {
215-
await page.goForward(options);
216-
response.appendResponseLine(
217-
`Successfully navigated forward to ${page.url()}.`,
218-
);
219-
} catch (error) {
220-
response.appendResponseLine(
221-
`Unable to navigate forward in the selected page: ${error.message}.`,
222-
);
223-
}
224-
break;
225-
case 'reload':
226-
try {
227-
await page.reload({
228-
...options,
229-
ignoreCache: request.params.ignoreCache,
230-
});
231-
response.appendResponseLine(`Successfully reloaded the page.`);
232-
} catch (error) {
233-
response.appendResponseLine(
234-
`Unable to reload the selected page: ${error.message}.`,
235-
);
236-
}
237-
break;
238-
}
239-
});
187+
await context.waitForEventsAfterAction(
188+
async () => {
189+
switch (request.params.type) {
190+
case 'url':
191+
if (!request.params.url) {
192+
throw new Error(
193+
'A URL is required for navigation of type=url.',
194+
);
195+
}
196+
try {
197+
await page.goto(request.params.url, options);
198+
response.appendResponseLine(
199+
`Successfully navigated to ${request.params.url}.`,
200+
);
201+
} catch (error) {
202+
response.appendResponseLine(
203+
`Unable to navigate in the selected page: ${error.message}.`,
204+
);
205+
}
206+
break;
207+
case 'back':
208+
try {
209+
await page.goBack(options);
210+
response.appendResponseLine(
211+
`Successfully navigated back to ${page.url()}.`,
212+
);
213+
} catch (error) {
214+
response.appendResponseLine(
215+
`Unable to navigate back in the selected page: ${error.message}.`,
216+
);
217+
}
218+
break;
219+
case 'forward':
220+
try {
221+
await page.goForward(options);
222+
response.appendResponseLine(
223+
`Successfully navigated forward to ${page.url()}.`,
224+
);
225+
} catch (error) {
226+
response.appendResponseLine(
227+
`Unable to navigate forward in the selected page: ${error.message}.`,
228+
);
229+
}
230+
break;
231+
case 'reload':
232+
try {
233+
await page.reload({
234+
...options,
235+
ignoreCache: request.params.ignoreCache,
236+
});
237+
response.appendResponseLine(`Successfully reloaded the page.`);
238+
} catch (error) {
239+
response.appendResponseLine(
240+
`Unable to reload the selected page: ${error.message}.`,
241+
);
242+
}
243+
break;
244+
}
245+
},
246+
{timeout: request.params.timeout},
247+
);
240248
} finally {
241249
page.off('dialog', dialogHandler);
242250
if (initScriptId) {

‎tests/tools/pages.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import assert from 'node:assert';
88
import {describe, it} from 'node:test';
99

1010
import type {Dialog} from 'puppeteer-core';
11+
import sinon from 'sinon';
1112

1213
import {
1314
listPages,
@@ -160,6 +161,34 @@ describe('pages', () => {
160161
}
161162
});
162163
});
164+
165+
it('respects the timeout parameter', async () => {
166+
await withMcpContext(async (response, context) => {
167+
const page = context.getSelectedPage();
168+
const stub = sinon.stub(page, 'waitForNavigation').resolves(null);
169+
170+
try {
171+
await navigatePage.handler(
172+
{
173+
params: {
174+
url: 'about:blank',
175+
timeout: 12345,
176+
},
177+
},
178+
response,
179+
context,
180+
);
181+
} finally {
182+
stub.restore();
183+
}
184+
185+
assert.strictEqual(
186+
stub.firstCall.args[0]?.timeout,
187+
12345,
188+
'The timeout parameter should be passed to waitForNavigation',
189+
);
190+
});
191+
});
163192
it('go back', async () => {
164193
await withMcpContext(async (response, context) => {
165194
const page = context.getSelectedPage();

0 commit comments

Comments
 (0)