Bug 1450944 - Throw in actor methods instead of returning an error packet; r=ochameau. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 17 May 2018 15:04:24 +0200
changeset 811788 dcc87d659281b909fe973da33b13b9a655d9d300
parent 811787 3c2400ba0bdc0d12daf339667fe8c50891c194a0
push id114426
push userbmo:nchevobbe@mozilla.com
push dateThu, 28 Jun 2018 10:24:35 +0000
reviewersochameau
bugs1450944
milestone63.0a1
Bug 1450944 - Throw in actor methods instead of returning an error packet; r=ochameau. Since protocol.js catches exceptions and returns an appropriate packet as a result, we can use this instead of returning manually an error packet. MozReview-Commit-ID: 6lREam5sEVs
devtools/server/actors/object.js
devtools/shared/protocol.js
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -479,18 +479,17 @@ const proto = {
    * Handle a protocol request to provide the property descriptor of the
    * object's specified property.
    *
    * @param name string
    *        The property we want the description of.
    */
   property: function(name) {
     if (!name) {
-      return { error: "missingParameter",
-               message: "no property name was specified" };
+      return this.throwError("missingParameter", "no property name was specified");
     }
 
     return { descriptor: this._propertyDescriptor(name) };
   },
 
   /**
    * Handle a protocol request to provide the display string for the object.
    */
@@ -570,56 +569,50 @@ const proto = {
 
   /**
    * Handle a protocol request to provide the source code of a function.
    *
    * @param pretty boolean
    */
   decompile: function(pretty) {
     if (this.obj.class !== "Function") {
-      return { error: "objectNotFunction",
-               message: "decompile request is only valid for object grips " +
-                        "with a 'Function' class." };
+      return this.throwError("objectNotFunction",
+        "decompile request is only valid for grips  with a 'Function' class.");
     }
 
     return { decompiledCode: this.obj.decompile(!!pretty) };
   },
 
   /**
    * Handle a protocol request to provide the parameters of a function.
    */
   parameterNames: function() {
     if (this.obj.class !== "Function") {
-      return { error: "objectNotFunction",
-               message: "'parameterNames' request is only valid for object " +
-                        "grips with a 'Function' class." };
+      return this.throwError("objectNotFunction",
+        "'parameterNames' request is only valid for grips with a 'Function' class.");
     }
 
     return { parameterNames: this.obj.parameterNames };
   },
 
   /**
    * Handle a protocol request to provide the lexical scope of a function.
    */
   scope: function() {
     if (this.obj.class !== "Function") {
-      return {
-        error: "objectNotFunction",
-        message: "scope request is only valid for object grips with a 'Function' class."
-      };
+      return this.throwError("objectNotFunction",
+        "scope request is only valid for grips with a 'Function' class.");
     }
 
     const { createEnvironmentActor } = this.hooks;
     const envActor = createEnvironmentActor(this.obj.environment, this.registeredPool);
 
     if (!envActor) {
-      return {
-        error: "notDebuggee",
-        message: "cannot access the environment of this function."
-      };
+      return this.throwError("notDebuggee",
+        "cannot access the environment of this function.");
     }
 
     return {
       scope: envActor
     };
   },
 
   /**
@@ -627,35 +620,33 @@ const proto = {
    * promise.
    *
    * @return object
    *         Returns an object containing an array of object grips of the
    *         dependent promises
    */
   dependentPromises: function() {
     if (this.obj.class != "Promise") {
-      return { error: "objectNotPromise",
-               message: "'dependentPromises' request is only valid for " +
-                        "object grips with a 'Promise' class." };
+      return this.throwError("objectNotPromise",
+        "'dependentPromises' request is only valid for grips with a 'Promise' class.");
     }
 
     const promises = this.obj.promiseDependentPromises
                            .map(p => this.hooks.createValueGrip(p));
 
     return { promises };
   },
 
   /**
    * Handle a protocol request to get the allocation stack of a promise.
    */
   allocationStack: function() {
     if (this.obj.class != "Promise") {
-      return { error: "objectNotPromise",
-               message: "'allocationStack' request is only valid for " +
-                        "object grips with a 'Promise' class." };
+      return this.throwError("objectNotPromise",
+        "'allocationStack' request is only valid for grips with a 'Promise' class.");
     }
 
     let stack = this.obj.promiseAllocationSite;
     const allocationStacks = [];
 
     while (stack) {
       if (stack.source) {
         const source = this._getSourceOriginalLocation(stack);
@@ -670,19 +661,18 @@ const proto = {
     return Promise.all(allocationStacks);
   },
 
   /**
    * Handle a protocol request to get the fulfillment stack of a promise.
    */
   fulfillmentStack: function() {
     if (this.obj.class != "Promise") {
-      return { error: "objectNotPromise",
-               message: "'fulfillmentStack' request is only valid for " +
-                        "object grips with a 'Promise' class." };
+      return this.throwError("objectNotPromise",
+        "'fulfillmentStack' request is only valid for grips with a 'Promise' class.");
     }
 
     let stack = this.obj.promiseResolutionSite;
     const fulfillmentStacks = [];
 
     while (stack) {
       if (stack.source) {
         const source = this._getSourceOriginalLocation(stack);
@@ -697,19 +687,18 @@ const proto = {
     return Promise.all(fulfillmentStacks);
   },
 
   /**
    * Handle a protocol request to get the rejection stack of a promise.
    */
   rejectionStack: function() {
     if (this.obj.class != "Promise") {
-      return { error: "objectNotPromise",
-               message: "'rejectionStack' request is only valid for " +
-                        "object grips with a 'Promise' class." };
+      return this.throwError("objectNotPromise",
+        "'rejectionStack' request is only valid for grips with a 'Promise' class.");
     }
 
     let stack = this.obj.promiseResolutionSite;
     const rejectionStacks = [];
 
     while (stack) {
       if (stack.source) {
         const source = this._getSourceOriginalLocation(stack);
--- a/devtools/shared/protocol.js
+++ b/devtools/shared/protocol.js
@@ -1024,16 +1024,30 @@ Actor.prototype = extend(Pool.prototype,
       message: error.message
     });
   },
 
   _queueResponse: function(create) {
     const pending = this._pendingResponse || Promise.resolve(null);
     const response = create(pending);
     this._pendingResponse = response;
+  },
+
+  /**
+   * Throw an error with the passed message and attach an `error` property to the Error
+   * object so it can be consumed by the writeError function.
+   * @param {String} error: A string (usually a single word serving as an id) that will
+   *                        be assign to error.error.
+   * @param {String} message: The string that will be passed to the Error constructor.
+   * @throws This always throw.
+   */
+  throwError: function(error, message) {
+    const err = new Error(message);
+    err.error = error;
+    throw err;
   }
 });
 exports.Actor = Actor;
 
 /**
  * Tags a prtotype method as an actor method implementation.
  *
  * @param function fn