Bug 1834725 - [devtools] Log source map errors to the console directly from SourceMapLoader. r=devtools-reviewers,nchevobbe

The main goal here is to re-throw the exception so that the debugger can start displaying source map errors.

Note that the previous changeset tweaked the StyleEditor code to handle exceptions.
The Debugger will be handled in the next changeset.

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

devtools/client/framework/source-map-url-service.js

@@ -319,7 +319,13 @@ class SourceMapURLService {
         let result = null;
         try {
           await map.loaded;
+        } catch (e) {
+          // SourceMapLoader.getOriginalURLs may throw, but it will handle
+          // the exception and notify the user via a console message.
+          // So ignore the exception here, which is meant to be used by the Debugger.
+        }
 
+        try {
           const position = await this._sourceMapLoader.getOriginalLocation({
             sourceId: map.id,
             line: query.line,

devtools/client/framework/toolbox.js

@@ -1409,10 +1409,7 @@ Toolbox.prototype = {
     if (this._sourceMapLoader) {
       return this._sourceMapLoader;
     }
-    this._sourceMapLoader = new SourceMapLoader();
-    this._sourceMapLoader.on("source-map-error", message =>
-      this.target.logWarningInPage(message, "source map")
-    );
+    this._sourceMapLoader = new SourceMapLoader(this.commands.targetCommand);
     return this._sourceMapLoader;
   },
 
@@ -4082,9 +4079,7 @@ Toolbox.prototype = {
     this._pausedTargets = null;
 
     if (this._sourceMapLoader) {
-      this._sourceMapLoader.stopSourceMapWorker();
-      // Unregister all listeners
-      this._sourceMapLoader.clearEvents();
+      this._sourceMapLoader.destroy();
       this._sourceMapLoader = null;
     }
 

devtools/client/shared/source-map-loader/index.js

@@ -25,31 +25,36 @@ const {
 } = require("resource://devtools/client/shared/source-map-loader/utils/index.js");
 
 class SourceMapLoader extends WorkerDispatcher {
+  constructor(targetCommand) {
+    super(SOURCE_MAP_WORKER_URL);
+    this.#targetCommand = targetCommand;
+  }
+
   #setSourceMapForGeneratedSources = this.task(
     "setSourceMapForGeneratedSources"
   );
   #getOriginalURLs = this.task("getOriginalURLs");
   #getOriginalSourceText = this.task("getOriginalSourceText");
-
-  constructor() {
-    super(SOURCE_MAP_WORKER_URL);
-  }
+  #targetCommand = null;
 
   async getOriginalURLs(urlInfo) {
     try {
       return await this.#getOriginalURLs(urlInfo);
     } catch (error) {
-      const message = L10N.getFormatStr(
-        "toolbox.sourceMapFailure",
-        error,
-        urlInfo.url,
-        urlInfo.sourceMapURL
-      );
-      this.emit("source-map-error", message);
+      // Note that tests may not pass the targetCommand
+      if (this.#targetCommand) {
+        // Catch all errors and log them to the Web Console for users to see.
+        const message = L10N.getFormatStr(
+          "toolbox.sourceMapFailure",
+          error,
+          urlInfo.url,
+          urlInfo.sourceMapURL
+        );
+        this.#targetCommand.targetFront.logWarningInPage(message, "source map");
+      }
 
-      // It's ok to swallow errors here, because a null
-      // result just means that no source map was found.
-      return null;
+      // And re-throw the error so that the debugger can also interpret the error
+      throw error;
     }
   }
 
@@ -80,7 +85,12 @@ class SourceMapLoader extends WorkerDispatcher {
         error.message,
         error.metadata ? error.metadata.url : "<unknown>"
       );
-      this.emit("source-map-error", message);
+
+      // Note that tests may not pass the targetCommand
+      if (this.#targetCommand) {
+        // Catch all errors and log them to the Web Console for users to see.
+        this.#targetCommand.targetFront.logWarningInPage(message, "source map");
+      }
 
       // Also replace the result with the error text.
       // Note that this result has to have the same form
@@ -109,7 +119,14 @@ class SourceMapLoader extends WorkerDispatcher {
     return rv;
   }
 
-  stopSourceMapWorker = this.stop.bind(this);
+  destroy() {
+    // Request to stop the underlyin DOM Worker
+    this.stop();
+    // Unregister all listeners
+    this.clearEvents();
+    // SourceMapLoader may be leaked and so it is important to clear most outer references
+    this.#targetCommand = null;
+  }
 }
 EventEmitter.decorate(SourceMapLoader.prototype);
 

devtools/client/shared/source-map-loader/test/browser/browser_source-map.js

@@ -154,16 +154,10 @@ add_task(async function testBaseURLErrorHandling() {
     sourceMapBaseURL: "http:://example.com/",
   };
 
-  const onError = gSourceMapLoader.once("source-map-error");
-  is(
-    await gSourceMapLoader.getOriginalURLs(source),
-    null,
-    "The error is silented..."
-  );
-  info("Wait for source-map-error event");
-  const error = await onError;
-  is(
-    error,
-    `Source map error: Error: URL constructor: http:://example.com/ is not a valid URL.\nResource URL: undefined\nSource Map URL: missingmap.js.map`
-  );
+  try {
+    await gSourceMapLoader.getOriginalURLs(source);
+    ok(false, "Should throw");
+  } catch (e) {
+    is(e.message, "URL constructor: http:://example.com/ is not a valid URL.");
+  }
 });