bug 1553317: remote: improve error message on missing method; r=remote-protocol-reviewers,ochameau
☠☠ backed out by df412702ed31 ☠ ☠
authorAndreas Tolfsen <ato@sny.no>
Mon, 27 May 2019 11:39:16 +0000
changeset 475704 f3d44dbce7e3d1b529ce37b21a4d7471a918edd4
parent 475703 8855dd7a2aa85c0480d180b97d2916b7da06c002
child 475705 1bdbfbb5b3424672257f3c850b69eced2e91f757
push id36072
push userdluca@mozilla.com
push dateTue, 28 May 2019 09:38:00 +0000
treeherdermozilla-central@c6a17846e2d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersremote-protocol-reviewers, ochameau
bugs1553317
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
remote/Error.jsm
remote/sessions/ContentProcessSession.jsm
remote/sessions/Session.jsm
remote/test/browser/browser.ini
remote/test/browser/browser_session.js
remote/test/unit/test_Error.js
--- a/remote/Error.jsm
+++ b/remote/Error.jsm
@@ -95,17 +95,25 @@ class FatalError extends RemoteAgentErro
     Services.startup.quit(mode);
   }
 }
 
 /** When an operation is not yet implemented. */
 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 = [];
 
   els.push(error.name);
   if (error.message) {
     els.push(": ");
     els.push(error.message);
--- a/remote/sessions/ContentProcessSession.jsm
+++ b/remote/sessions/ContentProcessSession.jsm
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 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) {
     this.messageManager = messageManager;
     this.browsingContext = browsingContext;
     this.content = content;
     this.docShell = docShell;
     // Most children or sibling classes are going to assume that docShell
@@ -56,24 +57,21 @@ class ContentProcessSession {
     if (browsingContextId != this.browsingContext.id) {
       return;
     }
 
     switch (name) {
     case "remote:request":
       try {
         const {id, domain, command, params} = data.request;
-
+        if (!this.domains.domainSupportsMethod(domain, command)) {
+          throw new UnknownMethodError(domain, command);
+        }
         const inst = this.domains.get(domain);
-        const func = inst[command];
-        if (!func || typeof func != "function") {
-          throw new Error(`Implementation missing: ${domain}.${command}`);
-        }
-
-        const result = await func.call(inst, params);
+        const result = await inst[command](params);
 
         this.messageManager.sendAsyncMessage("remote:result", {
           browsingContextId,
           id,
           result,
         });
       } catch (e) {
         this.messageManager.sendAsyncMessage("remote:error", {
--- a/remote/sessions/Session.jsm
+++ b/remote/sessions/Session.jsm
@@ -3,17 +3,20 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 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.
  *
  * Every new WebSocket connections is associated with one session that
  * deals with dispatching incoming command requests to the right
  * target, sending back responses, and propagating events originating
  * from domains.
@@ -64,16 +67,19 @@ class Session {
       const {domain, command} = Domains.splitMethod(method);
       await this.execute(id, domain, command, params);
     } catch (e) {
       this.onError(id, e);
     }
   }
 
   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);
   }
 
   onResult(id, result) {
     this.connection.send({
       id,
--- a/remote/test/browser/browser.ini
+++ b/remote/test/browser/browser.ini
@@ -11,10 +11,11 @@ support-files =
 [browser_page_bringToFront.js]
 [browser_page_frameNavigated.js]
 [browser_page_runtime_events.js]
 [browser_runtime_evaluate.js]
 [browser_runtime_remote_objects.js]
 [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]
new file mode 100644
--- /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();
+});
--- a/remote/test/unit/test_Error.js
+++ b/remote/test/unit/test_Error.js
@@ -95,10 +95,12 @@ add_test(function test_RemoteAgentError_
 
 add_test(function test_UnsupportedError() {
   ok(new UnsupportedError() instanceof RemoteAgentError);
   run_next_test();
 });
 
 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();
 });