Bug 1284232 - Guard against ill-behaved objects on error check; r=automatedtester
authorAndreas Tolfsen <ato@mozilla.com>
Fri, 08 Jul 2016 13:58:04 +0100
changeset 305445 bb0f0424f0f0c367c8878f2116463499c20b9fed
parent 305444 7b175334ebc6fb4b898a35331d9fa04ed55ea3c7
child 305446 53d2d214a699268aae7abb9fae5f204db5fa2fa5
push id30463
push usercbook@mozilla.com
push dateTue, 19 Jul 2016 14:02:45 +0000
treeherdermozilla-central@37cc0da01187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1284232
milestone50.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 1284232 - Guard against ill-behaved objects on error check; r=automatedtester It turns out that certain objects such as a DOMRectList have peculiarities that cause string comparison to throw. This is normally not a problem as error.isError is usually passed JSON serialisable data. But when it is not, this try condition helps diagnose problems. MozReview-Commit-ID: BLNSziwfxXs
testing/marionette/error.js
testing/marionette/test_error.js
--- a/testing/marionette/error.js
+++ b/testing/marionette/error.js
@@ -24,31 +24,31 @@ const ERRORS = [
   "TimeoutError",
   "UnableToSetCookieError",
   "UnknownCommandError",
   "UnknownError",
   "UnsupportedOperationError",
   "WebDriverError",
 ];
 
+const BUILTIN_ERRORS = new Set([
+  "Error",
+  "EvalError",
+  "InternalError",
+  "RangeError",
+  "ReferenceError",
+  "SyntaxError",
+  "TypeError",
+  "URIError",
+]);
+
 this.EXPORTED_SYMBOLS = ["error"].concat(ERRORS);
 
 this.error = {};
 
-error.BuiltinErrors = {
-  Error: 0,
-  EvalError: 1,
-  InternalError: 2,
-  RangeError: 3,
-  ReferenceError: 4,
-  SyntaxError: 5,
-  TypeError: 6,
-  URIError: 7,
-};
-
 /**
  * Checks if obj is an instance of the Error prototype in a safe manner.
  * Prefer using this over using instanceof since the Error prototype
  * isn't unique across browsers, and XPCOM nsIException's are special
  * snowflakes.
  *
  * @param {*} val
  *     Any value that should be undergo the test for errorness.
@@ -56,17 +56,23 @@ error.BuiltinErrors = {
  *     True if error, false otherwise.
  */
 error.isError = function(val) {
   if (val === null || typeof val != "object") {
     return false;
   } else if (val instanceof Ci.nsIException) {
     return true;
   } else {
-    return Object.getPrototypeOf(val) in error.BuiltinErrors;
+    // DOMRectList errors on string comparison
+   try {
+      let proto = Object.getPrototypeOf(val);
+      return BUILTIN_ERRORS.has(proto.toString());
+    } catch (e) {
+      return false;
+    }
   }
 };
 
 /**
  * Checks if obj is an object in the WebDriverError prototypal chain.
  */
 error.isWebDriverError = function(obj) {
   return error.isError(obj) &&
--- a/testing/marionette/test_error.js
+++ b/testing/marionette/test_error.js
@@ -5,29 +5,16 @@
 const {utils: Cu} = Components;
 
 Cu.import("chrome://marionette/content/error.js");
 
 function notok(condition) {
   ok(!(condition));
 }
 
-add_test(function test_BuiltinErrors() {
-  ok("Error" in error.BuiltinErrors);
-  ok("EvalError" in error.BuiltinErrors);
-  ok("InternalError" in error.BuiltinErrors);
-  ok("RangeError" in error.BuiltinErrors);
-  ok("ReferenceError" in error.BuiltinErrors);
-  ok("SyntaxError" in error.BuiltinErrors);
-  ok("TypeError" in error.BuiltinErrors);
-  ok("URIError" in error.BuiltinErrors);
-
-  run_next_test();
-});
-
 add_test(function test_isError() {
   notok(error.isError(null));
   notok(error.isError([]));
   notok(error.isError(new Date()));
 
   ok(error.isError(new Components.Exception()));
   ok(error.isError(new Error()));
   ok(error.isError(new EvalError()));