From 26201b29a2a9858078d636df6311864f48fd2afe Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Mon, 27 May 2019 11:39:16 +0000 Subject: [PATCH] bug 1553317: remote: improve error message on missing method; r=remote-protocol-reviewers,ochameau We return with this rather omnious message when we are missing the implementation of a CDP method: Error: Protocol error (Target.createBrowserContext): TypeError: inst[command] is not a function: This patch improves the error message so that debugging is not necessary to find out which domain or command is missing. Ideally Session.jsm and ContentProcessSession.jsm would share the same execute() function (there's really not reason they don't), but that involves more work. Differential Revision: https://phabricator.services.mozilla.com/D32069 --HG-- extra : moz-landing-system : lando --- remote/Error.jsm | 10 ++++++++- remote/sessions/ContentProcessSession.jsm | 12 +++++------ remote/sessions/Session.jsm | 8 +++++++- remote/test/browser/browser.ini | 1 + remote/test/browser/browser_session.js | 25 +++++++++++++++++++++++ remote/test/unit/test_Error.js | 2 ++ 6 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 remote/test/browser/browser_session.js diff --git a/remote/Error.jsm b/remote/Error.jsm index 3e888d00bfb5..75bb353f970a 100644 --- a/remote/Error.jsm +++ b/remote/Error.jsm @@ -100,7 +100,15 @@ class FatalError extends RemoteAgentError { class UnsupportedError extends RemoteAgentError {} /** The requested remote method does not exist. */ -class UnknownMethodError extends RemoteAgentError {} +class UnknownMethodError extends RemoteAgentError { + constructor(domain, command = null) { + if (command) { + super(`${domain}.${command}`); + } else { + super(domain); + } + } +} function formatError(error, {stack = false} = {}) { const els = []; diff --git a/remote/sessions/ContentProcessSession.jsm b/remote/sessions/ContentProcessSession.jsm index 30bd397862c4..a9930d8f0144 100644 --- a/remote/sessions/ContentProcessSession.jsm +++ b/remote/sessions/ContentProcessSession.jsm @@ -8,6 +8,7 @@ var EXPORTED_SYMBOLS = ["ContentProcessSession"]; const {ContentProcessDomains} = ChromeUtils.import("chrome://remote/content/domains/ContentProcessDomains.jsm"); const {Domains} = ChromeUtils.import("chrome://remote/content/domains/Domains.jsm"); +const {UnknownMethodError} = ChromeUtils.import("chrome://remote/content/Error.jsm"); class ContentProcessSession { constructor(messageManager, browsingContext, content, docShell) { @@ -61,14 +62,11 @@ class ContentProcessSession { case "remote:request": try { const {id, domain, command, params} = data.request; - - const inst = this.domains.get(domain); - const func = inst[command]; - if (!func || typeof func != "function") { - throw new Error(`Implementation missing: ${domain}.${command}`); + if (!this.domains.domainSupportsMethod(domain, command)) { + throw new UnknownMethodError(domain, command); } - - const result = await func.call(inst, params); + const inst = this.domains.get(domain); + const result = await inst[command](params); this.messageManager.sendAsyncMessage("remote:result", { browsingContextId, diff --git a/remote/sessions/Session.jsm b/remote/sessions/Session.jsm index 2dc6ff9fa6c5..46ffbd2f94c3 100644 --- a/remote/sessions/Session.jsm +++ b/remote/sessions/Session.jsm @@ -8,7 +8,10 @@ var EXPORTED_SYMBOLS = ["Session"]; const {ParentProcessDomains} = ChromeUtils.import("chrome://remote/content/domains/ParentProcessDomains.jsm"); const {Domains} = ChromeUtils.import("chrome://remote/content/domains/Domains.jsm"); -const {RemoteAgentError} = ChromeUtils.import("chrome://remote/content/Error.jsm"); +const { + RemoteAgentError, + UnknownMethodError, +} = ChromeUtils.import("chrome://remote/content/Error.jsm"); /** * A session represents exactly one client WebSocket connection. @@ -69,6 +72,9 @@ class Session { } async execute(id, domain, command, params) { + if (!this.domains.domainSupportsMethod(domain, command)) { + throw new UnknownMethodError(domain, command); + } const inst = this.domains.get(domain); const result = await inst[command](params); this.onResult(id, result); diff --git a/remote/test/browser/browser.ini b/remote/test/browser/browser.ini index a8df818e5bc0..e16474e93c5a 100644 --- a/remote/test/browser/browser.ini +++ b/remote/test/browser/browser.ini @@ -16,5 +16,6 @@ support-files = [browser_runtime_callFunctionOn.js] [browser_runtime_executionContext.js] skip-if = os == "mac" || (verify && os == 'win') # bug 1547961 +[browser_session.js] [browser_tabs.js] [browser_target.js] diff --git a/remote/test/browser/browser_session.js b/remote/test/browser/browser_session.js new file mode 100644 index 000000000000..18c63bf0b909 --- /dev/null +++ b/remote/test/browser/browser_session.js @@ -0,0 +1,25 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function() { + await RemoteAgent.listen(Services.io.newURI("http://localhost:9222")); + const CDP = await getCDP(); + const {webSocketDebuggerUrl} = await CDP.Version(); + const client = await CDP({"target": webSocketDebuggerUrl}); + + try { + await client.send("Hoobaflooba"); + } catch (e) { + ok(e.message.match(/TypeError: Invalid method format/)); + } + try { + await client.send("Hooba.flooba"); + } catch (e) { + ok(e.message.match(/UnknownMethodError/)); + } + + await client.close(); + await RemoteAgent.close(); +}); diff --git a/remote/test/unit/test_Error.js b/remote/test/unit/test_Error.js index 1b080f7a3201..ec45fdca1273 100644 --- a/remote/test/unit/test_Error.js +++ b/remote/test/unit/test_Error.js @@ -100,5 +100,7 @@ add_test(function test_UnsupportedError() { add_test(function test_UnknownMethodError() { ok(new UnknownMethodError() instanceof RemoteAgentError); + ok(new UnknownMethodError("domain").message.endsWith("domain")); + ok(new UnknownMethodError("domain", "command").message.endsWith("domain.command")); run_next_test(); });