Bug 1536854 - Log errors from logpoint as actual errors in the console r=davidwalsh,nchevobbe

If logpoint throws, set `level` to "error".
Add tests to ensure the correct level is set.

Demo function: https://luxuriant-system.glitch.me/
Add some logpoints. The first two statements are invalid. The third one is valid.
{F1311386}

In console:
{F1311387}

Differential Revision: https://phabricator.services.mozilla.com/D31157

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/test/mochitest/browser_dbg-log-points.js

@@ -31,6 +31,6 @@ add_task(async function() {
   await hasConsoleMessage(dbg, "firstCall");
 
   const { link, value } = await findConsoleMessage(dbg, "a b c");
-  is(link, "script-switching-01.js:8:2", "logs should have the relevant link");
+  is(link, "Logpoint @ script-switching-01.js:8:2", "logs should have the relevant link");
   is(value, "a b c", "logs should have multiple values");
 });

devtools/client/shared/components/Frame.js

@@ -180,6 +180,22 @@ class Frame extends Component {
       }
     }
 
+    // If the message comes from a logPoint or conditional breakpoint,
+    // prefix the source location accordingly
+    if (frame.origin) {
+      let locationPrefix;
+      if (frame.origin === "logPoint") {
+        locationPrefix = "Logpoint @ ";
+      }
+
+      if (locationPrefix) {
+        sourceElements.push(dom.span({
+          key: "locationPrefix",
+          className: "frame-link-prefix",
+        }, locationPrefix));
+      }
+    }
+
     let displaySource = showFullSourceUrl ? unicodeLong : unicodeShort;
     if (isSourceMapped) {
       displaySource = getSourceMappedFile(displaySource);

devtools/client/shared/components/test/mochitest/head.js

@@ -213,13 +213,14 @@ var TEST_TREE = {
  * Frame
  */
 function checkFrameString({
-  el, file, line, column, source, functionName, shouldLink, tooltip,
+  el, file, line, column, source, functionName, shouldLink, tooltip, locationPrefix,
 }) {
   const $ = selector => el.querySelector(selector);
 
   const $func = $(".frame-link-function-display-name");
   const $source = $(".frame-link-source");
   const $sourceInner = $(".frame-link-source-inner");
+  const $locationPrefix = $(".frame-link-prefix");
   const $filename = $(".frame-link-filename");
   const $line = $(".frame-link-line");
 
@@ -249,6 +250,12 @@ function checkFrameString({
   } else {
     ok(!$func, "Should not have an element for `functionName`");
   }
+
+  if (locationPrefix != null) {
+    is($locationPrefix.textContent, locationPrefix, "Correct prefix");
+  } else {
+    ok(!$locationPrefix, "Should not have an element for `locationPrefix`");
+  }
 }
 
 function checkSmartFrameString({ el, location, functionName, tooltip }) {

devtools/client/shared/components/test/mochitest/test_frame_01.html

@@ -314,6 +314,24 @@ window.onload = async function () {
       source: "https://bugzilla.mozilla.org/original.js",
     });
 
+    // Check when a message comes from a logPoint,
+    // a prefix should render before source
+    await checkFrameComponent({
+      frame: {
+        origin: "logPoint",
+        source: "http://myfile.com/mahscripts.js",
+        line: 55,
+        column: 10,
+      }
+    }, {
+      locationPrefix: "Logpoint @ ",
+      file: "mahscripts.js",
+      line: 55,
+      column: 10,
+      shouldLink: true,
+      tooltip: "View source in Debugger → http://myfile.com/mahscripts.js:55:10",
+    });
+
     function checkFrameComponent(input, expected) {
       let props = Object.assign({ onClick: () => {} }, input);
       let frame = ReactDOM.render(Frame(props), window.document.body);

devtools/client/webconsole/utils/messages.js

@@ -238,6 +238,10 @@ function transformConsoleAPICallPacket(packet) {
     column: message.columnNumber,
   } : null;
 
+  if (type === "logPointError" || type === "logPoint") {
+    frame.origin = "logPoint";
+  }
+
   return new ConsoleMessage({
     source: MESSAGE_SOURCE.CONSOLE_API,
     type,
@@ -457,6 +461,7 @@ function getLevelFromType(type) {
     error: levels.LEVEL_ERROR,
     exception: levels.LEVEL_ERROR,
     assert: levels.LEVEL_ERROR,
+    logPointError: levels.LEVEL_ERROR,
     warn: levels.LEVEL_WARNING,
     info: levels.LEVEL_INFO,
     log: levels.LEVEL_LOG,

devtools/server/actors/breakpoint.js

@@ -228,13 +228,16 @@ BreakpointActor.prototype = {
       const displayName = formatDisplayName(frame);
       const completion = frame.evalWithBindings(`[${logValue}]`, { displayName });
       let value;
+      let level = "logPoint";
+
       if (!completion) {
         // The evaluation was killed (possibly by the slow script dialog).
         value = ["Log value evaluation incomplete"];
       } else if ("return" in completion) {
         value = completion.return;
       } else {
-        value = [this.getThrownMessage(completion)];
+        value = ["[Logpoint threw]: " + this.getThrownMessage(completion)];
+        level = "logPointError";
       }
 
       if (value && typeof value.unsafeDereference === "function") {
@@ -245,8 +248,8 @@ BreakpointActor.prototype = {
         filename: sourceActor.url,
         lineNumber: line,
         columnNumber: column,
-        level: "logPoint",
         arguments: value,
+        level,
       };
       this.threadActor._parent._consoleActor.onConsoleAPICall(message);
 

devtools/server/tests/unit/test_logpoint-01.js

@@ -63,6 +63,7 @@ function test_simple_breakpoint() {
                    1);
   /* eslint-enable */
 
+  Assert.equal(lastMessage.level, "logPoint");
   Assert.equal(lastMessage.arguments[0], "three");
   finishClient(gClient);
 }

devtools/server/tests/unit/test_logpoint-03.js

@@ -0,0 +1,69 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable no-shadow, max-nested-callbacks */
+
+"use strict";
+
+/**
+ * Check that logpoints generate console errors if the logpoint statement is invalid.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+function run_test() {
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-logpoint");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect().then(function() {
+    attachTestTabAndResume(gClient, "test-logpoint",
+                           function(response, targetFront, threadClient) {
+                             gThreadClient = threadClient;
+                             test_simple_breakpoint();
+                           });
+  });
+  do_test_pending();
+}
+
+function test_simple_breakpoint() {
+  const rootActor = gClient.transport._serverConnection.rootActor;
+  const threadActor = rootActor._parameters.tabList._targetActors[0].threadActor;
+
+  let lastMessage;
+  threadActor._parent._consoleActor = {
+    onConsoleAPICall(message) {
+      lastMessage = message;
+    },
+  };
+
+  gThreadClient.once("paused", async function(packet) {
+    const source = await getSourceById(
+      gThreadClient,
+      packet.frame.where.actor
+    );
+
+    // Set a logpoint which should throw an error message.
+    gThreadClient.setBreakpoint({
+      sourceUrl: source.url,
+      line: 3,
+    }, { logValue: "c" });
+
+    // Execute the rest of the code.
+    gThreadClient.resume();
+  });
+
+  /* eslint-disable */
+  Cu.evalInSandbox("debugger;\n" + // 1
+                   "var a = 'three';\n" +  // 2
+                   "var b = 2;\n", // 3
+                   gDebuggee,
+                   "1.8",
+                   "test.js",
+                   1);
+  /* eslint-enable */
+
+  Assert.equal(lastMessage.level, "logPointError");
+  Assert.equal(lastMessage.arguments[0], "[Logpoint threw]: c is not defined");
+  finishClient(gClient);
+}

devtools/server/tests/unit/xpcshell.ini

@@ -139,6 +139,7 @@ skip-if = true # breakpoint sliding is not supported bug 1525685
 [test_conditional_breakpoint-03.js]
 [test_logpoint-01.js]
 [test_logpoint-02.js]
+[test_logpoint-03.js]
 [test_listsources-01.js]
 [test_listsources-02.js]
 [test_listsources-03.js]