Module script error handling is not quite right #2567

domenic · 2017-04-20T15:11:31Z

@GeorgNeis helped walk me through several error cases:

parsing fails
parsing of dependency fails
instantiation fails
evaluation could fail
in the map, but mapping to null
in the map, but in state "errored"

There are two possible error behaviors, sending to window.onerror and sending to scriptEl.onerror. The intent is that errors "fetching" the script (or its dependencies) go to scriptEl.onerror, while errors "running" the script (or its dependencies) go to window.onerror. Here "fetching" might not involve any actual fetching if the module map already has fetch results, and "running" encompasses parsing, instantiation, and evaluation (and again might not actually do any of those things if the module map already has a result).

The current spec at the very least does not get parsing errors right. It sends them to window.onerror, but then also sends them to scriptEl.onerror, since it results in storing null in the module map.

My tentative suggestion for a fix is that we morph the instantiation state and instantiation error fields into something more general ("state" and "error"?), and if parsing fails, store the error there. Then the module map no longer contains null for parsing failures, but instead an errored script entry. We then remove the "report the exception" inside "create a module script" and let the "report the exception" inside "execute a script block" do the window.onerror work.

I still need to check that this gives the expected behavior for all six cases, but I'm optimistic.

We may also want to, either as part of this or as a separate change, move ParseScript for classic scripts to a different part of the process, to make it more symmetric with module scripts and less confusing. This is not observable since unlike module scripts there is no multi-stage process for fetching/parsing/instantiating/running a classic script, which is why the current inconsistency is not a problem. But it's still confusing for readers.

This impacts several web platform tests.

/cc @whatwg/modules

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. Fixes #2567.

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. Fixes #2567.

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 tc39/ecma262#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.

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 tc39/ecma262#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: - #2595 (comment) - #2629 - #2630 But for now the improvement given by this commit is enough to merge it.

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 tc39/ecma262#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 whatwg#2567. However, there are still problems with this setup, which may need further infrastructure changes; see: - whatwg#2595 (comment) - whatwg#2629 - whatwg#2630 But for now the improvement given by this commit is enough to merge it.

domenic added the topic: script label Apr 20, 2017

domenic self-assigned this Apr 20, 2017

domenic mentioned this issue Apr 26, 2017

Fix error cases of <script type=module> #2595

Merged

domenic closed this as completed in #2595 May 12, 2017

domenic added the topic: error reporting label Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Module script error handling is not quite right #2567

Module script error handling is not quite right #2567

domenic commented Apr 20, 2017 •

edited

Loading

Module script error handling is not quite right #2567

Module script error handling is not quite right #2567

Comments

domenic commented Apr 20, 2017 • edited Loading

domenic commented Apr 20, 2017 •

edited

Loading