Bug 1448178 - include actorId in unknown error packet from DevTools server;r=jryans

This will allow client code to catch errors occuring on the server side when
interacting with old-style actors. Before that change, the call from the client
would just never resume.

MozReview-Commit-ID: Ar6q5KapsDy

--HG--
extra : rebase_source : 041584543737c570957fc8b55148e5a824013f2f

devtools/server/main.js

@@ -1604,10 +1604,9 @@ DebuggerServerConnection.prototype = {
     if (actor instanceof ObservedActorFactory) {
       try {
         actor = actor.createActor();
-      } catch (e) {
-        this.transport.send(this._unknownError(
-          "Error occurred while creating actor '" + actor.name,
-          e));
+      } catch (error) {
+        let prefix = "Error occurred while creating actor '" + actor.name;
+        this.transport.send(this._unknownError(actorID, prefix, error));
       }
     } else if (typeof (actor) !== "object") {
       // ActorPools should now contain only actor instances (i.e. objects)
@@ -1628,11 +1627,12 @@ DebuggerServerConnection.prototype = {
     return null;
   },
 
-  _unknownError(prefix, error) {
+  _unknownError(from, prefix, error) {
     let errorString = prefix + ": " + DevToolsUtils.safeErrorString(error);
     reportError(errorString);
     dumpn(errorString);
     return {
+      from,
       error: "unknownError",
       message: errorString
     };
@@ -1651,15 +1651,14 @@ DebuggerServerConnection.prototype = {
         response.from = from;
       }
       this.transport.send(response);
-    }).catch((e) => {
+    }).catch((error) => {
       if (!this.transport) {
         throw new Error(`Connection closed, pending error from ${from}, ` +
                         `type ${type} failed`);
       }
-      let errorPacket = this._unknownError(
-        "error occurred while processing '" + type, e);
-      errorPacket.from = from;
-      this.transport.send(errorPacket);
+
+      let prefix = "error occurred while processing '" + type;
+      this.transport.send(this._unknownError(from, prefix, error));
     });
 
     this._actorResponses.set(from, responsePromise);
@@ -1776,10 +1775,9 @@ DebuggerServerConnection.prototype = {
       try {
         this.currentPacket = packet;
         ret = actor.requestTypes[packet.type].bind(actor)(packet, this);
-      } catch (e) {
-        this.transport.send(this._unknownError(
-          "error occurred while processing '" + packet.type,
-          e));
+      } catch (error) {
+        let prefix = "error occurred while processing '" + packet.type;
+        this.transport.send(this._unknownError(actor.actorID, prefix, error));
       } finally {
         this.currentPacket = undefined;
       }
@@ -1839,10 +1837,10 @@ DebuggerServerConnection.prototype = {
     if (actor.requestTypes && actor.requestTypes[type]) {
       try {
         ret = actor.requestTypes[type].call(actor, packet);
-      } catch (e) {
-        this.transport.send(this._unknownError(
-          "error occurred while processing bulk packet '" + type, e));
-        packet.done.reject(e);
+      } catch (error) {
+        let prefix = "error occurred while processing bulk packet '" + type;
+        this.transport.send(this._unknownError(actorKey, prefix, error));
+        packet.done.reject(error);
       }
     } else {
       let message = "Actor " + actorKey +

devtools/server/tests/browser/browser.ini

@@ -10,6 +10,7 @@ support-files =
   doc_force_gc.html
   doc_innerHTML.html
   doc_perf.html
+  error-actor.js
   grid.html
   inspectedwindow-reload-target.sjs
   navigate-first.html
@@ -31,6 +32,7 @@ support-files =
 [browser_accessibility_node_events.js]
 [browser_accessibility_simple.js]
 [browser_accessibility_walker.js]
+[browser_actor_error.js]
 [browser_animation_emitMutations.js]
 [browser_animation_getFrames.js]
 [browser_animation_getProperties.js]

devtools/server/tests/browser/browser_actor_error.js

@@ -0,0 +1,38 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Test that clients can catch errors in old style actors.
+ */
+
+const ACTORS_URL = "chrome://mochitests/content/browser/devtools/server/tests/browser/error-actor.js";
+
+async function test() {
+  let gClient;
+
+  DebuggerServer.init();
+  DebuggerServer.registerAllActors();
+
+  DebuggerServer.addActors(ACTORS_URL);
+
+  let transport = DebuggerServer.connectPipe();
+  gClient = new DebuggerClient(transport);
+  await gClient.connect();
+
+  let { errorActor } = await gClient.listTabs();
+  ok(errorActor, "Found the error actor.");
+
+  try {
+    await gClient.request({ to: globalActor, type: "error" });
+    ok(false, "The request is expected to fail.");
+  } catch (e) {
+    ok(true, "The request failed as expected, and was caught by the client");
+  }
+  await gClient.close();
+
+  finish();
+}

devtools/server/tests/browser/error-actor.js

@@ -0,0 +1,30 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Test actor designed to check that clients are properly notified of errors when calling
+ * methods on old style actors.
+ */
+function ErrorActor(conn, tab) {
+  this.conn = conn;
+  this.tab = tab;
+}
+
+ErrorActor.prototype = {
+  actorPrefix: "error",
+
+  onError: function() {
+    throw new Error("error");
+  }
+};
+
+ErrorActor.prototype.requestTypes = {
+  "error": ErrorActor.prototype.onError
+};
+
+DebuggerServer.removeGlobalActor(ErrorActor);
+DebuggerServer.addGlobalActor(ErrorActor, "errorActor");