Repository navigation
[WIP] Reject executeTool() failures with the draft's DOMException names #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,8 @@ export class FrameBridge { | |
| toRegisteredTool(readToolMetadata(value), peer, reply.origin), | ||
| ); | ||
| } catch (error) { | ||
| if (!(error instanceof NativeDOMException && error.name === "UnknownError")) { | ||
| // Refusals and missing peers are expected; malformed replies and other errors are not. | ||
| if (!isExecutionError(error)) { | ||
| console.warn("WebMCP: could not read tools from a frame.", error); | ||
| } | ||
| return []; | ||
|
|
@@ -143,7 +144,7 @@ export class FrameBridge { | |
| signal, | ||
| ); | ||
| if (typeof reply.value !== "string") { | ||
| throw executionError(); | ||
| throw executionError("OperationError"); | ||
| } | ||
| return reply.value; | ||
| } | ||
|
|
@@ -173,8 +174,17 @@ export class FrameBridge { | |
| ): Promise<{ value: unknown; origin: string }> { | ||
| return new Promise((resolve, reject) => { | ||
| signal?.throwIfAborted(); | ||
| if (this.#hidden || !this.#related(target)) { | ||
| reject(executionError()); | ||
| if (this.#hidden) { | ||
| reject(new NativeDOMException("The document is not active.", "InvalidStateError")); | ||
| return; | ||
| } | ||
| // executeTool() rejects a target that is already closed, so one closed since has unloaded. | ||
| if (target.closed) { | ||
| reject(executionError("AbortError")); | ||
| return; | ||
| } | ||
| if (!this.#related(target)) { | ||
| reject(executionError("InvalidStateError")); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -189,7 +199,7 @@ export class FrameBridge { | |
| this.#handshakes.delete(id); | ||
| port1.close(); | ||
| }; | ||
| const fail = (error: unknown = executionError()): void => { | ||
| const fail = (error: unknown): void => { | ||
| port1.postMessage({ kind: "cancel" }); | ||
| close(); | ||
| reject(error); | ||
|
|
@@ -199,15 +209,16 @@ export class FrameBridge { | |
| if (peerOrigin === undefined) { | ||
| this.#forget(target); | ||
| } | ||
| fail(); | ||
| // A window that misses the deadline is treated as holding no polyfill, and so no tools. | ||
| fail(executionError("NotFoundError")); | ||
| }; | ||
| const connected = (event: MessageEvent<unknown>): void => { | ||
| if (event.source !== target) { | ||
| return; | ||
| } | ||
| this.#handshakes.delete(id); | ||
| if (origins && !origins.includes(event.origin)) { | ||
| fail(); | ||
| fail(executionError("NotFoundError")); | ||
| return; | ||
| } | ||
| peerOrigin = event.origin; | ||
|
|
@@ -220,12 +231,12 @@ export class FrameBridge { | |
| }; | ||
|
|
||
| let timer = setTimeout(expire, deadline); | ||
| const stopWatching = this.#watch(target, fail); | ||
| const stopWatching = this.#watch(target, () => fail(executionError("AbortError"))); | ||
| port1.onmessage = (event: MessageEvent<unknown>) => { | ||
| const reply = record(event.data); | ||
| // A peer that gave up may say so before its ready message is handled here. | ||
| if (reply?.kind === "error") { | ||
| fail(); | ||
| fail(executionError(readExecutionFailure(reply.name))); | ||
| return; | ||
| } | ||
| if (reply?.kind !== "result" || peerOrigin === undefined) { | ||
|
|
@@ -234,15 +245,15 @@ export class FrameBridge { | |
| close(); | ||
| resolve({ value: reply.value, origin: peerOrigin }); | ||
| }; | ||
| port1.onmessageerror = () => fail(); | ||
| port1.onmessageerror = () => fail(executionError("OperationError")); | ||
| signal?.addEventListener("abort", abort, { once: true }); | ||
| this.#handshakes.set(id, connected); | ||
| // The peer's origin is unknown until it answers, and this message carries only the id. | ||
| try { | ||
| target.postMessage(`${protocol}:connect:${id}`, "*", [port2]); | ||
| } catch { | ||
| port2.close(); | ||
| fail(); | ||
| fail(executionError("OperationError")); | ||
| } | ||
| }); | ||
| } | ||
|
|
@@ -319,15 +330,16 @@ export class FrameBridge { | |
| controller.abort(); | ||
| close(); | ||
| }; | ||
| const fail = (): void => { | ||
| port.postMessage({ kind: "error" }); | ||
| const fail = (name: ExecutionFailure): void => { | ||
| port.postMessage({ kind: "error", name }); | ||
| cancel(); | ||
| }; | ||
| // Tell a requester that was too slow to send its request; it may have no deadline left. | ||
| const timer = setTimeout(fail, deadline); | ||
| const stopWatching = this.#watch(source, fail); | ||
| const timer = setTimeout(() => fail("OperationError"), deadline); | ||
| // The target leaving the tree unloads its document; if the caller left, no one reads this. | ||
| const stopWatching = this.#watch(source, () => fail("AbortError")); | ||
|
|
||
| port.onmessageerror = fail; | ||
| port.onmessageerror = () => fail("OperationError"); | ||
| port.onmessage = async (event: MessageEvent<unknown>) => { | ||
| const request = record(event.data); | ||
| if (request?.kind === "cancel") { | ||
|
|
@@ -343,8 +355,8 @@ export class FrameBridge { | |
| const value = await this.#respond(source, origin, request, controller.signal); | ||
| port.postMessage({ kind: "result", value }); | ||
| close(); | ||
| } catch { | ||
| fail(); | ||
| } catch (error) { | ||
| fail(isExecutionError(error) ? error.name : "OperationError"); | ||
| } | ||
| }; | ||
| source.postMessage(`${protocol}:ready:${id}`, origin); | ||
|
|
@@ -356,8 +368,11 @@ export class FrameBridge { | |
| request: Record<string, unknown> | undefined, | ||
| signal: AbortSignal, | ||
| ): Promise<ReplyValue> { | ||
| if (!request || !this.#related(source)) { | ||
| throw executionError(); | ||
| if (!request) { | ||
| throw executionError("OperationError"); | ||
| } | ||
| if (!this.#related(source)) { | ||
| throw executionError("InvalidStateError"); | ||
| } | ||
| if (request.kind === "permission") { | ||
| const { childIndex, childOrigin } = request; | ||
|
|
@@ -367,16 +382,20 @@ export class FrameBridge { | |
| childIndex < 0 || | ||
| typeof childOrigin !== "string" | ||
| ) { | ||
| throw executionError(); | ||
| throw executionError("OperationError"); | ||
| } | ||
| return this.#childPermission(childIndex, childOrigin); | ||
| } | ||
| if (!(await this.allowed()) || !(await this.#sourceAllowed(source, origin))) { | ||
| throw executionError(); | ||
| if (!(await this.allowed())) { | ||
| // A document that may not use the API cannot register tools, so it has none to find. | ||
| throw executionError(this.#hidden ? "AbortError" : "NotFoundError"); | ||
| } | ||
| if (!(await this.#sourceAllowed(source, origin))) { | ||
| throw executionError("NotAllowedError"); | ||
| } | ||
| // The checks above may have waited on other frames. | ||
| if (!this.#related(source)) { | ||
| throw executionError(); | ||
| throw executionError("InvalidStateError"); | ||
| } | ||
| signal.throwIfAborted(); | ||
| if (request.kind === "getTools") { | ||
|
|
@@ -393,7 +412,7 @@ export class FrameBridge { | |
| await this.#handlers.changed(); | ||
| return undefined; | ||
| } | ||
| throw executionError(); | ||
| throw executionError("OperationError"); | ||
| } | ||
|
|
||
| async #childPermission(childIndex: number, childOrigin: string): Promise<boolean> { | ||
|
|
@@ -768,6 +787,36 @@ function isWindow(value: MessageEventSource): value is Window { | |
| } | ||
| } | ||
|
|
||
| export function executionError(): DOMException { | ||
| return new NativeDOMException("Tool execution failed", "UnknownError"); | ||
| // The first four names are the draft's tool execution errors: | ||
| // https://webmachinelearning.github.io/webmcp/#tool-execution-error | ||
| // InvalidStateError and NotAllowedError come from other executeTool() steps. Protocol failures | ||
| // without a draft counterpart, such as a malformed reply, are reported as OperationError. | ||
| // The messages are Chromium's: https://crrev.com/c/8546218 | ||
| const executionFailures = { | ||
| NotFoundError: "The requested tool was not found or is not accessible.", | ||
| DataError: "Failed to parse tool input arguments.", | ||
| OperationError: "Tool execution failed.", | ||
| AbortError: "Tool execution aborted.", | ||
| InvalidStateError: "Target frame is not accessible.", | ||
| NotAllowedError: 'Access to the feature "tools" is disallowed by permissions policy.', | ||
| } as const; | ||
|
Comment on lines
+795
to
+802
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can look at https://chromium-review.googlesource.com/c/chromium/src/+/8546218/6/third_party/blink/renderer/core/script_tools/model_context.cc for Chromium error messages
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! It now uses Chromium's messages |
||
|
|
||
| export type ExecutionFailure = keyof typeof executionFailures; | ||
|
|
||
| export function executionError(name: ExecutionFailure): DOMException { | ||
| // Each name has one fixed message, so a message reveals nothing beyond its name. | ||
| return new NativeDOMException(executionFailures[name], name); | ||
| } | ||
|
|
||
| function isExecutionError(error: unknown): error is DOMException & { name: ExecutionFailure } { | ||
| return error instanceof NativeDOMException && Object.hasOwn(executionFailures, error.name); | ||
| } | ||
|
|
||
| // Another frame's reply is untrusted input. | ||
| function readExecutionFailure(value: unknown): ExecutionFailure { | ||
| if (typeof value === "string" && Object.hasOwn(executionFailures, value)) { | ||
| // SAFETY: the own-property check above limits the string to the table's names. | ||
| return value as ExecutionFailure; | ||
| } | ||
| return "OperationError"; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to wait until https://chromium-review.googlesource.com/c/chromium/src/+/8546218 is merged so that we can check the polyfill works as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is a good idea to wait for Chromium changes here