Bug 1441333: Part 5 - Use proper async caller location in normalizeError. r=zombie
authorKris Maglione <maglione.k@gmail.com>
Thu, 01 Mar 2018 16:39:08 -0800
changeset 461263 cf87433f501d755e6cde616a6ec77a4f37c69a00
parent 461262 47c2e679000ff75affdb8314a7ee83726411806e
child 461264 930b44b7adc7e26e0b76cb46178489fc2e6704dc
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszombie
bugs1441333
milestone60.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 1441333: Part 5 - Use proper async caller location in normalizeError. r=zombie Currently, when we create an error object at the end of an aysnc operation, we only get a useful caller location if async stacks are enabled. This patch changes our behavior to use the saved caller location we've already stored when creating an Error object based on a plain string message. MozReview-Commit-ID: DDO0lAUHYRO
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/MessageChannel.jsm
toolkit/components/extensions/test/xpcshell/head.js
toolkit/components/extensions/test/xpcshell/test_ext_error_location.js
toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -376,17 +376,17 @@ class Messenger {
   sendMessage(messageManager, msg, recipient, responseCallback) {
     let holder = new StructuredCloneHolder(msg);
 
     let promise = this._sendMessage(messageManager, "Extension:Message", holder, recipient)
       .catch(error => {
         if (error.result == MessageChannel.RESULT_NO_HANDLER) {
           return Promise.reject({message: "Could not establish connection. Receiving end does not exist."});
         } else if (error.result != MessageChannel.RESULT_NO_RESPONSE) {
-          return Promise.reject({message: error.message});
+          return Promise.reject(error);
         }
       });
     holder = null;
 
     return this.context.wrapPromise(promise, responseCallback);
   }
 
   sendNativeMessage(messageManager, msg, recipient, responseCallback) {
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -384,33 +384,46 @@ class BaseContext {
    * the target is an error object which belongs to that scope, it is
    * returned as-is. If it is an ordinary object with a `message`
    * property, it is converted into an error belonging to the target
    * scope. If it is an Error object which does *not* belong to the
    * clone scope, it is reported, and converted to an unexpected
    * exception error.
    *
    * @param {Error|object} error
+   * @param {SavedFrame?} [caller]
    * @returns {Error}
    */
-  normalizeError(error) {
+  normalizeError(error, caller) {
     if (error instanceof this.cloneScope.Error) {
       return error;
     }
     let message, fileName;
-    if (error && typeof error === "object" &&
-        (ChromeUtils.getClassName(error) === "Object" ||
-         error instanceof ExtensionError ||
-         this.principal.subsumes(Cu.getObjectPrincipal(error)))) {
-      message = error.message;
-      fileName = error.fileName;
-    } else {
+    if (error && typeof error === "object") {
+      const isPlain = ChromeUtils.getClassName(error) === "Object";
+      if (isPlain && error.mozWebExtLocation) {
+        caller = error.mozWebExtLocation;
+      }
+      if (isPlain && caller && (error.mozWebExtLocation || !error.fileName)) {
+        caller = Cu.cloneInto(caller, this.cloneScope);
+        return ChromeUtils.createError(error.message, caller);
+      }
+
+      if (isPlain ||
+          error instanceof ExtensionError ||
+          this.principal.subsumes(Cu.getObjectPrincipal(error))) {
+        message = error.message;
+        fileName = error.fileName;
+      }
+    }
+
+    if (!message) {
       Cu.reportError(error);
+      message = "An unexpected error occurred";
     }
-    message = message || "An unexpected error occurred";
     return new this.cloneScope.Error(message, fileName);
   }
 
   /**
    * Sets the value of `.lastError` to `error`, calls the given
    * callback, and reports an error if the value has not been checked
    * when the callback returns.
    *
@@ -529,17 +542,17 @@ class BaseContext {
           value => {
             if (this.unloaded) {
               Cu.reportError(`Promise rejected after context unloaded: ${value && value.message}\n`,
                              caller);
             } else if (!this.active) {
               Cu.reportError(`Promise rejected while context is inactive: ${value && value.message}\n`,
                              caller);
             } else {
-              this.applySafeWithoutClone(reject, [this.normalizeError(value)],
+              this.applySafeWithoutClone(reject, [this.normalizeError(value, caller)],
                                          caller);
             }
           });
       });
     }
   }
 
   unload() {
--- a/toolkit/components/extensions/MessageChannel.jsm
+++ b/toolkit/components/extensions/MessageChannel.jsm
@@ -968,17 +968,18 @@ this.MessageChannel = {
 
         if (error && typeof(error) == "object") {
           if (error.result) {
             response.result = error.result;
           }
           // Error objects are not structured-clonable, so just copy
           // over the important properties.
           for (let key of ["fileName", "filename", "lineNumber",
-                           "columnNumber", "message", "stack", "result"]) {
+                           "columnNumber", "message", "stack", "result",
+                           "mozWebExtLocation"]) {
             if (key in error) {
               response.error[key] = error[key];
             }
           }
         }
 
         target.sendAsyncMessage(MESSAGE_RESPONSE, response);
       })
--- a/toolkit/components/extensions/test/xpcshell/head.js
+++ b/toolkit/components/extensions/test/xpcshell/head.js
@@ -80,16 +80,17 @@ var promiseConsoleOutput = async functio
   let listener;
   let messages = [];
   let awaitListener = new Promise(resolve => {
     listener = msg => {
       if (msg == DONE) {
         resolve();
       } else {
         void (msg instanceof Ci.nsIConsoleMessage);
+        void (msg instanceof Ci.nsIScriptError);
         messages.push(msg);
       }
     };
   });
 
   Services.console.registerListener(listener);
   try {
     let result = await task();
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_error_location.js
@@ -0,0 +1,48 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(async function test_error_location() {
+  let extension = ExtensionTestUtils.loadExtension({
+    async background() {
+      let {fileName} = new Error();
+
+      browser.test.sendMessage("fileName", fileName);
+
+      browser.runtime.sendMessage("Meh.", () => {});
+
+      await browser.test.assertRejects(
+        browser.runtime.sendMessage("Meh"),
+        error => {
+          return error.fileName === fileName && error.lineNumber === 9;
+        });
+
+      browser.test.notifyPass("error-location");
+    },
+  });
+
+  let fileName;
+  const {messages} = await promiseConsoleOutput(async () => {
+    await extension.startup();
+
+    fileName = await extension.awaitMessage("fileName");
+
+    await extension.awaitFinish("error-location");
+
+    await extension.unload();
+  });
+
+  let [msg] = messages.filter(m => m.message.includes("Unchecked lastError"));
+
+  equal(msg.sourceName, fileName, "Message source");
+  equal(msg.lineNumber, 6, "Message line");
+
+  let frame = msg.stack;
+  if (frame) {
+    equal(frame.source, fileName, "Frame source");
+    equal(frame.line, 6, "Frame line");
+    equal(frame.column, 7, "Frame column");
+    equal(frame.functionDisplayName, "background", "Frame function name");
+  }
+});
+
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -25,16 +25,17 @@ skip-if = os == "android" # Containers a
 [test_ext_downloads_download.js]
 skip-if = os == "android"
 [test_ext_downloads_misc.js]
 skip-if = os == "android" || (os=='linux' && bits==32) # linux32: bug 1324870
 [test_ext_downloads_private.js]
 skip-if = os == "android"
 [test_ext_downloads_search.js]
 skip-if = os == "android"
+[test_ext_error_location.js]
 [test_ext_experiments.js]
 [test_ext_extension.js]
 [test_ext_extensionPreferencesManager.js]
 [test_ext_extensionSettingsStore.js]
 [test_ext_extension_content_telemetry.js]
 skip-if = os == "android" # checking for telemetry needs to be updated: 1384923
 [test_ext_extension_startup_telemetry.js]
 [test_ext_idle.js]