Skip to content

Commit 06b331f

Browse files
authored
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
1 parent 66dacb8 commit 06b331f

5 files changed

Lines changed: 90 additions & 25 deletions

File tree

src/McpResponse.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ export class McpResponse implements Response {
203203
#args: ParsedArguments;
204204
#page?: McpPage;
205205
#redactNetworkHeaders = true;
206+
#error?: Error;
206207

207208
constructor(args: ParsedArguments) {
208209
this.#args = args;
@@ -307,6 +308,10 @@ export class McpResponse implements Response {
307308
};
308309
}
309310

311+
setError(error: Error): void {
312+
this.#error = error;
313+
}
314+
310315
attachNetworkRequest(
311316
reqId: number,
312317
options?: {requestFilePath?: string; responseFilePath?: string},
@@ -375,6 +380,10 @@ export class McpResponse implements Response {
375380
return this.#consoleDataOptions?.types;
376381
}
377382

383+
get error(): Error | undefined {
384+
return this.#error;
385+
}
386+
378387
appendResponseLine(value: string): void {
379388
this.#textResponseLines.push(value);
380389
}
@@ -662,6 +671,7 @@ export class McpResponse implements Response {
662671
lighthouseResult: this.#attachedLighthouseResult,
663672
inPageTools,
664673
webmcpTools,
674+
errorMessage: this.#error?.message,
665675
});
666676
}
667677

@@ -680,6 +690,7 @@ export class McpResponse implements Response {
680690
lighthouseResult?: LighthouseData;
681691
inPageTools?: ToolGroup<ToolDefinition>;
682692
webmcpTools?: WebMCPTool[];
693+
errorMessage?: string;
683694
},
684695
): {content: Array<TextContent | ImageContent>; structuredContent: object} {
685696
const structuredContent: {
@@ -718,6 +729,7 @@ export class McpResponse implements Response {
718729
heapSnapshotNodes?: readonly object[];
719730
extensionServiceWorkers?: object[];
720731
extensionPages?: object[];
732+
errorMessage?: string;
721733
} = {};
722734

723735
const response = [];
@@ -1080,6 +1092,11 @@ Call ${handleDialog.name} to handle it before continuing.`);
10801092
}
10811093
}
10821094

1095+
if (data.errorMessage) {
1096+
response.push(`Error: ${data.errorMessage}`);
1097+
structuredContent.errorMessage = data.errorMessage;
1098+
}
1099+
10831100
const text: TextContent = {
10841101
type: 'text',
10851102
text: response.join('\n'),

src/WaitForHelper.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ export class WaitForHelper {
128128
action: () => Promise<unknown>,
129129
options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},
130130
): Promise<void> {
131+
let dialogOpened = false;
131132
if (options?.handleDialog) {
132133
const dialogHandler = (dialog: Pick<Dialog, 'accept' | 'dismiss'>) => {
134+
dialogOpened = true;
133135
if (options.handleDialog === 'dismiss') {
134136
void dialog.dismiss();
135137
} else if (options.handleDialog === 'accept') {
@@ -167,6 +169,10 @@ export class WaitForHelper {
167169
try {
168170
await navigationFinished;
169171

172+
if (dialogOpened) {
173+
return;
174+
}
175+
170176
// Wait for stable dom after navigation so we execute in
171177
// the correct context
172178
await this.waitForStableDom();

src/index.ts

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -232,31 +232,35 @@ export async function createMcpServer(
232232
: new McpResponse(serverArgs);
233233

234234
response.setRedactNetworkHeaders(serverArgs.redactNetworkHeaders);
235-
if ('pageScoped' in tool && tool.pageScoped) {
236-
const page =
237-
serverArgs.experimentalPageIdRouting &&
238-
params.pageId &&
239-
!serverArgs.slim
240-
? context.getPageById(params.pageId)
241-
: context.getSelectedMcpPage();
242-
response.setPage(page);
243-
await tool.handler(
244-
{
245-
params,
246-
page,
247-
},
248-
response,
249-
context,
250-
);
251-
} else {
252-
await tool.handler(
253-
// @ts-expect-error types do not match.
254-
{
255-
params,
256-
},
257-
response,
258-
context,
259-
);
235+
try {
236+
if ('pageScoped' in tool && tool.pageScoped) {
237+
const page =
238+
serverArgs.experimentalPageIdRouting &&
239+
params.pageId &&
240+
!serverArgs.slim
241+
? context.getPageById(params.pageId)
242+
: context.getSelectedMcpPage();
243+
response.setPage(page);
244+
await tool.handler(
245+
{
246+
params,
247+
page,
248+
},
249+
response,
250+
context,
251+
);
252+
} else {
253+
await tool.handler(
254+
// @ts-expect-error types do not match.
255+
{
256+
params,
257+
},
258+
response,
259+
context,
260+
);
261+
}
262+
} catch (err) {
263+
response.setError(err);
260264
}
261265
const {content, structuredContent} = await response.handle(
262266
tool.name,
@@ -267,6 +271,9 @@ export async function createMcpServer(
267271
} = {
268272
content,
269273
};
274+
if (response.error) {
275+
result.isError = true;
276+
}
270277
success = true;
271278
if (serverArgs.experimentalStructuredContent) {
272279
result.structuredContent = structuredContent as Record<

tests/index.test.js.snapshot

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,7 @@ exports[`e2e > calls a tool 1`] = `
55
exports[`e2e > calls a tool multiple times 1`] = `
66
[{"type":"text","text":"## Pages\\n1: about:blank [selected]"}]
77
`;
8+
9+
exports[`e2e > returns blocked message when dialog is opened during tool execution 1`] = `
10+
{"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}
11+
`;

tests/index.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,37 @@ describe('e2e', () => {
245245
},
246246
);
247247
});
248+
249+
it('returns blocked message when dialog is opened during tool execution', async t => {
250+
await withClient(async client => {
251+
// Navigate to a page with a button that triggers a dialog on click
252+
await client.callTool({
253+
name: 'new_page',
254+
arguments: {
255+
url: `data:text/html,<button id="test" onclick="alert('test dialog')">Click me</button>`,
256+
},
257+
});
258+
259+
const snapshotResult = await client.callTool({
260+
name: 'take_snapshot',
261+
arguments: {},
262+
});
263+
264+
const snapshotText = (snapshotResult.content as TextContent[])[0].text;
265+
const match = snapshotText.match(/uid=(\d+_\d+)\s+button "Click me"/);
266+
const uid = match ? match[1] : '1_1';
267+
268+
// Trigger the dialog
269+
const result = await client.callTool({
270+
name: 'click',
271+
arguments: {
272+
uid,
273+
},
274+
});
275+
276+
t.assert.snapshot?.(JSON.stringify(result));
277+
});
278+
});
248279
});
249280

250281
async function getToolsWithFilteredCategories(

0 commit comments

Comments
 (0)