Bug 1534847 - Part 3: Pass originalText load failure through to UI instead of failing silently. r=jlast
authorLogan Smyth <loganfsmyth@gmail.com>
Thu, 14 Mar 2019 16:15:21 -0400
changeset 522011 f3500b0a2f3bbce27476b4602d1e7be26524d909
parent 522010 35af4e6b563343968f5556c7aebdabc929553c1d
child 522012 c525a24dffc34f710b52dfcb949fcafeb3f6bac6
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1534847
milestone67.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 1534847 - Part 3: Pass originalText load failure through to UI instead of failing silently. r=jlast Summary: Returning null here leaves us in an infinite loading state because null is treated as neither success nor failure. Reviewers: jlast Bug #: 1534847 Differential Revision: https://phabricator.services.mozilla.com/D23453
devtools/client/debugger/new/src/actions/sources/loadSourceText.js
devtools/client/debugger/new/src/reducers/sources.js
--- a/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
+++ b/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
@@ -17,31 +17,52 @@ import type { ThunkArgs } from "../types
 import type { Source } from "../../types";
 
 const requests = new Map();
 
 // Measures the time it takes for a source to load
 const loadSourceHistogram = "DEVTOOLS_DEBUGGER_LOAD_SOURCE_MS";
 const telemetry = new Telemetry();
 
-async function loadSource(state, source: Source, { sourceMaps, client }) {
+async function loadSource(
+  state,
+  source: Source,
+  { sourceMaps, client }
+): Promise<?{
+  text: string,
+  contentType: string
+}> {
   if (isOriginal(source)) {
-    return sourceMaps.getOriginalSourceText(source);
+    const result = await sourceMaps.getOriginalSourceText(source);
+    if (!result) {
+      // TODO: This allows pretty files to continue working the way they have
+      // been, but is very ugly. Remove this when we centralize pretty-printing
+      // in loadSource. https://github.com/firefox-devtools/debugger/issues/8071
+      if (source.isPrettyPrinted) {
+        return null;
+      }
+
+      // The way we currently try to load and select a pending
+      // selected location, it is possible that we will try to fetch the
+      // original source text right after the source map has been cleared
+      // after a navigation event.
+      throw new Error("Original source text unavailable");
+    }
+    return result;
   }
 
   if (!source.actors.length) {
     throw new Error("No source actor for loadSource");
   }
 
   telemetry.start(loadSourceHistogram, source);
   const response = await client.sourceContents(source.actors[0]);
   telemetry.finish(loadSourceHistogram, source);
 
   return {
-    id: source.id,
     text: response.source,
     contentType: response.contentType || "text/javascript"
   };
 }
 
 async function loadSourceTextPromise(
   source: Source,
   epoch: number,
--- a/devtools/client/debugger/new/src/reducers/sources.js
+++ b/devtools/client/debugger/new/src/reducers/sources.js
@@ -304,27 +304,29 @@ function updateProjectDirectoryRoot(stat
  */
 function updateLoadedState(
   state: SourcesState,
   action: LoadSourceAction
 ): SourcesState {
   const { sourceId } = action;
   let source;
 
-  // If there was a nativation between the time the action was started and
+  // If there was a navigation between the time the action was started and
   // completed, we don't want to update the store.
   if (action.epoch !== state.epoch) {
     return state;
   }
 
   if (action.status === "start") {
     source = { id: sourceId, loadedState: "loading" };
   } else if (action.status === "error") {
     source = { id: sourceId, error: action.error, loadedState: "loaded" };
   } else {
+    // TODO: Remove this once we centralize pretty-print and this can no longer
+    // return a null value when loading a in-progress prettyprinting file.
     if (!action.value) {
       return state;
     }
 
     source = {
       id: sourceId,
       text: action.value.text,
       contentType: action.value.contentType,