Bug 1452483 - Preserve stacktrace from sandbox evaluation. r?maja_zf
Now that the correct filename and line number is being passed to
Cu.evalInSandbox, the stacktrace of the thrown error is correct.
JavaScriptError uses the line number to index the injected source
script, but the line number refers to the file represented by the
"filename" parameter and not to the script.
This effectively means that the line numbers in the produced
stacktrace are wrong because line number 0 was hard-coded as an
argument to Cu.evalInSandbox.
This patch harmonises the stacktraces returned from
WebDriver:ExecuteScript and WebDriver:ExecuteAsyncScript with
stacktraces from normal JavaScript errors, by removing some extra clutter.
MozReview-Commit-ID: 9nm6HeA4YVJ
--- a/testing/marionette/error.js
+++ b/testing/marionette/error.js
@@ -344,67 +344,21 @@ class InvalidSelectorError extends WebDr
*/
class InvalidSessionIDError extends WebDriverError {
constructor(message) {
super(message);
this.status = "invalid session id";
}
}
-/**
- * An error occurred while executing JavaScript supplied by the user.
- *
- * Creates a richly annotated error for an error situation that occurred
- * whilst evaluating injected scripts.
- */
+/** An error occurred whilst executing JavaScript supplied by the user. */
class JavaScriptError extends WebDriverError {
- /**
- * @param {(string|Error)} x
- * An Error object instance or a string describing the error
- * situation.
- * @param {string=} fnName
- * Name of the function to use in the stack trace message.
- * @param {string=} file
- * Filename of the test file on the client.
- * @param {number=} line
- * Line number of |file|.
- * @param {string=} script
- * Script being executed, in text form.
- */
- constructor(x,
- {fnName = null, file = null, line = null, script = null} = {}) {
- let msg = String(x);
- let trace = "";
-
- if (fnName !== null) {
- trace += fnName;
- if (file !== null) {
- trace += ` @${file}`;
- if (line !== null) {
- trace += `, line ${line}`;
- }
- }
- }
-
- if (error.isError(x)) {
- let jsStack = x.stack.split("\n");
- let match = jsStack[0].match(/:(\d+):\d+$/);
- let jsLine = match ? parseInt(match[1]) : 0;
- if (script !== null) {
- let src = script.split("\n")[jsLine];
- trace += "\n" +
- `inline javascript, line ${jsLine}\n` +
- `src: "${src}"`;
- }
- trace += "\nStack:\n" + x.stack;
- }
-
- super(msg);
+ constructor(x) {
+ super(x);
this.status = "javascript error";
- this.stack = trace;
}
}
/**
* The target for mouse interaction is not in the browser's viewport
* and cannot be brought into that viewport.
*/
class MoveTargetOutOfBoundsError extends WebDriverError {
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -157,23 +157,17 @@ evaluate.sandbox = function(sb, script,
// timeout and unload handlers
scriptTimeoutID = setTimeout(timeoutHandler, timeout);
sb.window.onunload = unloadHandler;
let res;
try {
res = Cu.evalInSandbox(src, sb, "1.8", file, line);
} catch (e) {
- let err = new JavaScriptError(e, {
- fnName: "execute_script",
- file,
- line,
- script,
- });
- reject(err);
+ reject(new JavaScriptError(e));
}
if (!async) {
resolve(res);
}
});
return promise.then(res => {
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py
@@ -1,10 +1,12 @@
from __future__ import absolute_import
+import os
+
from marionette_driver.errors import (
JavascriptException,
ScriptTimeoutException,
)
from marionette_harness import MarionetteTestCase
@@ -56,26 +58,31 @@ class TestExecuteAsyncContent(Marionette
self.assertEqual(self.marionette.execute_async_script("marionetteScriptFinished()"), None)
def test_execute_js_exception(self):
try:
self.marionette.execute_async_script("""
let a = 1;
foo(bar);
""")
- self.assertFalse(True)
- except JavascriptException as inst:
- self.assertTrue('foo(bar)' in inst.stacktrace)
+ self.fail()
+ except JavascriptException as e:
+ self.assertIsNotNone(e.stacktrace)
+ self.assertIn(os.path.relpath(__file__.replace(".pyc", ".py")), e.stacktrace)
def test_execute_async_js_exception(self):
- self.assertRaises(JavascriptException,
- self.marionette.execute_async_script, """
- var callback = arguments[arguments.length - 1];
- callback(foo());
+ try:
+ self.marionette.execute_async_script("""
+ let [resolve] = arguments;
+ resolve(foo());
""")
+ self.fail()
+ except JavascriptException as e:
+ self.assertIsNotNone(e.stacktrace)
+ self.assertIn(os.path.relpath(__file__.replace(".pyc", ".py")), e.stacktrace)
def test_script_finished(self):
self.assertTrue(self.marionette.execute_async_script("""
marionetteScriptFinished(true);
"""))
def test_execute_permission(self):
self.assertRaises(JavascriptException, self.marionette.execute_async_script, """
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
@@ -158,20 +158,18 @@ class TestExecuteContent(MarionetteTestC
self.assertRaises(errors.JavascriptException,
self.marionette.execute_script, "return foo")
def test_stacktrace(self):
with self.assertRaises(errors.JavascriptException) as cm:
self.marionette.execute_script("return b")
# by default execute_script pass the name of the python file
- self.assertIn(os.path.basename(__file__.replace(".pyc", ".py")),
- cm.exception.stacktrace)
+ self.assertIn(os.path.relpath(__file__.replace(".pyc", ".py")), cm.exception.stacktrace)
self.assertIn("b is not defined", cm.exception.message)
- self.assertIn("return b", cm.exception.stacktrace)
def test_permission(self):
for sandbox in ["default", None]:
with self.assertRaises(errors.JavascriptException):
self.marionette.execute_script(
"Components.classes['@mozilla.org/preferences-service;1']")
def test_return_web_element(self):
--- a/testing/marionette/test_error.js
+++ b/testing/marionette/test_error.js
@@ -130,27 +130,20 @@ add_test(function test_toJSON() {
equal(e0s.message, "");
equal(e0s.stacktrace, e0.stack);
let e1 = new WebDriverError("a");
let e1s = e1.toJSON();
equal(e1s.message, e1.message);
equal(e1s.stacktrace, e1.stack);
- let e2 = new JavaScriptError("first", {
- fnName: "second",
- file: "third",
- line: "fourth",
- });
+ let e2 = new JavaScriptError("foo");
let e2s = e2.toJSON();
equal(e2.status, e2s.error);
equal(e2.message, e2s.message);
- ok(e2s.stacktrace.match(/second/));
- ok(e2s.stacktrace.match(/third/));
- ok(e2s.stacktrace.match(/fourth/));
run_next_test();
});
add_test(function test_fromJSON() {
Assert.throws(() => WebDriverError.fromJSON({error: "foo"}),
/Not of WebDriverError descent/);
Assert.throws(() => WebDriverError.fromJSON({error: "Error"}),
@@ -183,19 +176,21 @@ add_test(function test_fromJSON() {
ok(e3r instanceof WebDriverError);
ok(e3r instanceof NoSuchElementError);
equal(e3r.name, "NoSuchElementError");
equal(e3r.status, e3j.error);
equal(e3r.message, e3j.message);
equal(e3r.stack, e3j.stacktrace);
// parity with toJSON
- let e4 = new JavaScriptError("first", "second", "third", "fourth");
- let e4s = e4.toJSON();
- deepEqual(e4, WebDriverError.fromJSON(e4s));
+ let e4j = new JavaScriptError("foo").toJSON();
+ let e4 = WebDriverError.fromJSON(e4j);
+ equal(e4j.error, e4.status);
+ equal(e4j.message, e4.message);
+ equal(e4j.stacktrace, e4.stack);
run_next_test();
});
add_test(function test_WebDriverError() {
let err = new WebDriverError("foo");
equal("WebDriverError", err.name);
equal("foo", err.message);
@@ -317,23 +312,22 @@ add_test(function test_InvalidSessionIDE
add_test(function test_JavaScriptError() {
let err = new JavaScriptError("foo");
equal("JavaScriptError", err.name);
equal("foo", err.message);
equal("javascript error", err.status);
ok(err instanceof WebDriverError);
- equal("undefined", new JavaScriptError(undefined).message);
- // TODO(ato): Bug 1240550
- //equal("funcname @file", new JavaScriptError("message", {fnName: "funcname", file: "file"}).stack);
- equal("funcname @file, line line",
- new JavaScriptError("message", {fnName: "funcname", file: "file", line: "line"}).stack);
+ equal("", new JavaScriptError(undefined).message);
- // TODO(ato): More exhaustive tests for JS stack computation
+ let superErr = new RangeError("foo");
+ let inheritedErr = new JavaScriptError(superErr);
+ equal("RangeError: foo", inheritedErr.message);
+ equal(superErr.stack, inheritedErr.stack);
run_next_test();
});
add_test(function test_NoSuchAlertError() {
let err = new NoSuchAlertError("foo");
equal("NoSuchAlertError", err.name);
equal("foo", err.message);