From 6c818f7b17cb5255b75d6e6edfe7cca86597575e Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Fri, 30 Jan 2026 10:17:42 +0100 Subject: [PATCH] fix: respect custom timeouts in navigate tools --- src/McpContext.ts | 7 +- src/WaitForHelper.ts | 3 +- src/tools/ToolDefinition.ts | 5 +- src/tools/pages.ts | 130 +++++++++++++++++++----------------- tests/tools/pages.test.ts | 29 ++++++++ 5 files changed, 109 insertions(+), 65 deletions(-) diff --git a/src/McpContext.ts b/src/McpContext.ts index 11c0a089b..bee6d637c 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -727,7 +727,10 @@ export class McpContext implements Context { return new WaitForHelper(page, cpuMultiplier, networkMultiplier); } - waitForEventsAfterAction(action: () => Promise): Promise { + waitForEventsAfterAction( + action: () => Promise, + options?: {timeout?: number}, + ): Promise { const page = this.getSelectedPage(); const cpuMultiplier = this.getCpuThrottlingRate(); const networkMultiplier = getNetworkMultiplierFromString( @@ -738,7 +741,7 @@ export class McpContext implements Context { cpuMultiplier, networkMultiplier, ); - return waitForHelper.waitForEventsAfterAction(action); + return waitForHelper.waitForEventsAfterAction(action, options); } getNetworkRequestStableId(request: HTTPRequest): number { diff --git a/src/WaitForHelper.ts b/src/WaitForHelper.ts index 44dbdd3c2..6c84daf12 100644 --- a/src/WaitForHelper.ts +++ b/src/WaitForHelper.ts @@ -125,12 +125,13 @@ export class WaitForHelper { async waitForEventsAfterAction( action: () => Promise, + options?: {timeout?: number}, ): Promise { const navigationFinished = this.waitForNavigationStarted() .then(navigationStated => { if (navigationStated) { return this.#page.waitForNavigation({ - timeout: this.#navigationTimeout, + timeout: options?.timeout ?? this.#navigationTimeout, signal: this.#abortController.signal, }); } diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index 4657283f3..ed38a2cce 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -131,7 +131,10 @@ export type Context = Readonly<{ data: Uint8Array, filename: string, ): Promise<{filename: string}>; - waitForEventsAfterAction(action: () => Promise): Promise; + waitForEventsAfterAction( + action: () => Promise, + options?: {timeout?: number}, + ): Promise; waitForTextOnPage(text: string, timeout?: number): Promise; getDevToolsData(): Promise; /** diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 7b1a6e9e0..60fc8f3b6 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -98,11 +98,14 @@ export const newPage = defineTool({ handler: async (request, response, context) => { const page = await context.newPage(request.params.background); - await context.waitForEventsAfterAction(async () => { - await page.goto(request.params.url, { - timeout: request.params.timeout, - }); - }); + await context.waitForEventsAfterAction( + async () => { + await page.goto(request.params.url, { + timeout: request.params.timeout, + }); + }, + {timeout: request.params.timeout}, + ); response.setIncludePages(true); }, @@ -181,62 +184,67 @@ export const navigatePage = defineTool({ page.on('dialog', dialogHandler); try { - await context.waitForEventsAfterAction(async () => { - switch (request.params.type) { - case 'url': - if (!request.params.url) { - throw new Error('A URL is required for navigation of type=url.'); - } - try { - await page.goto(request.params.url, options); - response.appendResponseLine( - `Successfully navigated to ${request.params.url}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate in the selected page: ${error.message}.`, - ); - } - break; - case 'back': - try { - await page.goBack(options); - response.appendResponseLine( - `Successfully navigated back to ${page.url()}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate back in the selected page: ${error.message}.`, - ); - } - break; - case 'forward': - try { - await page.goForward(options); - response.appendResponseLine( - `Successfully navigated forward to ${page.url()}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate forward in the selected page: ${error.message}.`, - ); - } - break; - case 'reload': - try { - await page.reload({ - ...options, - ignoreCache: request.params.ignoreCache, - }); - response.appendResponseLine(`Successfully reloaded the page.`); - } catch (error) { - response.appendResponseLine( - `Unable to reload the selected page: ${error.message}.`, - ); - } - break; - } - }); + await context.waitForEventsAfterAction( + async () => { + switch (request.params.type) { + case 'url': + if (!request.params.url) { + throw new Error( + 'A URL is required for navigation of type=url.', + ); + } + try { + await page.goto(request.params.url, options); + response.appendResponseLine( + `Successfully navigated to ${request.params.url}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate in the selected page: ${error.message}.`, + ); + } + break; + case 'back': + try { + await page.goBack(options); + response.appendResponseLine( + `Successfully navigated back to ${page.url()}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate back in the selected page: ${error.message}.`, + ); + } + break; + case 'forward': + try { + await page.goForward(options); + response.appendResponseLine( + `Successfully navigated forward to ${page.url()}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate forward in the selected page: ${error.message}.`, + ); + } + break; + case 'reload': + try { + await page.reload({ + ...options, + ignoreCache: request.params.ignoreCache, + }); + response.appendResponseLine(`Successfully reloaded the page.`); + } catch (error) { + response.appendResponseLine( + `Unable to reload the selected page: ${error.message}.`, + ); + } + break; + } + }, + {timeout: request.params.timeout}, + ); } finally { page.off('dialog', dialogHandler); if (initScriptId) { diff --git a/tests/tools/pages.test.ts b/tests/tools/pages.test.ts index b3f6767dd..2d4110f8b 100644 --- a/tests/tools/pages.test.ts +++ b/tests/tools/pages.test.ts @@ -8,6 +8,7 @@ import assert from 'node:assert'; import {describe, it} from 'node:test'; import type {Dialog} from 'puppeteer-core'; +import sinon from 'sinon'; import { listPages, @@ -160,6 +161,34 @@ describe('pages', () => { } }); }); + + it('respects the timeout parameter', async () => { + await withMcpContext(async (response, context) => { + const page = context.getSelectedPage(); + const stub = sinon.stub(page, 'waitForNavigation').resolves(null); + + try { + await navigatePage.handler( + { + params: { + url: 'about:blank', + timeout: 12345, + }, + }, + response, + context, + ); + } finally { + stub.restore(); + } + + assert.strictEqual( + stub.firstCall.args[0]?.timeout, + 12345, + 'The timeout parameter should be passed to waitForNavigation', + ); + }); + }); it('go back', async () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPage();