[ES6 Module] Reflect Spec PR 2991 to make error handling deterministic

Spec update: whatwg/html#2991 V8-side change: https://chromium-review.googlesource.com/763369 Data members for errors are restructured: Removed module script's "is errored" #concept-module-script-is-errored => Removed ModuleScript::IsErrored(). Removed module script's "error" #concept-module-script-error => Removed Modulator::GetError(), ModuleScript::CreateError(), and ModuleScript::CreateErrorInternal(). Removed "set the pre-instantiation error" #concept-module-script-set-pre-instantiation-error => Removed ModuleScript::SetErrorAndClearRecord(). Introduced script's "error to rethrow" #concept-script-error-to-rethrow => Added ModuleScript::error_to_rethrow_ and its accessors. Also renamed ModuleScript::preinstantiation_error_ to parse_error_, and added its accessors, HTML/Blink no longer checks modules' status: Removed references to [[Status]] and [[ErrorCompletion]] field. => Removed Modulator::GetRecordStatus(), ModuleScript::RecordStatus(), and calls to ScriptModule::ErrorCompletion(). Removed module script's "has instantiated" #concept-module-script-has-instantiated => Removed ModuleScript::HasInstantiated(). A subsequent CL will do further cleanup: https://chromium-review.googlesource.com/802465 Error handling algorithms in the HTML spec were updated: - #creating-a-module-script - #run-a-module-script - [FFPE] #finding-the-first-parse-error - [IMSGF] #internal-module-script-graph-fetching-procedure - [FD] #fetch-the-descendants-of-a-module-script - [FDaI] #fetch-the-descendants-of-and-instantiate-a-module-script - #hostresolveimportedmodule(referencingscriptormodule,-specifier) And thus this CL updates the following accordingly: - ModuleScript::Create() - ModulatorImplBase::ExecuteModule() - ModuleTreeLinker.cpp - ScriptModuleResolverImpl::Resolve() The behavior changes are: - User-facing: the error reported (to window.onerror etc.) is changed and made deterministic, as intended by the spec update. - V8-facing: this CL - invokes module instantiation of a module graph with existing instantiation/evaluation errors. - invokes evaluation of a module graph with existing evaluation errors. These cases already occur, but this CL does these intentionally. Bug: 763597 Change-Id: Ia02bb484290c5c07ab27e9c966db19a6e8d2596f
web-platform-tests · Dec 18, 2017 · e7d1146 · e7d1146
1 parent 0f4a89d
commit e7d1146
Show file tree

Hide file tree

Showing 18 changed files with 229 additions and 19 deletions.
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html b/html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<title>Handling of different types of errors</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    window.log = [];
+
+    window.addEventListener("error", ev => log.push(ev.error));
+
+    const test_load = async_test(
+        "instantiation error has higher priority than evaluation error");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(log.length, 4);
+
+      // Two different parse errors from different module scripts
+      // should be reported for each <script> element.
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+
+      assert_not_equals(log[0], log[2],
+          'two different parse errors should be reported');
+    }));
+
+    function unreachable() { log.push("unexpected"); }
+</script>
+<script type="module" src="./choice-of-error-1a.js"
+    onerror="unreachable()" onload="log.push(1)"></script>
+<script type="module" src="./choice-of-error-1b.js"
+    onerror="unreachable()" onload="log.push(2)"></script>
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-1a.js b/html/semantics/scripting-1/the-script-element/module/choice-of-error-1a.js
@@ -0,0 +1,2 @@
+import './choice-of-error-1b.js';
+import './errorhandling-parseerror-dependent.js?1c';
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-1b.js b/html/semantics/scripting-1/the-script-element/module/choice-of-error-1b.js
@@ -0,0 +1,2 @@
+import './choice-of-error-1a.js';
+import './errorhandling-parseerror-dependent.js?1d';
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-2.html b/html/semantics/scripting-1/the-script-element/module/choice-of-error-2.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<title>Handling of different types of errors</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    window.log = [];
+
+    window.addEventListener("error", ev => log.push(ev.error));
+
+    const test_load = async_test(
+        "instantiation error has higher priority than evaluation error");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(log.length, 4);
+
+      // Two different instantiation errors from different module scripts
+      // should be reported for each <script> element.
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+
+      assert_not_equals(log[0], log[2],
+          'two different instantiation errors should be reported');
+    }));
+
+    function unreachable() { log.push("unexpected"); }
+</script>
+<script type="module" src="./choice-of-error-2a.js"
+    onerror="unreachable()" onload="log.push(1)"></script>
+<script type="module" src="./choice-of-error-2b.js"
+    onerror="unreachable()" onload="log.push(2)"></script>
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-2a.js b/html/semantics/scripting-1/the-script-element/module/choice-of-error-2a.js
@@ -0,0 +1,2 @@
+import './choice-of-error-2b.js';
+import './instantiation-error-1.js?2c';
diff --git a/html/semantics/scripting-1/the-script-element/module/choice-of-error-2b.js b/html/semantics/scripting-1/the-script-element/module/choice-of-error-2b.js
@@ -0,0 +1,2 @@
+import './choice-of-error-2a.js';
+import './instantiation-error-1.js?2d';
diff --git a/...cs/scripting-1/the-script-element/module/dynamic-import/dynamic-imports-script-error.html b/...cs/scripting-1/the-script-element/module/dynamic-import/dynamic-imports-script-error.html
@@ -9,7 +9,7 @@
   ["parse error", "../syntaxerror.js", new SyntaxError],
   ["bad module specifier", "does-not-start-with-dot.js", new TypeError, { differentErrorObjects: true }],
   ["bad module specifier in a dependency", "../bad-module-specifier.js", new TypeError],
-  ["instantiation error", "../instantiation-error-1.js", new SyntaxError],
+  ["instantiation error", "../instantiation-error-1.js", new SyntaxError, { differentErrorObjects: true }],
   ["evaluation error", "../throw-error.js", new Error]
 ];
 

diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-1.html b/html/semantics/scripting-1/the-script-element/module/error-type-1.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<title>Handling of different types of errors</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    window.log = [];
+
+    window.addEventListener("error", ev => log.push(ev.error));
+
+    const test_load = async_test(
+        "network error has higher priority than parse error");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(log.length, 3);
+
+      // A parse error is reported for the first top-level
+      // <script> element for errorhandling-parseerror-dependent.js.
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+
+      // onerror is called (with no errors reported) due to a network error
+      // for the second top-level <script>.
+      assert_equals(log[2], 2);
+    }));
+
+    function unreachable() { log.push("unexpected"); }
+</script>
+<script type="module" src="./errorhandling-parseerror-dependent.js"
+    onerror="unreachable()" onload="log.push(1)"></script>
+<script type="module" src="./error-type-1.js"
+    onerror="log.push(2)" onload="unreachable()"></script>
diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-1.js b/html/semantics/scripting-1/the-script-element/module/error-type-1.js
@@ -0,0 +1,2 @@
+import './errorhandling-parseerror-dependent.js';
+import './404.js';
diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-2.html b/html/semantics/scripting-1/the-script-element/module/error-type-2.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<title>Handling of different types of errors</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    window.log = [];
+
+    window.addEventListener("error", ev => log.push(ev.error));
+
+    const test_load = async_test(
+        "parse error has higher priority than instantiation error");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(log.length, 4);
+
+      // An instantiation error is reported for the first top-level
+      // <script> element for instantiation-error-1.js.
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+
+      // A parse error is reported for the second top-level <script>.
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+      assert_not_equals(log[0], log[2]);
+    }));
+
+    function unreachable() { log.push("unexpected"); }
+</script>
+<script type="module" src="./instantiation-error-1.js"
+    onerror="unreachable()" onload="log.push(1)"></script>
+<script type="module" src="./error-type-2.js"
+    onerror="unreachable()" onload="log.push(2)"></script>
diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-2.js b/html/semantics/scripting-1/the-script-element/module/error-type-2.js
@@ -0,0 +1,2 @@
+import './instantiation-error-1.js';
+import './errorhandling-parseerror-dependent.js';
diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-3.html b/html/semantics/scripting-1/the-script-element/module/error-type-3.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<title>Handling of different types of errors</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    window.log = [];
+
+    window.addEventListener("error", ev => log.push(ev.error));
+
+    const test_load = async_test(
+        "instantiation error has higher priority than evaluation error");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(log.length, 5);
+
+      // An evaluation error is reported for the first top-level
+      // <script> element for throw.js.
+      assert_equals(log[0], 'throw');
+      assert_true(log[1].foo);
+      assert_equals(log[2], 1);
+
+      // An instantiation error is reported for the second top-level <script>.
+      assert_equals(log[3].constructor, SyntaxError);
+      assert_equals(log[4], 2);
+    }));
+
+    function unreachable() { log.push("unexpected"); }
+</script>
+<script type="module" src="./throw.js"
+    onerror="unreachable()" onload="log.push(1)"></script>
+<script type="module" src="./error-type-3.js"
+    onerror="unreachable()" onload="log.push(2)"></script>
diff --git a/html/semantics/scripting-1/the-script-element/module/error-type-3.js b/html/semantics/scripting-1/the-script-element/module/error-type-3.js
@@ -0,0 +1,2 @@
+import './throw.js';
+import './instantiation-error-1.js';
diff --git a/html/semantics/scripting-1/the-script-element/module/instantiation-error-1.html b/html/semantics/scripting-1/the-script-element/module/instantiation-error-1.html
@@ -12,11 +12,19 @@
 
     const test_load = async_test(
         "Test that missing exports lead to SyntaxError events on window and " +
-        "load events on script, and that exceptions are remembered");
+        "load events on script");
     window.addEventListener("load", test_load.step_func_done(ev => {
-      const exn = log[0];
-      assert_array_equals(log, [exn, 1, exn, 2, exn, 3, exn, 4, exn, 5]);
-      assert_equals(exn.constructor, SyntaxError);
+      assert_equals(log.length, 10);
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+      assert_equals(log[4].constructor, SyntaxError);
+      assert_equals(log[5], 3);
+      assert_equals(log[6].constructor, SyntaxError);
+      assert_equals(log[7], 4);
+      assert_equals(log[8].constructor, SyntaxError);
+      assert_equals(log[9], 5);
     }));
 
     function unreachable() { log.push("unexpected"); }

diff --git a/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html b/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html
@@ -12,11 +12,19 @@
 
     const test_load = async_test(
         "Test that missing exports lead to SyntaxError events on window and " +
-        "load events on script, and that exceptions are remembered");
+        "load events on script");
     window.addEventListener("load", test_load.step_func_done(ev => {
-      const exn = log[0];
-      assert_array_equals(log, [exn, 1, exn, 2, exn, 3, exn, 4, exn, 5]);
-      assert_equals(exn.constructor, SyntaxError);
+      assert_equals(log.length, 10);
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+      assert_equals(log[4].constructor, SyntaxError);
+      assert_equals(log[5], 3);
+      assert_equals(log[6].constructor, SyntaxError);
+      assert_equals(log[7], 4);
+      assert_equals(log[8].constructor, SyntaxError);
+      assert_equals(log[9], 5);
     }));
 
     function unreachable() { log.push("unexpected"); }

diff --git a/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html b/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html
@@ -12,11 +12,15 @@
 
     const test_load = async_test(
         "Test that unresolvable cycles lead to SyntaxError events on window " +
-        "and load events on script, and that exceptions are remembered");
+        "and load events on script");
     window.addEventListener("load", test_load.step_func_done(ev => {
-      const exn = log[0];
-      assert_array_equals(log, [exn, 1, exn, 2, exn, 3]);
-      assert_equals(exn.constructor, SyntaxError);
+      assert_equals(log.length, 6);
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+      assert_equals(log[4].constructor, SyntaxError);
+      assert_equals(log[5], 3);
     }));
 
     function unreachable() { log.push("unexpected"); }

diff --git a/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html b/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html
@@ -14,9 +14,11 @@
         "Test that loading a graph in which a module is already " +
         "errored results in that module's error.");
     window.addEventListener("load", test_load.step_func_done(ev => {
-      const exn = log[0];
-      assert_array_equals(log, [exn, 1, exn, 2]);
-      assert_equals(exn.constructor, SyntaxError);
+      assert_equals(log.length, 4);
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
     }));
 
     function unreachable() { log.push("unexpected"); }

diff --git a/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html b/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html
@@ -14,9 +14,12 @@
         "Test that loading a graph in which a module is already " +
         "errored results in that module's error.");
     window.addEventListener("load", test_load.step_func_done(ev => {
-      const exn = log[0];
-      assert_array_equals(log, [exn, 1, exn, 2]);
-      assert_equals(exn.constructor, SyntaxError);
+      assert_equals(log.length, 4);
+      assert_equals(log[0].constructor, SyntaxError);
+      assert_equals(log[1], 1);
+      assert_equals(log[2].constructor, SyntaxError);
+      assert_equals(log[3], 2);
+      assert_not_equals(log[0], log[2], "errors should be different");
     }));
 
     function unreachable() { log.push("unexpected"); }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './choice-of-error-1b.js';
		import './errorhandling-parseerror-dependent.js?1c';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './choice-of-error-1a.js';
		import './errorhandling-parseerror-dependent.js?1d';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './choice-of-error-2b.js';
		import './instantiation-error-1.js?2c';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './choice-of-error-2a.js';
		import './instantiation-error-1.js?2d';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './errorhandling-parseerror-dependent.js';
		import './404.js';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './instantiation-error-1.js';
		import './errorhandling-parseerror-dependent.js';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import './throw.js';
		import './instantiation-error-1.js';