Bug 906795 - safeErrorString should include a stack; r=vporof

toolkit/devtools/DevToolsUtils.js

@@ -6,12 +6,26 @@
 
 /* General utilities used throughout devtools. */
 
-/* Turn the error e into a string, without fail. */
+/**
+ * Turn the error |aError| into a string, without fail.
+ */
 this.safeErrorString = function safeErrorString(aError) {
   try {
-    var s = aError.toString();
+    let errorString = aError.toString();
-    if (typeof s === "string")
+    if (typeof errorString === "string") {
-      return s;
+      // Attempt to attach a stack to |errorString|. If it throws an error, or
+      // isn't a string, don't use it.
+      try {
+        if (aError.stack) {
+          let stack = aError.stack.toString();
+          if (typeof stack === "string") {
+            errorString += "\nStack: " + stack;
+          }
+        }
+      } catch (ee) { }
+
+      return errorString;
+    }
   } catch (ee) { }
 
   return "<failed trying to find error description>";
@@ -22,9 +36,6 @@ this.safeErrorString = function safeErrorString(aError) {
  */
 this.reportException = function reportException(aWho, aException) {
   let msg = aWho + " threw an exception: " + safeErrorString(aException);
-  if (aException.stack) {
-    msg += "\nCall stack:\n" + aException.stack;
-  }
 
   dump(msg + "\n");
 

toolkit/devtools/moz.build

@@ -4,6 +4,8 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+TEST_DIRS += ['tests']
+
 PARALLEL_DIRS += [
     'server',
     'client',

toolkit/devtools/server/main.js

@@ -835,7 +835,6 @@ DebuggerServerConnection.prototype = {
 
   _unknownError: function DSC__unknownError(aPrefix, aError) {
     let errorString = safeErrorString(aError);
-    errorString += "\n" + aError.stack;
     Cu.reportError(errorString);
     dumpn(errorString);
     return {

toolkit/devtools/tests/moz.build

@@ -0,0 +1,9 @@
+# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+MODULE = 'test_devtools'
+
+XPCSHELL_TESTS_MANIFESTS += ['unit/xpcshell.ini']

toolkit/devtools/tests/unit/head_devtools.js

@@ -0,0 +1,42 @@
+"use strict";
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm");
+
+// Register a console listener, so console messages don't just disappear
+// into the ether.
+let errorCount = 0;
+let listener = {
+  observe: function (aMessage) {
+    errorCount++;
+    try {
+      // If we've been given an nsIScriptError, then we can print out
+      // something nicely formatted, for tools like Emacs to pick up.
+      var scriptError = aMessage.QueryInterface(Ci.nsIScriptError);
+      dump(aMessage.sourceName + ":" + aMessage.lineNumber + ": " +
+           scriptErrorFlagsToKind(aMessage.flags) + ": " +
+           aMessage.errorMessage + "\n");
+      var string = aMessage.errorMessage;
+    } catch (x) {
+      // Be a little paranoid with message, as the whole goal here is to lose
+      // no information.
+      try {
+        var string = "" + aMessage.message;
+      } catch (x) {
+        var string = "<error converting error message to string>";
+      }
+    }
+
+    // Make sure we exit all nested event loops so that the test can finish.
+    while (DebuggerServer.xpcInspector.eventLoopNestLevel > 0) {
+      DebuggerServer.xpcInspector.exitNestedEventLoop();
+    }
+    do_throw("head_dbg.js got console message: " + string + "\n");
+  }
+};
+
+let consoleService = Cc["@mozilla.org/consoleservice;1"].getService(Ci.nsIConsoleService);
+consoleService.registerListener(listener);

toolkit/devtools/tests/unit/test_safeErrorString.js

@@ -0,0 +1,54 @@
+/* -*- Mode: js; js-indent-level: 2; -*- */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test DevToolsUtils.safeErrorString
+
+function run_test() {
+  test_with_error();
+  test_with_tricky_error();
+  test_with_string();
+  test_with_thrower();
+  test_with_psychotic();
+}
+
+function test_with_error() {
+  let s = DevToolsUtils.safeErrorString(new Error("foo bar"));
+  // Got the message.
+  do_check_true(s.contains("foo bar"));
+  // Got the stack.
+  do_check_true(s.contains("test_with_error"))
+  do_check_true(s.contains("test_safeErrorString.js"));
+}
+
+function test_with_tricky_error() {
+  let e = new Error("batman");
+  e.stack = { toString: Object.create(null) };
+  let s = DevToolsUtils.safeErrorString(e);
+  // Still got the message, despite a bad stack property.
+  do_check_true(s.contains("batman"));
+}
+
+function test_with_string() {
+  let s = DevToolsUtils.safeErrorString("not really an error");
+  // Still get the message.
+  do_check_true(s.contains("not really an error"));
+}
+
+function test_with_thrower() {
+  let s = DevToolsUtils.safeErrorString({
+    toString: () => {
+      throw new Error("Muahahaha");
+    }
+  });
+  // Still don't fail, get string back.
+  do_check_eq(typeof s, "string");
+}
+
+function test_with_psychotic() {
+  let s = DevToolsUtils.safeErrorString({
+    toString: () => Object.create(null)
+  });
+  // Still get a string out, and no exceptions thrown
+  do_check_eq(typeof s, "string");
+}

toolkit/devtools/tests/unit/xpcshell.ini

@@ -0,0 +1,5 @@
+[DEFAULT]
+head = head_devtools.js
+tail =
+
+[test_safeErrorString.js]