From 101fe60bb598d2ae14e74795df6dffa58e2756e2 Mon Sep 17 00:00:00 2001
From: Domenic Denicola <d@domenic.me>
Date: Fri, 12 May 2017 12:50:49 -0400
Subject: [PATCH] Fix error cases of <script type=module>

There are several different ways things can go wrong with
<script type=module>. In order from earliest to latest, for a single
module script, they are:

- Fetching failures
- Parsing failures
- Invalid module specifiers
- Instantiation failures
- Evaluation failures

This tweaks the way that these errors interact, to ensure that fetching
failures are treated one way, causing the <script>'s "error" event to
fire, and the other failures are uniformly treated as errors running the
script, causing the global's "error" event to fire.

This also makes it clear that when fetching descendant module scripts,
you can bail out early if one of them fails to fetch or has previously
been discovered to be errored.

Evaluation failures are particularly tricky for dependencies, as usually
ModuleEvaluation() is a no-op if it has happened previously
(either successfully or not). This is discussed in more depth in
https://github.com/tc39/ecma262/issues/862. To work around this, we need
to store the evaluation error, if one occurs, similar to what we already
do for instantiation errors.

Fixes #2567.

However, there are still problems with this setup, which may need
further infrastructure changes; see:

- https://github.com/whatwg/html/pull/2595#issuecomment-299242321
- https://github.com/whatwg/html/issues/2629
- https://github.com/whatwg/html/issues/2630

But for now the improvement given by this commit is enough to merge it.
---
 source | 201 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 121 insertions(+), 80 deletions(-)

diff --git a/source b/source
index e3efa4da421..a64f61a25bc 100644
--- a/source
+++ b/source
@@ -86557,24 +86557,24 @@ interface <dfn>NavigatorOnLine</dfn> {
 
    </dd>
 
-   <dt>An <dfn data-x="concept-module-script-instantiation-state">instantiation state</dfn></dt>
+   <dt>A <dfn data-x="concept-module-script-state">state</dfn></dt>
 
    <dd>
 
     <p>One of "<code data-x="">uninstantiated</code>", "<code data-x="">errored</code>", or "<code
     data-x="">instantiated</code>", used to prevent reinvocation of <span
     data-x="js-ModuleDeclarationInstantiation">ModuleDeclarationInstantiation</span> on modules that
-    failed to instantiate previously.</p>
+    failed to instantiate previously, and to ensure errors during parsing, instantiation, or
+    evaluation are remembered and propagated correctly.</p>
 
    </dd>
 
-   <dt>An <dfn data-x="concept-module-script-instantiation-error">instantiation error</dfn></dt>
+   <dt>An <dfn data-x="concept-module-script-error">error</dfn></dt>
 
    <dd>
 
     <p>A JavaScript value, which has meaning only if the <span
-    data-x="concept-module-script-instantiation-state">instantiation state</span> is "<code
-    data-x="">errored</code>".</p>
+    data-x="concept-module-script-state">state</span> is "<code data-x="">errored</code>".</p>
 
    </dd>
 
@@ -86607,6 +86607,33 @@ interface <dfn>NavigatorOnLine</dfn> {
 
   </dl>
 
+  <p>To <dfn>error a module script</dfn> <var>script</var> with a given value <var>error</var>,
+  perform the following steps:</p>
+
+  <ol>
+   <li><p>Assert: <var>script</var>'s <span data-x="concept-module-script-state">state</span> is not
+   "<code data-x="">errored</code>".</p></li>
+
+   <li>
+    <p>If <var>script</var>'s <span data-x="concept-module-script-module-record">module
+    record</span> is set, then:</p>
+
+    <ol>
+     <li><p>Set <var>script</var> <span data-x="concept-module-script-module-record">module
+     record</span>'s [[HostDefined]] field to undefined.</p></li>
+
+     <li><p>Set <var>script</var>'s <span data-x="concept-module-script-module-record">module
+     record</span> to null.</p></li>
+    </ol>
+   </li>
+
+   <li><p>Set <var>script</var>'s <span data-x="concept-module-script-state">state</span> to
+   "<code data-x="">errored</code>".</p></li>
+
+   <li><p>Set <var>script</var>'s <span data-x="concept-module-script-error">error</span> to
+   <var>error</var>.</p></li>
+  </ol>
+
   <hr>
 
   <p>An <dfn data-export="">environment</dfn> is an object that identifies the settings of a
@@ -87041,13 +87068,19 @@ interface <dfn>NavigatorOnLine</dfn> {
    <li><p>If <var>result</var> is null, asynchronously complete this algorithm with null and abort
    these steps.</p></li>
 
+   <li><p>If <var>result</var>'s <span data-x="concept-module-script-state">state</span> is "<code
+   data-x="">instantiated</code>" or "<code data-x="">errored</code>", asynchronously complete this
+   algorithm with <var>result</var>, and abort these steps.</p></li>
+
+   <li><p>Assert: <var>result</var>'s <span data-x="concept-module-script-state">state</span> is
+   "<code data-x="">uninstantiated</code>".</p></li>
+
    <li>
-    <p>Otherwise, <var>result</var> is a <span>module script</span>. <span data-x="fetch the
-    descendants of a module script">Fetch the descendants</span> of <var>result</var> given
-    <var>destination</var> and an ancestor list obtained by appending <var>url</var> to <var>ancestor
-    list</var>. Wait for <span data-x="fetch the descendants of a module script">fetching the
-    descendants of a module script</span> to asynchronously complete with <var>descendants
-    result</var> before proceeding to the next step.</p>
+    <p><span data-x="fetch the descendants of a module script">Fetch the
+    descendants</span> of <var>result</var> given <var>destination</var> and an ancestor list
+    obtained by appending <var>url</var> to <var>ancestor list</var>. Wait for <span data-x="fetch
+    the descendants of a module script">fetching the descendants of a module script</span> to
+    asynchronously complete with <var>descendants result</var> before proceeding to the next step.</p>
 
     <p class="note">If the asynchronous completion result is null, meaning that fetching one of the
     descendants failed, we still proceed through the next set of steps. A failure will shortly occur
@@ -87055,59 +87088,29 @@ interface <dfn>NavigatorOnLine</dfn> {
     propagated to the caller of this algorithm in the last step.</p>
    </li>
 
-   <li><p>Let <var>instantiationStatus</var> be null.</p></li>
-
-   <li><p>If <var>result</var>'s <span data-x="concept-module-script-instantiation-state">instantiation
-   state</span> is "<code data-x="">errored</code>", then set <var>instantiationStatus</var> to
-   Completion { [[Type]]: throw, [[Value]]: <var>result</var>'s <span
-   data-x="concept-module-script-instantiation-error">instantiation error</span>, [[Target]]:
-   empty }.</p></li>
+   <li><p>Let <var>record</var> be <var>result</var>'s <span
+   data-x="concept-module-script-module-record">module record</span>.</p></li>
 
    <li>
-    <p>Otherwise:</p>
-
-    <ol>
-     <li><p>Let <var>record</var> be <var>result</var>'s <span
-     data-x="concept-module-script-module-record">module record</span>.</p></li>
-
-     <li>
-      <p>Set <var>instantiationStatus</var> to <var>record</var>.<span
-      data-x="js-ModuleDeclarationInstantiation">ModuleDeclarationInstantiation</span>().</p>
+    <p>Let <var>instantiationStatus</var> be <var>record</var>.<span
+    data-x="js-ModuleDeclarationInstantiation">ModuleDeclarationInstantiation</span>().</p>
 
-      <p class="note">This step will recursively call <span
-      data-x="js-ModuleDeclarationInstantiation">ModuleDeclarationInstantiation</span> all of the
-      module's uninstantiated dependencies.</p>
-     </li>
-    </ol>
+    <p class="note">This step will recursively call <span
+    data-x="js-ModuleDeclarationInstantiation">ModuleDeclarationInstantiation</span> all of the
+    module's uninstantiated dependencies.</p>
    </li>
 
    <li>
-    <p>For each <var>script</var> in <var>result</var>'s <span>uninstantiated inclusive descendant module
-    scripts</span>, perform the following steps:</p>
+    <p>For each <var>script</var> in <var>result</var>'s <span>uninstantiated inclusive descendant
+    module scripts</span>, perform the following steps:</p>
 
     <ol>
-     <li>
-      <p>If <var>instantiationStatus</var> is an abrupt completion, then:</p>
-
-      <ol>
-       <li><p>Set <var>script</var> <span data-x="concept-module-script-module-record">module
-       record</span>'s [[HostDefined]] field to undefined.</p></li>
-
-       <li><p>Set <var>script</var>'s <span data-x="concept-module-script-module-record">module
-       record</span> to null.</p></li>
-
-       <li><p>Set <var>script</var>'s <span
-       data-x="concept-module-script-instantiation-state">instantiation state</span> to "<code
-       data-x="">errored</code>".</p></li>
-
-       <li><p>Set <var>script</var>'s <span
-       data-x="concept-module-script-instantiation-error">instantiation error</span> to
-       <var>instantiationStatus</var>.[[Value]].</p></li>
-      </ol>
-     </li>
+     <li><p>If <var>instantiationStatus</var> is an abrupt completion, then <span data-x="error a
+     module script">error</span> <var>script</var> with
+     <var>instantiationStatus</var>.[[Value]].</p></li>
 
      <li><p>Otherwise, set <var>script</var>'s <span
-     data-x="concept-module-script-instantiation-state">instantiation state</span> to "<code
+     data-x="concept-module-script-state">state</span> to "<code
      data-x="">instantiated</code>".</p></li>
     </ol>
    </li>
@@ -87125,6 +87128,9 @@ interface <dfn>NavigatorOnLine</dfn> {
   data-x="module script">module scripts</span> determined as follows:</p>
 
   <ol>
+   <li><p>If <var>script</var>'s <span data-x="concept-module-script-module-record">module
+   record</span> is null, return the empty set.</p></li>
+
    <li><p>Let <var>moduleMap</var> be <var>script</var>'s <span>settings object</span>'s
    <span data-x="concept-settings-object-module-map">module map</span>.</p></li>
 
@@ -87183,7 +87189,7 @@ interface <dfn>NavigatorOnLine</dfn> {
    </li>
 
    <li><p>Return a <span>set</span> containing all items of <var>inclusive descendants</var> whose
-   <span data-x="concept-module-script-instantiation-state">instantiation state</span> is "<code
+   <span data-x="concept-module-script-state">state</span> is "<code
    data-x="">uninstantiated</code>".</p></li>
   </ol>
 
@@ -87316,10 +87322,11 @@ interface <dfn>NavigatorOnLine</dfn> {
       <ol>
        <li><p>Let <var>error</var> be a new <code>TypeError</code> exception.</p></li>
 
-       <li><p><span>Report the exception</span> <var>error</var> for <var>module
-       script</var>.</p></li>
+       <li><p><span data-x="error a module script">Error</span> <var>module script</var> with
+       <var>error</var>.</p></li>
 
-       <li><p>Abort this algorithm, and asynchronously complete it with null.</p></li>
+       <li><p>Abort this algorithm, and asynchronously complete it with <var>module
+       script</var>.</p></li>
       </ol>
      </li>
 
@@ -87342,10 +87349,25 @@ interface <dfn>NavigatorOnLine</dfn> {
     <span data-x="fetching-scripts-perform-fetch">perform the fetch</span> steps, pass those along
     while performing the <span>internal module script graph fetching procedure</span>.</p>
 
-    <p>Wait for all of the <span>internal module script graph fetching procedure</span> invocations
-    to asynchronously complete. If any of them asynchronously complete with null, then
-    asynchronously complete this algorithm with null. Otherwise, asynchronously complete this
-    algorithm with <var>module script</var>.</p>
+    <p>These invocations of the <span>internal module script graph fetching procedure</span> should
+    be performed in parallel to each other.</p>
+
+    <p>If any invocation of the <span>internal module script graph fetching procedure</span>
+    asynchronously completes with null, optionally abort all other invocations, and then
+    asynchronously complete this algorithm with null.</p>
+
+    <p>If any invocation of the <span>internal module script graph fetching procedure</span>
+    asynchronously completes with a <span>module script</span> whose <span
+    data-x="concept-module-script-state">state</span> is "<code data-x="">errored</code>",
+    optionally abort all other invocations, and then asynchronously complete this algorithm with
+    <var>module script</var>. (The errored descendant will cause errors later in the module-fetching
+    process, but for now we treat it as a premature "success".)</p>
+
+    <p>Otherwise, wait for all of the <span>internal module script graph fetching procedure</span>
+    invocations to asynchronously complete, with <span data-x="module script">module scripts</span>
+    whose <span data-x="concept-module-script-state">states</span> are not "<code
+    data-x="">errored</code>". Then, asynchronously complete this algorithm with <var>module
+    script</var>.</p>
    </li>
   </ol>
 
@@ -87406,9 +87428,19 @@ interface <dfn>NavigatorOnLine</dfn> {
     <var>result</var>.[[HostDefined]] will be <var>script</var>.</p>
    </li>
 
-   <li><p>If <var>result</var> is a <span>List</span> of errors, <span>report the exception</span>
-   given by the first element of <var>result</var> for <var>script</var>, return null, and abort
-   these steps.</p></li>
+   <li>
+    <p>If <var>result</var> is a <span>List</span> of errors, then:</p>
+
+    <ol>
+     <li><p><span data-x="error a module script">Error</span> <var>script</var> with
+     <var>errors</var>[0].</p></li>
+
+     <li><p>Return <var>script</var>.</p></li>
+    </ol>
+   </li>
+
+   <li><p>Set <var>script</var>'s <span data-x="concept-module-script-state">state</span> to "<code
+   data-x="">uninstantiated</code>".</p></li>
 
    <li><p>Set <var>script</var>'s <span data-x="concept-module-script-module-record">module
    record</span> to <var>result</var>.</p></li>
@@ -87506,14 +87538,12 @@ interface <dfn>NavigatorOnLine</dfn> {
    <li><p><span>Check if we can run script</span> with <var>settings</var>. If this returns "do
    not run" then abort these steps.</p></li>
 
-   <li><p>If <var>s</var>'s <span data-x="concept-module-script-instantiation-state">instantiation
-   state</span> is "<code data-x="">errored</code>", then <span>report the
-   exception</span> given by <var>s</var>'s <span
-   data-x="concept-module-script-instantiation-error">instantiation error</span> for <var>s</var>
-   and abort these steps.</p></li>
+   <li><p>If <var>s</var>'s <span data-x="concept-module-script-state">state</span> is "<code
+   data-x="">errored</code>", then <span>report the exception</span> given by <var>s</var>'s <span
+   data-x="concept-module-script-error">error</span> for <var>s</var> and abort these
+   steps.</p></li>
 
-   <li><p>Assert: <var>s</var>'s <span
-   data-x="concept-module-script-instantiation-state">instantiation state</span> is "<code
+   <li><p>Assert: <var>s</var>'s <span data-x="concept-module-script-state">state</span> is "<code
    data-x="">instantiated</code>" (and thus its <span
    data-x="concept-module-script-module-record">module record</span> is not null).</p></li>
 
@@ -87527,13 +87557,24 @@ interface <dfn>NavigatorOnLine</dfn> {
     data-x="js-ModuleEvaluation">ModuleEvaluation</span>().</p>
 
     <p class="note">This step will recursively evaluate all of the module's dependencies.</p>
+
+    <p>If <span data-x="js-ModuleEvaluation">ModuleEvaluation</span> fails to complete as a result
+    of the user agent <span data-x="abort a running script">aborting the running script</span>, then
+    set <var>evaluationStatus</var> to Completion { [[Type]]: throw, [[Value]]: a new
+    <span>"<code>QuotaExceededError</code>"</span> <code>DOMException</code>, [[Target]]: empty }.
    </li>
 
-   <li><p>If <var>evaluationStatus</var> is an abrupt completion, then <span>report the
-   exception</span> given by <var>evaluationStatus</var>.[[Value]] for <var>s</var>. (Do not perform
-   this step if <span data-x="js-ModuleEvaluation">ModuleEvaluation</span> fails to complete as a
-   result of the user agent <span data-x="abort a running script">aborting the running
-   script</span>.)</p></li>
+   <li>
+    <p>If <var>evaluationStatus</var> is an abrupt completion, then:</p>
+
+    <ol>
+     <li><p><span data-x="error a module script">Error</span> <var>script</var> with
+     <var>evaluationStatus</var>.[[Value]].</p></li>
+
+     <li><p><span>Report the exception</span> given by <var>evaluationStatus</var>.[[Value]] for
+     <var>s</var>.</p>
+    </ol>
+   </li>
 
    <li><p><span>Clean up after running script</span> with <var>settings</var>.</p></li>
   </ol>
@@ -88389,9 +88430,9 @@ import "https://example.com/foo/../module2.js";</pre>
    steps.</p></li>
 
    <li><p>If <var>resolved module script</var>'s <span
-   data-x="concept-module-script-instantiation-state">instantiation state</span> is "<code
-   data-x="">errored</code>", then throw <var>resolved module script</var>'s <span
-   data-x="concept-module-script-instantiation-error">instantiation error</span>.</p></li>
+   data-x="concept-module-script-state">state</span> is "<code data-x="">errored</code>", then throw
+   <var>resolved module script</var>'s <span
+   data-x="concept-module-script-error">error</span>.</p></li>
 
    <li><p>Assert: <var>resolved module script</var>'s <span
    data-x="concept-module-script-module-record">module record</span> is not null.</p></li>