Bug Bug 1388728 - Update module loader error handling to match the spec r=bkelly
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 22 Aug 2017 10:34:14 +0100
changeset 428118 d7dcccf15b0defa13178d26659baa0482ef1a2f4
parent 428117 60a74915871b5ee78ad143b0eef4e26ef51bb55a
child 428119 4c8a48ebf9994d0be86606b4cb00aa3d4a1b5eb1
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1388728
milestone57.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 Bug 1388728 - Update module loader error handling to match the spec r=bkelly
dom/script/ModuleLoadRequest.cpp
dom/script/ModuleLoadRequest.h
dom/script/ModuleScript.cpp
dom/script/ModuleScript.h
dom/script/ScriptLoader.cpp
dom/script/ScriptLoader.h
--- a/dom/script/ModuleLoadRequest.cpp
+++ b/dom/script/ModuleLoadRequest.cpp
@@ -43,20 +43,26 @@ ModuleLoadRequest::ModuleLoadRequest(nsI
 {}
 
 void
 ModuleLoadRequest::Cancel()
 {
   ScriptLoadRequest::Cancel();
   mModuleScript = nullptr;
   mProgress = ScriptLoadRequest::Progress::Ready;
+  CancelImports();
+  mReady.RejectIfExists(NS_ERROR_DOM_ABORT_ERR, __func__);
+}
+
+void
+ModuleLoadRequest::CancelImports()
+{
   for (size_t i = 0; i < mImports.Length(); i++) {
     mImports[i]->Cancel();
   }
-  mReady.RejectIfExists(NS_ERROR_FAILURE, __func__);
 }
 
 void
 ModuleLoadRequest::SetReady()
 {
   // Mark a module as ready to execute. This means that this module and all it
   // dependencies have had their source loaded, parsed as a module and the
   // modules instantiated.
@@ -79,38 +85,69 @@ void
 ModuleLoadRequest::ModuleLoaded()
 {
   // A module that was found to be marked as fetching in the module map has now
   // been loaded.
 
   LOG(("ScriptLoadRequest (%p): Module loaded", this));
 
   mModuleScript = mLoader->GetFetchedModule(mURI);
+  if (!mModuleScript || mModuleScript->IsErrored()) {
+    ModuleErrored();
+    return;
+  }
+
   mLoader->StartFetchingModuleDependencies(this);
 }
 
 void
+ModuleLoadRequest::ModuleErrored()
+{
+  LOG(("ScriptLoadRequest (%p): Module errored", this));
+
+  mLoader->CheckModuleDependenciesLoaded(this);
+  MOZ_ASSERT(!mModuleScript || mModuleScript->IsErrored());
+
+  CancelImports();
+  SetReady();
+  LoadFinished();
+}
+
+void
 ModuleLoadRequest::DependenciesLoaded()
 {
   // The module and all of its dependencies have been successfully fetched and
   // compiled.
 
   LOG(("ScriptLoadRequest (%p): Module dependencies loaded", this));
 
+  MOZ_ASSERT(mModuleScript);
+
+  mLoader->CheckModuleDependenciesLoaded(this);
   SetReady();
-  mLoader->ProcessLoadedModuleTree(this);
-  mLoader = nullptr;
-  mParent = nullptr;
+  LoadFinished();
 }
 
 void
 ModuleLoadRequest::LoadFailed()
 {
+  // We failed to load the source text or an error occurred unrelated to the
+  // content of the module (e.g. OOM).
+
   LOG(("ScriptLoadRequest (%p): Module load failed", this));
 
+  MOZ_ASSERT(!mModuleScript);
+
   Cancel();
+  LoadFinished();
+}
+
+void
+ModuleLoadRequest::LoadFinished()
+{
   mLoader->ProcessLoadedModuleTree(this);
+
   mLoader = nullptr;
   mParent = nullptr;
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/script/ModuleLoadRequest.h
+++ b/dom/script/ModuleLoadRequest.h
@@ -41,19 +41,25 @@ public:
   {
     return mIsTopLevel;
   }
 
   void SetReady() override;
   void Cancel() override;
 
   void ModuleLoaded();
+  void ModuleErrored();
   void DependenciesLoaded();
   void LoadFailed();
 
+ private:
+  void LoadFinished();
+  void CancelImports();
+
+ public:
   // Is this a request for a top level module script or an import?
   bool mIsTopLevel;
 
   // The base URL used for resolving relative module imports.
   nsCOMPtr<nsIURI> mBaseURL;
 
   // Pointer to the script loader, used to trigger actions when the module load
   // finishes.
--- a/dom/script/ModuleScript.cpp
+++ b/dom/script/ModuleScript.cpp
@@ -17,58 +17,102 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(ModuleScript)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ModuleScript)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mBaseURL)
   tmp->UnlinkModuleRecord();
+  tmp->mError.setUndefined();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ModuleScript)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLoader)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ModuleScript)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mModuleRecord)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mError)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleScript)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(ModuleScript)
 
-ModuleScript::ModuleScript(ScriptLoader* aLoader, nsIURI* aBaseURL,
-                           JS::Handle<JSObject*> aModuleRecord)
+ModuleScript::ModuleScript(ScriptLoader* aLoader, nsIURI* aBaseURL)
  : mLoader(aLoader),
-   mBaseURL(aBaseURL),
-   mModuleRecord(aModuleRecord)
+   mBaseURL(aBaseURL)
 {
   MOZ_ASSERT(mLoader);
   MOZ_ASSERT(mBaseURL);
-  MOZ_ASSERT(mModuleRecord);
+  MOZ_ASSERT(!mModuleRecord);
+  MOZ_ASSERT(mError.isUndefined());
+}
+
+void
+ModuleScript::UnlinkModuleRecord()
+{
+  // Remove module's back reference to this object request if present.
+  if (mModuleRecord) {
+    MOZ_ASSERT(JS::GetModuleHostDefinedField(mModuleRecord).toPrivate() ==
+               this);
+    JS::SetModuleHostDefinedField(mModuleRecord, JS::UndefinedValue());
+    mModuleRecord = nullptr;
+  }
+}
+
+ModuleScript::~ModuleScript()
+{
+  // The object may be destroyed without being unlinked first.
+  UnlinkModuleRecord();
+  DropJSObjects(this);
+}
+
+void
+ModuleScript::SetModuleRecord(JS::Handle<JSObject*> aModuleRecord)
+{
+  MOZ_ASSERT(!mModuleRecord);
+  MOZ_ASSERT(mError.isUndefined());
+
+  mModuleRecord = aModuleRecord;
 
   // Make module's host defined field point to this module script object.
   // This is cleared in the UnlinkModuleRecord().
   JS::SetModuleHostDefinedField(mModuleRecord, JS::PrivateValue(this));
   HoldJSObjects(this);
 }
 
 void
-ModuleScript::UnlinkModuleRecord()
+ModuleScript::SetPreInstantiationError(const JS::Value& aError)
 {
-  // Remove module's back reference to this object request if present.
-  MOZ_ASSERT(JS::GetModuleHostDefinedField(mModuleRecord).toPrivate() ==
-             this);
-  JS::SetModuleHostDefinedField(mModuleRecord, JS::UndefinedValue());
-  mModuleRecord = nullptr;
+  MOZ_ASSERT(!aError.isUndefined());
+
+  UnlinkModuleRecord();
+  mError = aError;
+
+  HoldJSObjects(this);
 }
 
-ModuleScript::~ModuleScript()
+bool
+ModuleScript::IsErrored() const
 {
-  if (mModuleRecord) {
-    // The object may be destroyed without being unlinked first.
-    UnlinkModuleRecord();
+  if (!mModuleRecord) {
+    MOZ_ASSERT(!mError.isUndefined());
+    return true;
   }
-  DropJSObjects(this);
+
+  return JS::IsModuleErrored(mModuleRecord);
+}
+
+JS::Value
+ModuleScript::Error() const
+{
+  MOZ_ASSERT(IsErrored());
+
+  if (!mModuleRecord) {
+    return mError;
+  }
+
+  return JS::GetModuleError(mModuleRecord);
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/script/ModuleScript.h
+++ b/dom/script/ModuleScript.h
@@ -18,30 +18,36 @@ namespace dom {
 
 class ScriptLoader;
 
 class ModuleScript final : public nsISupports
 {
   RefPtr<ScriptLoader> mLoader;
   nsCOMPtr<nsIURI> mBaseURL;
   JS::Heap<JSObject*> mModuleRecord;
+  JS::Heap<JS::Value> mError;
 
   ~ModuleScript();
 
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ModuleScript)
 
   ModuleScript(ScriptLoader* aLoader,
-               nsIURI* aBaseURL,
-               JS::Handle<JSObject*> aModuleRecord);
+               nsIURI* aBaseURL);
+
+  void SetModuleRecord(JS::Handle<JSObject*> aModuleRecord);
+  void SetPreInstantiationError(const JS::Value& aError);
 
   ScriptLoader* Loader() const { return mLoader; }
   JSObject* ModuleRecord() const { return mModuleRecord; }
   nsIURI* BaseURL() const { return mBaseURL; }
 
+  bool IsErrored() const;
+  JS::Value Error() const;
+
   void UnlinkModuleRecord();
 };
 
 } // dom namespace
 } // mozilla namespace
 
 #endif // mozilla_dom_ModuleScript_h
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -69,17 +69,17 @@ using JS::SourceBufferHolder;
 namespace mozilla {
 namespace dom {
 
 LazyLogModule ScriptLoader::gCspPRLog("CSP");
 LazyLogModule ScriptLoader::gScriptLoaderLog("ScriptLoader");
 
 #undef LOG
 #define LOG(args) \
-  MOZ_LOG(gScriptLoaderLog, mozilla::LogLevel::Debug, args)
+  MOZ_LOG(ScriptLoader::gScriptLoaderLog, mozilla::LogLevel::Debug, args)
 
 #define LOG_ENABLED() \
   MOZ_LOG_TEST(ScriptLoader::gScriptLoaderLog, mozilla::LogLevel::Debug)
 
 // Alternate Data MIME type used by the ScriptLoader to register that we want to
 // store bytecode without reading it.
 static NS_NAMED_LITERAL_CSTRING(kNullMimeType, "javascript/null");
 
@@ -353,39 +353,39 @@ ScriptLoader::SetModuleFetchStarted(Modu
   MOZ_ASSERT(!ModuleMapContainsModule(aRequest));
   mFetchingModules.Put(aRequest->mURI, nullptr);
 }
 
 void
 ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest* aRequest,
                                                              nsresult aResult)
 {
-  // Update module map with the result of fetching a single module script.  The
-  // module script pointer is nullptr on error.
+  // Update module map with the result of fetching a single module script.
   //
   // If any requests for the same URL are waiting on this one to complete, they
   // will have ModuleLoaded or LoadFailed on them when the promise is
   // resolved/rejected. This is set up in StartLoad.
 
-  LOG(("ScriptLoadRequest (%p): Module fetch finished (result == %u)",
-       aRequest, unsigned(aResult)));
-
-  MOZ_ASSERT(!aRequest->IsReadyToRun());
+  LOG(("ScriptLoadRequest (%p): Module fetch finished (script == %p, result == %u)",
+       aRequest, aRequest->mModuleScript.get(), unsigned(aResult)));
 
   RefPtr<GenericPromise::Private> promise;
   MOZ_ALWAYS_TRUE(mFetchingModules.Remove(aRequest->mURI, getter_AddRefs(promise)));
 
-  RefPtr<ModuleScript> ms(aRequest->mModuleScript);
-  MOZ_ASSERT(NS_SUCCEEDED(aResult) == (ms != nullptr));
-  mFetchedModules.Put(aRequest->mURI, ms);
+  RefPtr<ModuleScript> moduleScript(aRequest->mModuleScript);
+  MOZ_ASSERT(NS_FAILED(aResult) == !moduleScript);
+
+  mFetchedModules.Put(aRequest->mURI, moduleScript);
 
   if (promise) {
-    if (ms) {
+    if (moduleScript) {
+      LOG(("ScriptLoadRequest (%p):   resolving %p", aRequest, promise.get()));
       promise->Resolve(true, __func__);
     } else {
+      LOG(("ScriptLoadRequest (%p):   rejecting %p", aRequest, promise.get()));
       promise->Reject(aResult, __func__);
     }
   }
 }
 
 RefPtr<GenericPromise>
 ScriptLoader::WaitForModuleFetch(ModuleLoadRequest* aRequest)
 {
@@ -423,33 +423,45 @@ ScriptLoader::GetFetchedModule(nsIURI* a
 }
 
 nsresult
 ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest)
 {
   MOZ_ASSERT(!aRequest->mModuleScript);
 
   nsresult rv = CreateModuleScript(aRequest);
+  MOZ_ASSERT(NS_FAILED(rv) == !aRequest->mModuleScript);
+
   SetModuleFetchFinishedAndResumeWaitingRequests(aRequest, rv);
 
   aRequest->mScriptText.clearAndFree();
 
-  if (NS_SUCCEEDED(rv)) {
+  if (NS_FAILED(rv)) {
+    aRequest->LoadFailed();
+    return rv;
+  }
+
+  if (!aRequest->mModuleScript->IsErrored()) {
     StartFetchingModuleDependencies(aRequest);
   }
 
-  return rv;
+  return NS_OK;
 }
 
+static nsresult
+ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray<nsIURI>& aUrls);
+
 nsresult
 ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest)
 {
   MOZ_ASSERT(!aRequest->mModuleScript);
   MOZ_ASSERT(aRequest->mBaseURL);
 
+  LOG(("ScriptLoadRequest (%p): Create module script", aRequest));
+
   nsCOMPtr<nsIScriptGlobalObject> globalObject = GetScriptGlobalObject();
   if (!globalObject) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIScriptContext> context = globalObject->GetScriptContext();
   if (!context) {
     return NS_ERROR_FAILURE;
@@ -480,77 +492,96 @@ ScriptLoader::CreateModuleScript(ModuleL
       rv = FillCompileOptionsForRequest(aes, aRequest, global, &options);
 
       if (NS_SUCCEEDED(rv)) {
         nsAutoString inlineData;
         SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
         rv = nsJSUtils::CompileModule(cx, srcBuf, global, options, &module);
       }
     }
+
     MOZ_ASSERT(NS_SUCCEEDED(rv) == (module != nullptr));
-    if (module) {
-      aRequest->mModuleScript =
-        new ModuleScript(this, aRequest->mBaseURL, module);
+    RefPtr<ModuleScript> moduleScript = new ModuleScript(this, aRequest->mBaseURL);
+    aRequest->mModuleScript = moduleScript;
+
+    if (!module) {
+      LOG(("ScriptLoadRequest (%p):   compilation failed (%d)",
+           aRequest, unsigned(rv)));
+
+      MOZ_ASSERT(aes.HasException());
+      JS::Rooted<JS::Value> error(cx);
+      if (!aes.StealException(&error)) {
+        aRequest->mModuleScript = nullptr;
+        return NS_ERROR_FAILURE;
+      }
+
+      moduleScript->SetPreInstantiationError(error);
+      aRequest->ModuleErrored();
+      return NS_OK;
+    }
+
+    moduleScript->SetModuleRecord(module);
+
+    // Validate requested modules and treat failure to resolve module specifiers
+    // the same as a parse error.
+    nsCOMArray<nsIURI> urls;
+    rv = ResolveRequestedModules(aRequest, urls);
+    if (NS_FAILED(rv)) {
+      // ResolveRequestedModules sets pre-instanitation error on failure.
+      MOZ_ASSERT(moduleScript->IsErrored());
+      aRequest->ModuleErrored();
+      return NS_OK;
     }
   }
 
   context->SetProcessingScriptTag(oldProcessingScriptTag);
 
+  LOG(("ScriptLoadRequest (%p):   module script == %p",
+       aRequest, aRequest->mModuleScript.get()));
+
   return rv;
 }
 
 static bool
-ThrowTypeError(JSContext* aCx, ModuleScript* aScript,
-               const nsString& aMessage)
+CreateTypeError(JSContext* aCx, ModuleScript* aScript, const nsString& aMessage,
+                JS::MutableHandle<JS::Value> aError)
 {
   JS::Rooted<JSObject*> module(aCx, aScript->ModuleRecord());
   JS::Rooted<JSScript*> script(aCx, JS::GetModuleScript(aCx, module));
   JS::Rooted<JSString*> filename(aCx);
   filename = JS_NewStringCopyZ(aCx, JS_GetScriptFilename(script));
   if (!filename) {
     return false;
   }
 
   JS::Rooted<JSString*> message(aCx, JS_NewUCStringCopyZ(aCx, aMessage.get()));
   if (!message) {
     return false;
   }
 
-  JS::Rooted<JS::Value> error(aCx);
-  if (!JS::CreateError(aCx, JSEXN_TYPEERR, nullptr, filename, 0, 0, nullptr,
-                       message, &error)) {
-    return false;
-  }
-
-  JS_SetPendingException(aCx, error);
-  return false;
+  return JS::CreateError(aCx, JSEXN_TYPEERR, nullptr, filename, 0, 0, nullptr,
+                         message, aError);
 }
 
-static bool
+static nsresult
 HandleResolveFailure(JSContext* aCx, ModuleScript* aScript,
                      const nsAString& aSpecifier)
 {
   // TODO: How can we get the line number of the failed import?
 
   nsAutoString message(NS_LITERAL_STRING("Error resolving module specifier: "));
   message.Append(aSpecifier);
 
-  return ThrowTypeError(aCx, aScript, message);
-}
-
-static bool
-HandleModuleNotFound(JSContext* aCx, ModuleScript* aScript,
-                     const nsAString& aSpecifier)
-{
-  // TODO: How can we get the line number of the failed import?
-
-  nsAutoString message(NS_LITERAL_STRING("Resolved module not found in map: "));
-  message.Append(aSpecifier);
-
-  return ThrowTypeError(aCx, aScript, message);
+  JS::Rooted<JS::Value> error(aCx);
+  if (!CreateTypeError(aCx, aScript, message, &error)) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  aScript->SetPreInstantiationError(error);
+  return NS_OK;
 }
 
 static already_AddRefed<nsIURI>
 ResolveModuleSpecifier(ModuleScript* aScript,
                        const nsAString& aSpecifier)
 {
   // The following module specifiers are allowed by the spec:
   //  - a valid absolute URL
@@ -638,17 +669,18 @@ ResolveRequestedModules(ModuleLoadReques
     if (!specifier.init(cx, val)) {
       return NS_ERROR_FAILURE;
     }
 
     // Let url be the result of resolving a module specifier given module script and requested.
     ModuleScript* ms = aRequest->mModuleScript;
     nsCOMPtr<nsIURI> uri = ResolveModuleSpecifier(ms, specifier);
     if (!uri) {
-      HandleResolveFailure(cx, ms, specifier);
+      nsresult rv = HandleResolveFailure(cx, ms, specifier);
+      NS_ENSURE_SUCCESS(rv, rv);
       return NS_ERROR_FAILURE;
     }
 
     bool isAncestor;
     nsresult rv = RequestedModuleIsInAncestorList(aRequest, uri, &isAncestor);
     NS_ENSURE_SUCCESS(rv, rv);
     if (!isAncestor) {
       aUrls.AppendElement(uri.forget());
@@ -657,24 +689,27 @@ ResolveRequestedModules(ModuleLoadReques
 
   return NS_OK;
 }
 
 void
 ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest)
 {
   MOZ_ASSERT(aRequest->mModuleScript);
+  MOZ_ASSERT(!aRequest->mModuleScript->IsErrored());
   MOZ_ASSERT(!aRequest->IsReadyToRun());
 
+  LOG(("ScriptLoadRequest (%p): Start fetching module dependencies", aRequest));
+
   aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports;
 
   nsCOMArray<nsIURI> urls;
   nsresult rv = ResolveRequestedModules(aRequest, urls);
   if (NS_FAILED(rv)) {
-    aRequest->LoadFailed();
+    aRequest->ModuleErrored();
     return;
   }
 
   if (urls.Length() == 0) {
     // There are no descendents to load so this request is ready.
     aRequest->DependenciesLoaded();
     return;
   }
@@ -688,17 +723,17 @@ ScriptLoader::StartFetchingModuleDepende
     importsReady.AppendElement(childReady);
   }
 
   // Wait for all imports to become ready.
   RefPtr<GenericPromise::AllPromiseType> allReady =
     GenericPromise::All(GetMainThreadSerialEventTarget(), importsReady);
   allReady->Then(GetMainThreadSerialEventTarget(), __func__, aRequest,
                  &ModuleLoadRequest::DependenciesLoaded,
-                 &ModuleLoadRequest::LoadFailed);
+                 &ModuleLoadRequest::ModuleErrored);
 }
 
 RefPtr<GenericPromise>
 ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest,
                                                  nsIURI* aURI)
 {
   MOZ_ASSERT(aURI);
 
@@ -707,61 +742,75 @@ ScriptLoader::StartFetchingModuleAndDepe
                             aRequest->mCORSMode, aRequest->mIntegrity, this);
 
   childRequest->mIsTopLevel = false;
   childRequest->mURI = aURI;
   childRequest->mIsInline = false;
   childRequest->mReferrerPolicy = aRequest->mReferrerPolicy;
   childRequest->mParent = aRequest;
 
+  if (LOG_ENABLED()) {
+    nsAutoCString url1;
+    aRequest->mURI->GetAsciiSpec(url1);
+
+    nsAutoCString url2;
+    aURI->GetAsciiSpec(url2);
+
+    LOG(("ScriptLoadRequest (%p): Start fetching dependency %p", aRequest, childRequest.get()));
+    LOG(("StartFetchingModuleAndDependencies \"%s\" -> \"%s\"", url1.get(), url2.get()));
+  }
+
   RefPtr<GenericPromise> ready = childRequest->mReady.Ensure(__func__);
 
   nsresult rv = StartLoad(childRequest);
   if (NS_FAILED(rv)) {
+    MOZ_ASSERT(!childRequest->mModuleScript);
+    LOG(("ScriptLoadRequest (%p):   rejecting %p", aRequest, &childRequest->mReady));
     childRequest->mReady.Reject(rv, __func__);
     return ready;
   }
 
   aRequest->mImports.AppendElement(childRequest);
   return ready;
 }
 
+// 8.1.3.8.1 HostResolveImportedModule(referencingModule, specifier)
 bool
 HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp)
 {
+
   MOZ_ASSERT(argc == 2);
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   JS::Rooted<JSObject*> module(aCx, &args[0].toObject());
   JS::Rooted<JSString*> specifier(aCx, args[1].toString());
 
   // Let referencing module script be referencingModule.[[HostDefined]].
   JS::Value value = JS::GetModuleHostDefinedField(module);
   auto script = static_cast<ModuleScript*>(value.toPrivate());
   MOZ_ASSERT(script->ModuleRecord() == module);
 
   // Let url be the result of resolving a module specifier given referencing
-  // module script and specifier. If the result is failure, throw a TypeError
-  // exception and abort these steps.
+  // module script and specifier.
   nsAutoJSString string;
   if (!string.init(aCx, specifier)) {
     return false;
   }
 
   nsCOMPtr<nsIURI> uri = ResolveModuleSpecifier(script, string);
-  if (!uri) {
-    return HandleResolveFailure(aCx, script, string);
-  }
-
-  // Let resolved module script be the value of the entry in module map whose
-  // key is url. If no such entry exists, throw a TypeError exception and abort
-  // these steps.
+
+  // This cannot fail because resolving a module specifier must have been
+  // previously successful with these same two arguments.
+  MOZ_ASSERT(uri, "Failed to resolve previously-resolved module specifier");
+
+  // Let resolved module script be moduleMap[url]. (This entry must exist for us
+  // to have gotten to this point.)
   ModuleScript* ms = script->Loader()->GetFetchedModule(uri);
-  if (!ms) {
-    return HandleModuleNotFound(aCx, script, string);
-  }
+  MOZ_ASSERT(ms, "Resolved module not found in module map");
+
+  MOZ_ASSERT(!ms->IsErrored());
 
   *vp = JS::ObjectValue(*ms->ModuleRecord());
   return true;
 }
 
 static nsresult
 EnsureModuleResolveHook(JSContext* aCx)
 {
@@ -776,28 +825,98 @@ EnsureModuleResolveHook(JSContext* aCx)
     return NS_ERROR_FAILURE;
   }
 
   JS::SetModuleResolveHook(aCx, func);
   return NS_OK;
 }
 
 void
+ScriptLoader::CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest)
+{
+  LOG(("ScriptLoadRequest (%p): Check dependencies loaded", aRequest));
+
+  RefPtr<ModuleScript> moduleScript = aRequest->mModuleScript;
+  if (moduleScript && !moduleScript->IsErrored()) {
+    for (auto childRequest : aRequest->mImports) {
+      ModuleScript* childScript = childRequest->mModuleScript;
+      if (!childScript) {
+        aRequest->mModuleScript = nullptr;
+        LOG(("ScriptLoadRequest (%p):   %p failed (load error)", aRequest, childScript));
+        return;
+      } else if (childScript->IsErrored()) {
+        moduleScript->SetPreInstantiationError(childScript->Error());
+        LOG(("ScriptLoadRequest (%p):   %p failed (error)", aRequest, childScript));
+        return;
+      }
+    }
+  }
+
+  LOG(("ScriptLoadRequest (%p):   all ok", aRequest));
+}
+
+void
 ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest)
 {
   if (aRequest->IsTopLevel()) {
+    ModuleScript* moduleScript = aRequest->mModuleScript;
+    if (moduleScript && !moduleScript->IsErrored()) {
+      if (!InstantiateModuleTree(aRequest)) {
+        aRequest->mModuleScript = nullptr;
+      }
+    }
     MaybeMoveToLoadedList(aRequest);
     ProcessPendingRequests();
   }
 
   if (aRequest->mWasCompiledOMT) {
     mDocument->UnblockOnload(false);
   }
 }
 
+bool
+ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest)
+{
+  // Instantiate a top-level module and record any error.
+
+  MOZ_ASSERT(aRequest);
+  MOZ_ASSERT(aRequest->IsTopLevel());
+
+  LOG(("ScriptLoadRequest (%p): Instantiate module tree", aRequest));
+
+  ModuleScript* moduleScript = aRequest->mModuleScript;
+  MOZ_ASSERT(moduleScript);
+  MOZ_ASSERT(moduleScript->ModuleRecord());
+
+  nsAutoMicroTask mt;
+  AutoJSAPI jsapi;
+  if (NS_WARN_IF(!jsapi.Init(moduleScript->ModuleRecord()))) {
+    return false;
+  }
+
+  nsresult rv = EnsureModuleResolveHook(jsapi.cx());
+  NS_ENSURE_SUCCESS(rv, false);
+
+  JS::Rooted<JSObject*> module(jsapi.cx(), moduleScript->ModuleRecord());
+  bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module));
+
+  if (!ok) {
+    LOG(("ScriptLoadRequest (%p): Instantiate failed", aRequest));
+    MOZ_ASSERT(jsapi.HasException());
+    JS::RootedValue exception(jsapi.cx());
+    if (!jsapi.StealException(&exception)) {
+      return false;
+    }
+    MOZ_ASSERT(!exception.isUndefined());
+    // Ignore the exception. It will be recorded in the module record.
+  }
+
+  return true;
+}
+
 nsresult
 ScriptLoader::RestartLoad(ScriptLoadRequest* aRequest)
 {
   MOZ_ASSERT(aRequest->IsBytecode());
   aRequest->mScriptBytecode.clearAndFree();
   TRACE_FOR_TEST(aRequest->mElement, "scriptloader_fallback");
 
   // Start a new channel from which we explicitly request to stream the source
@@ -1368,27 +1487,36 @@ ScriptLoader::ProcessScriptElement(nsISc
   request->mIsInline = true;
   request->mURI = mDocument->GetDocumentURI();
   request->mLineNo = aElement->GetScriptLineNumber();
   request->mProgress = ScriptLoadRequest::Progress::Loading_Source;
   request->mDataType = ScriptLoadRequest::DataType::Source;
   TRACE_FOR_TEST_BOOL(request->mElement, "scriptloader_load_source");
   CollectScriptTelemetry(nullptr, request);
 
+  LOG(("ScriptLoadRequest (%p): Created request for inline script",
+       request.get()));
+
   if (request->IsModuleRequest()) {
     ModuleLoadRequest* modReq = request->AsModuleRequest();
     modReq->mBaseURL = mDocument->GetDocBaseURI();
     rv = CreateModuleScript(modReq);
-    NS_ENSURE_SUCCESS(rv, false);
-    StartFetchingModuleDependencies(modReq);
+    MOZ_ASSERT(NS_FAILED(rv) == !modReq->mModuleScript);
+    if (NS_FAILED(rv)) {
+      modReq->LoadFailed();
+      return false;
+    }
     if (aElement->GetScriptAsync()) {
       mLoadingAsyncRequests.AppendElement(request);
     } else {
       AddDeferRequest(request);
     }
+    if (!modReq->mModuleScript->IsErrored()) {
+      StartFetchingModuleDependencies(modReq);
+    }
     return false;
   }
   request->mProgress = ScriptLoadRequest::Progress::Ready;
   if (aElement->GetParserCreated() == FROM_PARSER_XSLT &&
       (!ReadyToExecuteParserBlockingScripts() || !mXSLTRequests.isEmpty())) {
     // Need to maintain order for XSLT-inserted scripts
     NS_ASSERTION(!mParserBlockingRequest,
         "Parser-blocking scripts and XSLT scripts in the same doc!");
@@ -1469,21 +1597,17 @@ ScriptLoader::ProcessOffThreadRequest(Sc
   MOZ_ASSERT(aRequest->mProgress == ScriptLoadRequest::Progress::Compiling);
   MOZ_ASSERT(!aRequest->mWasCompiledOMT);
 
   aRequest->mWasCompiledOMT = true;
 
   if (aRequest->IsModuleRequest()) {
     MOZ_ASSERT(aRequest->mOffThreadToken);
     ModuleLoadRequest* request = aRequest->AsModuleRequest();
-    nsresult rv = ProcessFetchedModuleSource(request);
-    if (NS_FAILED(rv)) {
-      request->LoadFailed();
-    }
-    return rv;
+    return ProcessFetchedModuleSource(request);
   }
 
   aRequest->SetReady();
 
   if (aRequest == mParserBlockingRequest) {
     if (!ReadyToExecuteParserBlockingScripts()) {
       // If not ready to execute scripts, schedule an async call to
       // ProcessPendingRequests to handle it.
@@ -1668,17 +1792,17 @@ ScriptLoader::ProcessRequest(ScriptLoadR
   NS_ASSERTION(aRequest->IsReadyToRun(),
                "Processing a request that is not ready to run.");
 
   NS_ENSURE_ARG(aRequest);
 
   if (aRequest->IsModuleRequest() &&
       !aRequest->AsModuleRequest()->mModuleScript)
   {
-    // There was an error parsing a module script.  Nothing to do here.
+    // There was an error fetching a module script.  Nothing to do here.
     FireScriptAvailable(NS_ERROR_FAILURE, aRequest);
     return NS_OK;
   }
 
   nsCOMPtr<nsINode> scriptElem = do_QueryInterface(aRequest->mElement);
 
   nsCOMPtr<nsIDocument> doc;
   if (!aRequest->mIsInline) {
@@ -1977,81 +2101,94 @@ ScriptLoader::EvaluateScript(ScriptLoadR
   if (version == JSVERSION_UNKNOWN) {
     return NS_OK;
   }
 
   // New script entry point required, due to the "Create a script" sub-step of
   // http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block
   nsAutoMicroTask mt;
   AutoEntryScript aes(globalObject, "<script> element", true);
-  JS::Rooted<JSObject*> global(aes.cx(),
-                               globalObject->GetGlobalJSObject());
+  JSContext* cx = aes.cx();
+  JS::Rooted<JSObject*> global(cx, globalObject->GetGlobalJSObject());
 
   bool oldProcessingScriptTag = context->GetProcessingScriptTag();
   context->SetProcessingScriptTag(true);
   nsresult rv;
   {
     // Update our current script.
     AutoCurrentScriptUpdater scriptUpdater(this, aRequest->mElement);
 
     if (aRequest->IsModuleRequest()) {
-      rv = EnsureModuleResolveHook(aes.cx());
-      NS_ENSURE_SUCCESS(rv, rv);
-
       // When a module is already loaded, it is not feched a second time and the
       // mDataType of the request might remain set to DataType::Unknown.
       MOZ_ASSERT(!aRequest->IsBytecode());
       LOG(("ScriptLoadRequest (%p): Evaluate Module", aRequest));
+
+      rv = EnsureModuleResolveHook(cx);
+      NS_ENSURE_SUCCESS(rv, rv);
+
       ModuleLoadRequest* request = aRequest->AsModuleRequest();
       MOZ_ASSERT(request->mModuleScript);
       MOZ_ASSERT(!request->mOffThreadToken);
-      JS::Rooted<JSObject*> module(aes.cx(),
-                                   request->mModuleScript->ModuleRecord());
+
+      ModuleScript* moduleScript = request->mModuleScript;
+      if (moduleScript->IsErrored()) {
+        LOG(("ScriptLoadRequest (%p):   module is errored", aRequest));
+        JS::Rooted<JS::Value> error(cx, moduleScript->Error());
+        JS_SetPendingException(cx, error);
+        return NS_OK; // An error is reported by AutoEntryScript.
+      }
+
+      JS::Rooted<JSObject*> module(cx, moduleScript->ModuleRecord());
       MOZ_ASSERT(module);
-      rv = nsJSUtils::ModuleInstantiate(aes.cx(), module);
-      if (NS_SUCCEEDED(rv)) {
-        rv = nsJSUtils::ModuleEvaluate(aes.cx(), module);
+
+      rv = nsJSUtils::ModuleEvaluate(cx, module);
+      MOZ_ASSERT(NS_FAILED(rv) == aes.HasException());
+      if (NS_FAILED(rv)) {
+        LOG(("ScriptLoadRequest (%p):   evaluation failed", aRequest));
+        rv = NS_OK; // An error is reported by AutoEntryScript.
       }
+
       aRequest->mCacheInfo = nullptr;
     } else {
-      JS::CompileOptions options(aes.cx());
+      JS::CompileOptions options(cx);
       rv = FillCompileOptionsForRequest(aes, aRequest, global, &options);
 
       if (NS_SUCCEEDED(rv)) {
         if (aRequest->IsBytecode()) {
           TRACE_FOR_TEST(aRequest->mElement, "scriptloader_execute");
-          nsJSUtils::ExecutionContext exec(aes.cx(), global);
+          nsJSUtils::ExecutionContext exec(cx, global);
           if (aRequest->mOffThreadToken) {
             LOG(("ScriptLoadRequest (%p): Decode Bytecode & Join and Execute", aRequest));
             AutoTimer<DOM_SCRIPT_OFF_THREAD_DECODE_EXEC_MS> timer;
             rv = exec.DecodeJoinAndExec(&aRequest->mOffThreadToken);
           } else {
             LOG(("ScriptLoadRequest (%p): Decode Bytecode and Execute", aRequest));
             AutoTimer<DOM_SCRIPT_MAIN_THREAD_DECODE_EXEC_MS> timer;
             rv = exec.DecodeAndExec(options, aRequest->mScriptBytecode,
                                     aRequest->mBytecodeOffset);
           }
           // We do not expect to be saving anything when we already have some
           // bytecode.
           MOZ_ASSERT(!aRequest->mCacheInfo);
         } else {
           MOZ_ASSERT(aRequest->IsSource());
-          JS::Rooted<JSScript*> script(aes.cx());
+          JS::Rooted<JSScript*> script(cx);
           bool encodeBytecode = ShouldCacheBytecode(aRequest);
 
           TimeStamp start;
           if (Telemetry::CanRecordExtended()) {
             // Only record telemetry for scripts which are above the threshold.
             if (aRequest->mCacheInfo && aRequest->mScriptText.length() >= 1024) {
               start = TimeStamp::Now();
             }
           }
 
           {
-            nsJSUtils::ExecutionContext exec(aes.cx(), global);
+            nsJSUtils::ExecutionContext exec(cx, global);
             exec.SetEncodeBytecode(encodeBytecode);
             TRACE_FOR_TEST(aRequest->mElement, "scriptloader_execute");
             if (aRequest->mOffThreadToken) {
               // Off-main-thread parsing.
               LOG(("ScriptLoadRequest (%p): Join (off-thread parsing) and Execute",
                    aRequest));
               rv = exec.JoinAndExec(&aRequest->mOffThreadToken, &script);
               if (start) {
--- a/dom/script/ScriptLoader.h
+++ b/dom/script/ScriptLoader.h
@@ -470,17 +470,19 @@ private:
 
   // Returns wether we should save the bytecode of this script after the
   // execution of the script.
   static bool
   ShouldCacheBytecode(ScriptLoadRequest* aRequest);
 
   nsresult CreateModuleScript(ModuleLoadRequest* aRequest);
   nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest);
+  void CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest);
   void ProcessLoadedModuleTree(ModuleLoadRequest* aRequest);
+  bool InstantiateModuleTree(ModuleLoadRequest* aRequest);
   void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest);
 
   RefPtr<mozilla::GenericPromise>
   StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, nsIURI* aURI);
 
   nsIDocument* mDocument;                   // [WEAK]
   nsCOMArray<nsIScriptLoaderObserver> mObservers;
   ScriptLoadRequestList mNonAsyncExternalScriptInsertedRequests;