Bug 1834725 - [devtools] Retrieve the resolved source map URL and source map error. r=devtools-reviewers,nchevobbe

Extract information out of the source map worker to be stored into the Debugger store.

Renamed SourceMapLoader getOriginalURLs to loadSourceMap to better translate the heavy process involved in this method.
While doing that, also inline the transfer of source map ignore list in this call (instead of always having another worker message right after).

This will be used and tested in the next changeset.

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

devtools/client/debugger/panel.js

@@ -51,6 +51,8 @@ const DBG_STRINGS_URI = [
   // These are used in the AppErrorBoundary component
   "devtools/client/locales/startup.properties",
   "devtools/client/locales/components.properties",
+  // Used by SourceMapLoader
+  "devtools/client/locales/toolbox.properties",
 ];
 const L10N = new MultiLocalizationHelper(...DBG_STRINGS_URI);
 

devtools/client/debugger/src/actions/sources/newSources.js

@@ -6,7 +6,6 @@
  * Redux actions for the sources state
  * @module actions/sources
  */
-import { PROMISE } from "../utils/middleware/promise";
 import { insertSourceActors } from "../../actions/source-actors";
 import {
   makeSourceId,
@@ -69,36 +68,67 @@ function loadSourceMaps(sources) {
  * @static
  */
 function loadSourceMap(sourceActor) {
-  return async function ({ dispatch, getState, sourceMapLoader }) {
+  return async function ({ dispatch, getState, sourceMapLoader, panel }) {
     if (!prefs.clientSourceMapsEnabled || !sourceActor.sourceMapURL) {
       return [];
     }
 
-    let data = null;
+    let sources, ignoreListUrls, resolvedSourceMapURL, exception;
     try {
       // Ignore sourceMapURL on scripts that are part of HTML files, since
       // we currently treat sourcemaps as Source-wide, not SourceActor-specific.
       const source = getSourceByActorId(getState(), sourceActor.id);
       if (source) {
-        data = await sourceMapLoader.getOriginalURLs({
-          // Using source ID here is historical and eventually we'll want to
-          // switch to all of this being per-source-actor.
-          id: source.id,
-          url: sourceActor.url || "",
-          sourceMapBaseURL: sourceActor.sourceMapBaseURL || "",
-          sourceMapURL: sourceActor.sourceMapURL || "",
-          isWasm: sourceActor.introductionType === "wasm",
-        });
-        dispatch({
-          type: "ADD_SOURCEMAP_IGNORE_LIST_SOURCES",
-          [PROMISE]: sourceMapLoader.getSourceMapIgnoreList(source.id),
-        });
+        ({ sources, ignoreListUrls, resolvedSourceMapURL, exception } =
+          await sourceMapLoader.loadSourceMap({
+            // Using source ID here is historical and eventually we'll want to
+            // switch to all of this being per-source-actor.
+            id: source.id,
+            url: sourceActor.url || "",
+            sourceMapBaseURL: sourceActor.sourceMapBaseURL || "",
+            sourceMapURL: sourceActor.sourceMapURL || "",
+            isWasm: sourceActor.introductionType === "wasm",
+          }));
       }
     } catch (e) {
-      console.error(e);
+      exception = `Internal error: ${e.message}`;
     }
 
-    if (!data || !data.length) {
+    if (resolvedSourceMapURL) {
+      dispatch({
+        type: "RESOLVED_SOURCEMAP_URL",
+        sourceActor,
+        resolvedSourceMapURL,
+      });
+    }
+
+    if (ignoreListUrls?.length) {
+      dispatch({
+        type: "ADD_SOURCEMAP_IGNORE_LIST_SOURCES",
+        ignoreListUrls,
+      });
+    }
+
+    if (exception) {
+      // Catch all errors and log them to the Web Console for users to see.
+      const message = L10N.getFormatStr(
+        "toolbox.sourceMapFailure",
+        exception,
+        sourceActor.url,
+        sourceActor.sourceMapURL
+      );
+      panel.toolbox.commands.targetCommand.targetFront.logWarningInPage(
+        message,
+        "source map",
+        resolvedSourceMapURL
+      );
+
+      dispatch({
+        type: "SOURCE_MAP_ERROR",
+        sourceActor,
+        errorMessage: exception,
+      });
+
       // If this source doesn't have a sourcemap or there are no original files
       // existing, enable it for pretty printing
       dispatch({
@@ -110,7 +140,7 @@ function loadSourceMap(sourceActor) {
 
     // Before dispatching this action, ensure that the related sourceActor is still registered
     validateSourceActor(getState(), sourceActor);
-    return data;
+    return sources;
   };
 }
 

devtools/client/debugger/src/reducers/source-actors.js

@@ -25,6 +25,14 @@ function initialSourceActorsState() {
     // but this may be invalid. The source map URL or source map file content may be invalid.
     // In these scenarios we will remove the source actor from this set.
     mutableSourceActorsWithSourceMap: new Set(),
+
+    // Map(Source Actor ID: string => string)
+    // Store the exception message when processing the sourceMapURL field of the source actor.
+    mutableSourceMapErrors: new Map(),
+
+    // Map(Source Actor ID: string => string)
+    // When a bundle has a functional sourcemap, reports the resolved source map URL.
+    mutableResolvedSourceMapURL: new Map(),
   };
 }
 
@@ -79,6 +87,22 @@ export default function update(state = initialSourceActorsState(), action) {
         };
       }
       return state;
+
+    case "SOURCE_MAP_ERROR": {
+      state.mutableSourceMapErrors.set(
+        action.sourceActor.id,
+        action.errorMessage
+      );
+      return { ...state };
+    }
+
+    case "RESOLVED_SOURCEMAP_URL": {
+      state.mutableResolvedSourceMapURL.set(
+        action.sourceActor.id,
+        action.resolvedSourceMapURL
+      );
+      return { ...state };
+    }
   }
 
   return state;

devtools/client/debugger/src/reducers/source-blackbox.js

@@ -125,17 +125,15 @@ function update(state = initialSourceBlackBoxState(), action) {
       };
     }
 
-    case "ADD_SOURCEMAP_IGNORE_LIST_SOURCES":
-      if (action.status == "done") {
-        return {
-          ...state,
-          sourceMapIgnoreListUrls: [
-            ...state.sourceMapIgnoreListUrls,
-            ...action.value,
-          ],
-        };
-      }
-      return state;
+    case "ADD_SOURCEMAP_IGNORE_LIST_SOURCES": {
+      return {
+        ...state,
+        sourceMapIgnoreListUrls: [
+          ...state.sourceMapIgnoreListUrls,
+          ...action.ignoreListUrls,
+        ],
+      };
+    }
 
     case "NAVIGATE":
       return initialSourceBlackBoxState(state);

devtools/client/debugger/src/selectors/source-actors.js

@@ -40,6 +40,14 @@ export function isSourceActorWithSourceMap(state, sourceActorId) {
   return state.sourceActors.mutableSourceActorsWithSourceMap.has(sourceActorId);
 }
 
+export function getSourceMapErrorForSourceActor(state, sourceActorId) {
+  return state.sourceActors.mutableSourceMapErrors.get(sourceActorId);
+}
+
+export function getSourceMapResolvedURL(state, sourceActorId) {
+  return state.sourceActors.mutableResolvedSourceMapURL.get(sourceActorId);
+}
+
 // Used by threads selectors
 /**
  * Get all Source Actor objects for a given thread. See create.js:createSourceActor()

devtools/client/debugger/test/mochitest/browser_dbg-sourcemaps-redirect.js

@@ -48,7 +48,7 @@ add_task(async function () {
 
   is(
     value,
-    `Source map error: Error: NetworkError when attempting to fetch resource.\nResource URL: ${BASE_URL}/index.html\nSource Map URL: ${BASE_URL}/redirect[Learn More]`,
+    `Source map error: NetworkError when attempting to fetch resource.\nResource URL: ${BASE_URL}/index.html\nSource Map URL: ${BASE_URL}/redirect[Learn More]`,
     "A source map error message is logged indicating the redirect failed"
   );
 });

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

@@ -74,7 +74,7 @@ class SourceMapLoader extends WorkerDispatcher {
   getOriginalLocations = this.task("getOriginalLocations");
   getGeneratedRangesForOriginal = this.task("getGeneratedRangesForOriginal");
   getFileGeneratedRange = this.task("getFileGeneratedRange");
-  getSourceMapIgnoreList = this.task("getSourceMapIgnoreList");
+  loadSourceMap = this.task("loadSourceMap");
 
   async getOriginalSourceText(originalSourceId) {
     try {

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

@@ -25,6 +25,7 @@ const {
 const assert = require("resource://devtools/client/shared/source-map-loader/utils/assert.js");
 const {
   fetchSourceMap,
+  resolveSourceMapURL,
   hasOriginalURL,
   clearOriginalURLs,
 } = require("resource://devtools/client/shared/source-map-loader/utils/fetchSourceMap.js");
@@ -45,15 +46,87 @@ const {
   clearWasmXScopes,
 } = require("resource://devtools/client/shared/source-map-loader/wasm-dwarf/wasmXScopes.js");
 
-async function getOriginalURLs(generatedSource) {
-  await fetchSourceMap(generatedSource);
-  const data = await getSourceMapWithMetadata(generatedSource.id);
-  return data ? data.sources : null;
+/**
+ * Create "original source info" objects being handed over to the main thread
+ * to describe original sources referenced in a source map
+ */
+function mapToOriginalSourceInfos(generatedId, urls) {
+  return urls.map(url => {
+    return {
+      id: generatedToOriginalId(generatedId, url),
+      url,
+    };
+  });
 }
 
-async function getSourceMapIgnoreList(generatedSourceId) {
-  const data = await getSourceMapWithMetadata(generatedSourceId);
-  return data ? data.ignoreListUrls : [];
+/**
+ * Load the source map and retrieved infos about all the original sources
+ * referenced in that source map.
+ *
+ * @param {Object} generatedSource
+ *        Source object for a bundle referencing a source map
+ * @return {Array<Object>|null}
+ *        List of object with id and url attributes describing the original sources.
+ */
+async function getOriginalURLs(generatedSource) {
+  const { resolvedSourceMapURL, baseURL } =
+    resolveSourceMapURL(generatedSource);
+  const map = await fetchSourceMap(
+    generatedSource,
+    resolvedSourceMapURL,
+    baseURL
+  );
+  return map ? mapToOriginalSourceInfos(generatedSource.id, map.sources) : null;
+}
+
+/**
+ * Load the source map for a given bundle and return information
+ * about the related original sources and the source map itself.
+ *
+ * @param {Object} generatedSource
+ *        Source object for the bundle.
+ * @return {Object}
+ *  - {Array<Object>} sources
+ *    Object with id and url attributes, refering to the related original sources
+ *    referenced in the source map.
+ *  - [String} resolvedSourceMapURL
+ *    Absolute URL for the source map file.
+ *  - {Array<String>} ignoreListUrls
+ *    List of URLs of sources, designated by the source map, to be ignored in the debugger.
+ *  - {String} exception
+ *    In case of error, a string describing the situation.
+ */
+async function loadSourceMap(generatedSource) {
+  const { resolvedSourceMapURL, baseURL } =
+    resolveSourceMapURL(generatedSource);
+  try {
+    const map = await fetchSourceMap(
+      generatedSource,
+      resolvedSourceMapURL,
+      baseURL
+    );
+    if (!map.sources.length) {
+      throw new Error("No sources are declared in this source map.");
+    }
+    let ignoreListUrls = [];
+    if (map.x_google_ignoreList?.length) {
+      ignoreListUrls = map.x_google_ignoreList.map(
+        sourceIndex => map.sources[sourceIndex]
+      );
+    }
+    return {
+      sources: mapToOriginalSourceInfos(generatedSource.id, map.sources),
+      resolvedSourceMapURL,
+      ignoreListUrls,
+    };
+  } catch (e) {
+    return {
+      sources: [],
+      resolvedSourceMapURL,
+      ignoreListUrls: [],
+      exception: e.message,
+    };
+  }
 }
 
 const COMPUTED_SPANS = new WeakSet();
@@ -558,6 +631,7 @@ function clearSourceMaps() {
 
 module.exports = {
   getOriginalURLs,
+  loadSourceMap,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
@@ -567,7 +641,6 @@ module.exports = {
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
-  getSourceMapIgnoreList,
   setSourceMapForGeneratedSources,
   clearSourceMaps,
 };

devtools/client/shared/source-map-loader/utils/fetchSourceMap.js

@@ -33,14 +33,14 @@ function hasOriginalURL(url) {
   return originalURLs.has(url);
 }
 
-function _resolveSourceMapURL(source) {
+function resolveSourceMapURL(source) {
   let { sourceMapBaseURL, sourceMapURL } = source;
   sourceMapBaseURL = sourceMapBaseURL || "";
   sourceMapURL = sourceMapURL || "";
 
   if (!sourceMapBaseURL) {
     // If the source doesn't have a URL, don't resolve anything.
-    return { sourceMapURL, baseURL: sourceMapURL };
+    return { resolvedSourceMapURL: sourceMapURL, baseURL: sourceMapURL };
   }
 
   let resolvedString;
@@ -63,14 +63,11 @@ function _resolveSourceMapURL(source) {
     baseURL = resolvedString;
   }
 
-  return { sourceMapURL: resolvedString, baseURL };
+  return { resolvedSourceMapURL: resolvedString, baseURL };
 }
 
-async function _resolveAndFetch(generatedSource) {
-  // Fetch the sourcemap over the network and create it.
-  const { sourceMapURL, baseURL } = _resolveSourceMapURL(generatedSource);
-
-  let fetched = await networkRequest(sourceMapURL, {
+async function _fetch(generatedSource, resolvedSourceMapURL, baseURL) {
+  let fetched = await networkRequest(resolvedSourceMapURL, {
     loadFromCache: false,
     // Blocking redirects on the sourceMappingUrl as its not easy to verify if the
     // redirect protocol matches the supported ones.
@@ -105,7 +102,7 @@ async function _resolveAndFetch(generatedSource) {
   return map;
 }
 
-function fetchSourceMap(generatedSource) {
+function fetchSourceMap(generatedSource, resolvedSourceMapURL, baseURL) {
   const existingRequest = getSourceMap(generatedSource.id);
 
   // If it has already been requested, return the request. Make sure
@@ -124,7 +121,7 @@ function fetchSourceMap(generatedSource) {
   }
 
   // Fire off the request, set it in the cache, and return it.
-  const req = _resolveAndFetch(generatedSource);
+  const req = _fetch(generatedSource, resolvedSourceMapURL, baseURL);
   // Make sure the cached promise does not reject, because we only
   // want to report the error once.
   setSourceMap(
@@ -134,4 +131,9 @@ function fetchSourceMap(generatedSource) {
   return req;
 }
 
-module.exports = { fetchSourceMap, hasOriginalURL, clearOriginalURLs };
+module.exports = {
+  fetchSourceMap,
+  hasOriginalURL,
+  clearOriginalURLs,
+  resolveSourceMapURL,
+};

devtools/client/shared/source-map-loader/utils/sourceMapRequests.js

@@ -78,22 +78,12 @@ function setSourceMap(generatedId, request) {
       }
 
       const urlsById = new Map();
-      const sources = [];
-      let ignoreListUrls = [];
-
-      if (map.x_google_ignoreList?.length) {
-        ignoreListUrls = map.x_google_ignoreList.map(
-          sourceIndex => map.sources[sourceIndex]
-        );
-      }
 
       for (const url of map.sources) {
         const id = generatedToOriginalId(generatedId, url);
-
         urlsById.set(id, url);
-        sources.push({ id, url });
       }
-      return { map, urlsById, sources, ignoreListUrls };
+      return { map, urlsById };
     })
   );
 }

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

@@ -17,7 +17,7 @@ const {
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
-  getSourceMapIgnoreList,
+  loadSourceMap,
   clearSourceMaps,
   setSourceMapForGeneratedSources,
 } = require("resource://devtools/client/shared/source-map-loader/source-map.js");
@@ -44,7 +44,7 @@ self.onmessage = workerHandler({
   getOriginalStackFrames,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
-  getSourceMapIgnoreList,
+  loadSourceMap,
   setSourceMapForGeneratedSources,
   clearSourceMaps,
 });