fix: handle errors due to open dialogs during tool calls (#1953)

This change is part of the fix for #1069 
- Errors during tool calls are now consumed and handled as part of
McpResponse format
This commit is contained in:
Samiya Caur
2026-04-28 18:45:48 +02:00
committed by GitHub
parent 66dacb83fd
commit 06b331f403
5 changed files with 90 additions and 25 deletions
+17
View File
@@ -203,6 +203,7 @@ export class McpResponse implements Response {
#args: ParsedArguments;
#page?: McpPage;
#redactNetworkHeaders = true;
#error?: Error;
constructor(args: ParsedArguments) {
this.#args = args;
@@ -307,6 +308,10 @@ export class McpResponse implements Response {
};
}
setError(error: Error): void {
this.#error = error;
}
attachNetworkRequest(
reqId: number,
options?: {requestFilePath?: string; responseFilePath?: string},
@@ -375,6 +380,10 @@ export class McpResponse implements Response {
return this.#consoleDataOptions?.types;
}
get error(): Error | undefined {
return this.#error;
}
appendResponseLine(value: string): void {
this.#textResponseLines.push(value);
}
@@ -662,6 +671,7 @@ export class McpResponse implements Response {
lighthouseResult: this.#attachedLighthouseResult,
inPageTools,
webmcpTools,
errorMessage: this.#error?.message,
});
}
@@ -680,6 +690,7 @@ export class McpResponse implements Response {
lighthouseResult?: LighthouseData;
inPageTools?: ToolGroup<ToolDefinition>;
webmcpTools?: WebMCPTool[];
errorMessage?: string;
},
): {content: Array<TextContent | ImageContent>; structuredContent: object} {
const structuredContent: {
@@ -718,6 +729,7 @@ export class McpResponse implements Response {
heapSnapshotNodes?: readonly object[];
extensionServiceWorkers?: object[];
extensionPages?: object[];
errorMessage?: string;
} = {};
const response = [];
@@ -1080,6 +1092,11 @@ Call ${handleDialog.name} to handle it before continuing.`);
}
}
if (data.errorMessage) {
response.push(`Error: ${data.errorMessage}`);
structuredContent.errorMessage = data.errorMessage;
}
const text: TextContent = {
type: 'text',
text: response.join('\n'),
+6
View File
@@ -128,8 +128,10 @@ export class WaitForHelper {
action: () => Promise<unknown>,
options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},
): Promise<void> {
let dialogOpened = false;
if (options?.handleDialog) {
const dialogHandler = (dialog: Pick<Dialog, 'accept' | 'dismiss'>) => {
dialogOpened = true;
if (options.handleDialog === 'dismiss') {
void dialog.dismiss();
} else if (options.handleDialog === 'accept') {
@@ -167,6 +169,10 @@ export class WaitForHelper {
try {
await navigationFinished;
if (dialogOpened) {
return;
}
// Wait for stable dom after navigation so we execute in
// the correct context
await this.waitForStableDom();
+32 -25
View File
@@ -232,31 +232,35 @@ export async function createMcpServer(
: new McpResponse(serverArgs);
response.setRedactNetworkHeaders(serverArgs.redactNetworkHeaders);
if ('pageScoped' in tool && tool.pageScoped) {
const page =
serverArgs.experimentalPageIdRouting &&
params.pageId &&
!serverArgs.slim
? context.getPageById(params.pageId)
: context.getSelectedMcpPage();
response.setPage(page);
await tool.handler(
{
params,
page,
},
response,
context,
);
} else {
await tool.handler(
// @ts-expect-error types do not match.
{
params,
},
response,
context,
);
try {
if ('pageScoped' in tool && tool.pageScoped) {
const page =
serverArgs.experimentalPageIdRouting &&
params.pageId &&
!serverArgs.slim
? context.getPageById(params.pageId)
: context.getSelectedMcpPage();
response.setPage(page);
await tool.handler(
{
params,
page,
},
response,
context,
);
} else {
await tool.handler(
// @ts-expect-error types do not match.
{
params,
},
response,
context,
);
}
} catch (err) {
response.setError(err);
}
const {content, structuredContent} = await response.handle(
tool.name,
@@ -267,6 +271,9 @@ export async function createMcpServer(
} = {
content,
};
if (response.error) {
result.isError = true;
}
success = true;
if (serverArgs.experimentalStructuredContent) {
result.structuredContent = structuredContent as Record<
+4
View File
@@ -5,3 +5,7 @@ exports[`e2e > calls a tool 1`] = `
exports[`e2e > calls a tool multiple times 1`] = `
[{"type":"text","text":"## Pages\\n1: about:blank [selected]"}]
`;
exports[`e2e > returns blocked message when dialog is opened during tool execution 1`] = `
{"content":[{"type":"text","text":"# Open dialog\\nalert: test dialog.\\nCall handle_dialog to handle it before continuing.\\nError: Failed to interact with the element with uid 1_1. The element did not become interactive within the configured timeout."}],"isError":true}
`;
+31
View File
@@ -245,6 +245,37 @@ describe('e2e', () => {
},
);
});
it('returns blocked message when dialog is opened during tool execution', async t => {
await withClient(async client => {
// Navigate to a page with a button that triggers a dialog on click
await client.callTool({
name: 'new_page',
arguments: {
url: `data:text/html,<button id="test" onclick="alert('test dialog')">Click me</button>`,
},
});
const snapshotResult = await client.callTool({
name: 'take_snapshot',
arguments: {},
});
const snapshotText = (snapshotResult.content as TextContent[])[0].text;
const match = snapshotText.match(/uid=(\d+_\d+)\s+button "Click me"/);
const uid = match ? match[1] : '1_1';
// Trigger the dialog
const result = await client.callTool({
name: 'click',
arguments: {
uid,
},
});
t.assert.snapshot?.(JSON.stringify(result));
});
});
});
async function getToolsWithFilteredCategories(