Bug 1551222 - Fix stacktrace for throwing console evaluated expressions. r=bhackett.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Mon, 13 May 2019 18:42:31 +0000
changeset 532550 867447201d5d56927638aaed1423fb38370ddfc2
parent 532549 092e0808f3f8f921164933e96b42100e4bf4192f
child 532551 b90c24bb6358c8ad4b0b56cd312457e0548c26f3
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1551222
milestone68.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 1551222 - Fix stacktrace for throwing console evaluated expressions. r=bhackett. We now have stacktrace for expressions evaluated in the console that throws, and we are stripping any frames that are devtools internals. But the way we were doing this meant that we were only having one `debugger eval code` frame, even if the expression in the console had multiple frames. Also, if an expression was throwing without having any `debugger eval code` frame (e.g. expression with SyntaxError), we were sending internal frames. This patch should fix those 2 cases and a linux 64 ccov intermittent caused by a different line number for an internal frame in a fixture packet. Tests cases are added to the existing mochitest to ensure this works as expected. Differential Revision: https://phabricator.services.mozilla.com/D30934
devtools/client/webconsole/test/fixtures/stubs/evaluationResult.js
devtools/client/webconsole/test/mochitest/browser_webconsole_eval_error.js
devtools/server/actors/webconsole/utils.js
--- a/devtools/client/webconsole/test/fixtures/stubs/evaluationResult.js
+++ b/devtools/client/webconsole/test/fixtures/stubs/evaluationResult.js
@@ -101,61 +101,18 @@ stubPreparedMessages.set(`1 + @`, new Co
   "level": "error",
   "category": null,
   "messageText": "SyntaxError: illegal character",
   "parameters": [
     {
       "type": "undefined"
     }
   ],
-  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":4},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"SyntaxError: illegal character\",\"parameters\":[{\"type\":\"undefined\"}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null,\"stacktrace\":[{\"filename\":\"resource://devtools/server/actors/webconsole/eval-with-debugger.js\",\"sourceId\":null,\"lineNumber\":134,\"columnNumber\":28,\"functionName\":\"getEvalResult\"},{\"filename\":\"resource://devtools/server/actors/webconsole/eval-with-debugger.js\",\"sourceId\":null,\"lineNumber\":105,\"columnNumber\":18,\"functionName\":\"exports.evalWithDebugger\"},{\"filename\":\"resource://devtools/server/actors/webconsole.js\",\"sourceId\":null,\"lineNumber\":1005,\"columnNumber\":22,\"functionName\":\"evaluateJS\"},{\"filename\":\"self-hosted\",\"sourceId\":null,\"lineNumber\":1005,\"columnNumber\":17,\"functionName\":\"evaluateJS\"},{\"filename\":\"resource://devtools/server/main.js\",\"sourceId\":null,\"lineNumber\":1291,\"columnNumber\":58,\"functionName\":\"onPacket\"},{\"filename\":\"resource://devtools/shared/transport/child-transport.js\",\"sourceId\":null,\"lineNumber\":66,\"columnNumber\":16,\"functionName\":\"receiveMessage\"}]}",
-  "stacktrace": [
-    {
-      "filename": "resource://devtools/server/actors/webconsole/eval-with-debugger.js",
-      "sourceId": null,
-      "lineNumber": 134,
-      "columnNumber": 28,
-      "functionName": "getEvalResult"
-    },
-    {
-      "filename": "resource://devtools/server/actors/webconsole/eval-with-debugger.js",
-      "sourceId": null,
-      "lineNumber": 105,
-      "columnNumber": 18,
-      "functionName": "exports.evalWithDebugger"
-    },
-    {
-      "filename": "resource://devtools/server/actors/webconsole.js",
-      "sourceId": null,
-      "lineNumber": 1005,
-      "columnNumber": 22,
-      "functionName": "evaluateJS"
-    },
-    {
-      "filename": "self-hosted",
-      "sourceId": null,
-      "lineNumber": 1005,
-      "columnNumber": 17,
-      "functionName": "evaluateJS"
-    },
-    {
-      "filename": "resource://devtools/server/main.js",
-      "sourceId": null,
-      "lineNumber": 1291,
-      "columnNumber": 58,
-      "functionName": "onPacket"
-    },
-    {
-      "filename": "resource://devtools/shared/transport/child-transport.js",
-      "sourceId": null,
-      "lineNumber": 66,
-      "columnNumber": 16,
-      "functionName": "receiveMessage"
-    }
-  ],
+  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":4},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"SyntaxError: illegal character\",\"parameters\":[{\"type\":\"undefined\"}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null,\"stacktrace\":null}",
+  "stacktrace": null,
   "frame": {
     "source": "debugger eval code",
     "line": 1,
     "column": 4
   },
   "groupId": null,
   "errorMessageName": "JSMSG_ILLEGAL_CHARACTER",
   "exceptionDocURL": "https://developer.mozilla.org/docs/Web/JavaScript/Reference/Errors/Illegal_character?utm_source=mozilla&utm_medium=firefox-console-errors&utm_campaign=default",
@@ -485,60 +442,17 @@ stubPackets.set(`1 + @`, {
       "stack": "",
       "fileName": "debugger eval code",
       "lineNumber": 1,
       "columnNumber": 4
     }
   },
   "exceptionMessage": "SyntaxError: illegal character",
   "exceptionDocURL": "https://developer.mozilla.org/docs/Web/JavaScript/Reference/Errors/Illegal_character?utm_source=mozilla&utm_medium=firefox-console-errors&utm_campaign=default",
-  "exceptionStack": [
-    {
-      "filename": "resource://devtools/server/actors/webconsole/eval-with-debugger.js",
-      "sourceId": null,
-      "lineNumber": 134,
-      "columnNumber": 28,
-      "functionName": "getEvalResult"
-    },
-    {
-      "filename": "resource://devtools/server/actors/webconsole/eval-with-debugger.js",
-      "sourceId": null,
-      "lineNumber": 105,
-      "columnNumber": 18,
-      "functionName": "exports.evalWithDebugger"
-    },
-    {
-      "filename": "resource://devtools/server/actors/webconsole.js",
-      "sourceId": null,
-      "lineNumber": 1005,
-      "columnNumber": 22,
-      "functionName": "evaluateJS"
-    },
-    {
-      "filename": "self-hosted",
-      "sourceId": null,
-      "lineNumber": 1005,
-      "columnNumber": 17,
-      "functionName": "evaluateJS"
-    },
-    {
-      "filename": "resource://devtools/server/main.js",
-      "sourceId": null,
-      "lineNumber": 1291,
-      "columnNumber": 58,
-      "functionName": "onPacket"
-    },
-    {
-      "filename": "resource://devtools/shared/transport/child-transport.js",
-      "sourceId": null,
-      "lineNumber": 66,
-      "columnNumber": 16,
-      "functionName": "receiveMessage"
-    }
-  ],
+  "exceptionStack": null,
   "errorMessageName": "JSMSG_ILLEGAL_CHARACTER",
   "frame": {
     "source": "debugger eval code",
     "line": 1,
     "column": 4
   },
   "helperResult": null,
   "notes": null
--- a/devtools/client/webconsole/test/mochitest/browser_webconsole_eval_error.js
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_eval_error.js
@@ -14,9 +14,23 @@ const TEST_URI = "http://example.com/bro
 add_task(async function() {
   const hud = await openNewTabAndConsole(TEST_URI);
 
   hud.jsterm.execute("throwErrorObject()");
   await checkMessageStack(hud, "ThrowErrorObject", [6, 1]);
 
   hud.jsterm.execute("throwValue(40 + 2)");
   await checkMessageStack(hud, "42", [14, 10, 1]);
+
+  hud.jsterm.execute(`
+    a = () => {throw "bloop"};
+    b =  () => a();
+    c =  () => b();
+    d =  () => c();
+    d();
+  `);
+  await checkMessageStack(hud, "Error: bloop", [2, 3, 4, 5, 6]);
+
+  hud.jsterm.execute(`1 + @`);
+  const messageNode = await waitFor(() => findMessage(hud, "illegal character"));
+  is(messageNode.querySelector(".frames"), null,
+    "There's no stacktrace for a SyntaxError evaluation");
 });
--- a/devtools/server/actors/webconsole/utils.js
+++ b/devtools/server/actors/webconsole/utils.js
@@ -202,23 +202,37 @@ var WebConsoleUtils = {
    * @param array stack
    *        An array of frames, with the topmost first, and each of which has a
    *        'filename' property.
    * @return array
    *         An array of stack frames with any devtools server frames removed.
    *         The original array is not modified.
    */
   removeFramesAboveDebuggerEval(stack) {
-    // Remove any frames for server code above the debugger eval.
-    const evalIndex = stack.findIndex(({ filename }) => {
-      return filename == "debugger eval code";
+    const debuggerEvalFilename = "debugger eval code";
+
+    // Remove any frames for server code above the last debugger eval frame.
+    const evalIndex = stack.findIndex(({ filename }, idx, arr) => {
+      const nextFrame = arr[idx + 1];
+      return filename == debuggerEvalFilename
+        && (!nextFrame || nextFrame.filename !== debuggerEvalFilename);
     });
     if (evalIndex != -1) {
       return stack.slice(0, evalIndex + 1);
     }
+
+    // In some cases (e.g. evaluated expression with SyntaxError), we might not have a
+    // "debugger eval code" frame but still have internal ones. If that's the case, we
+    // return null as the end user shouldn't see those frames.
+    if (stack.some(({ filename }) =>
+      filename && filename.startsWith("resource://devtools/"))
+    ) {
+      return null;
+    }
+
     return stack;
   },
 };
 
 exports.WebConsoleUtils = WebConsoleUtils;
 
 /**
  * WebConsole commands manager.