Repository navigation
[WIP] Reject executeTool() failures with the draft's DOMException names - #12
Kulikowski wants to merge 2 commits into
Conversation
f7bcce6 to
cbfab42
Compare
Draft b206dae replaced UnknownError with NotFoundError, DataError, OperationError, AbortError, and InvalidStateError, and eada13b rejects a target without a navigable with InvalidStateError. Frames now carry the failure name in their error replies, and the draft pin moves to eada13b. Pinned WPT still expects, and Chromium still throws, UnknownError (crbug.com/509555636), so six subtests become expected failures and one detachment case passes.
cbfab42 to
5a3e74c
Compare
| still tests them. | ||
| - **Draft differences:** results are JSON-serialized; some pinned tests expect raw | ||
| strings. Omitted or `undefined` input becomes `{}`; `null` and primitives reject. | ||
| - **Execution errors:** failures reject with the draft's `NotFoundError`, `DataError`, |
There was a problem hiding this comment.
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.
I think it is a good idea to wait for Chromium changes here
|
|
||
| `executeTool()` accepts an object and returns a JSON-serialized result. Omitted or `undefined` input defaults to a fresh empty object. Callbacks must validate their inputs; schema inference provides TypeScript checks only. | ||
|
|
||
| A failed execution rejects with the draft's `DOMException` name: `NotFoundError` when the target has no such tool exposed to the caller, `DataError` when the input does not parse to an object, `OperationError` when the callback throws, rejects, or returns an unserializable result, `AbortError` when the target document unloads during the call, and `InvalidStateError` when the target frame was already removed or is in another frame tree. The callback's own error stays in its frame. |
There was a problem hiding this comment.
It may be a bit much in this README. WDYT?
There was a problem hiding this comment.
Agreed. I cut it to one sentence
| const executionFailures = { | ||
| NotFoundError: "The tool is not registered in the target document", | ||
| DataError: "The tool input could not be parsed", | ||
| OperationError: "The tool failed or returned an unserializable result", | ||
| AbortError: "The target document was unloaded", | ||
| InvalidStateError: "The target document is detached or in another frame tree", | ||
| NotAllowedError: "WebMCP is disabled by Permissions Policy", | ||
| } as const; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Thanks! It now uses Chromium's messages
|
Changes have just landed: https://chromiumdash.appspot.com/commit/0b6fe2677649e16ed923849bb9ab70dd77c1a7c7 |
The PR is updated to match. |
2 participants
The spec now gives each
executeTool()failure its own error name instead ofUnknownError(webmcp#339), and rejects a call to an already-removed frame withInvalidStateError(webmcp#337). This PR does the same, and TESTING.md now tracks spec versioneada13b.UnknownErrorNotFoundErrorUnknownErrorDataErrorUnknownErrorOperationErrorUnknownErrorAbortErrorUnknownErrorInvalidStateErrorTBD