Clarify "report an exception"

[Ms2ger]

the user agent must report the error for the relevant script

What is "the relevant script"?

---

Edit by @domenic: the current plan to be executed in order to fix this bug is #958 (comment). Comments between here and there are by now somewhat misleading and historical

[Ms2ger]

In particular in relation to the tests in https://bugzilla.mozilla.org/show_bug.cgi?id=1259784

[domenic]

I at first thought this was best solved by having "report an exception" explicitly specify which global the error goes to. However, I realize that doesn't really work, as we actually need the information on which script caused the exception, for filename (and line/column number) purposes.

I am pretty sure that the relevant script should generally be GetActiveScriptOrModule's [[HostDefined]] value (i.e. its corresponding HTML "script").

As discussed in the new #1189, there are scenarios where this returns null. Maybe this means that #1189 needs to expand to have a "backup incumbent script stack", not a "backup incumbent settings object stack".

/cc @bzbarsky.

[domenic]

Dang, nope, that makes no sense, because of cases like the user clicking on a button with throwing error handlers. In that case there's no GetActiveScriptOrModule() and no backup possible while inside the "dispatch an event" algorithm. Nevermind...

[bzbarsky]

The way this was meant to work, which everyone agreed on back when this was last discussed and when Hixie wrote the spec for this, before the later changes to it was like so:

1. When invoking callbacks the entry settings represent the thing being invoked and the incumbent settings represent the entity that added the callback.
2. Error reporting in the simple window1.setTimeout(functionFromWindow2) case should happen on window2, not window1. Conveniently, that corresponds to the entry settings object for the call in that case.

At the time, it seemed to me that the "invoking callbacks" section in HTML made this all fairly clear. It looks like that's all been removed since then, so now nothing specifies things... It doesn't help (but is far for the course) that some UAs agreed on all the above at the time and then never actually changed their behavior to align with the resulting spec.

[bzbarsky]

The function that threw an exception may not have been declared in a script at all. It can be a built-in function.

[bzbarsky]

And I guess it's possible that after the various discussion about this stuff things never made it back into the spec after all. :( I distinctly recall there being language proposed that made all this much clearer than what you link to. :(

[domenic]

For posterity:

@bzbarsky and I worked out a plan for fixing this in whatwg/webidl#113 (comment) (steps 2-4) plus @bzbarsky's subsequent reply (on which I agree with all points).

I've explained where this fits in my priority queue of "fix script execution" at the bottom of whatwg/webidl#113 (comment).

[domenic]

The plan:

Fix "report the exception"/"report an error"

Happening in #10404.

The goal is to consolidate both of the existing algorithms "report the exception" and "report an error" into a single new algorithm, and update all call sites.

• The new algorithm's name will be "report an exception". (This is intentionally slightly different from both old algorithms. We will break spec cross-references in this way, to help move the ecosystem, but we can preserve the IDs so that actual links are not broken.)
• Its arguments will be an exception value, a global object, and an optional script
• We can get rid of the "handled" and "not handled" concepts, and instead include the line about reporting the error to the developer console directly inside the algorithm.

The contents of this algorithm will be based on "report an error", with the following notable differences:

• message, line, col, and filename are derived from the exception value, in an unspecified way. This allows the better results that in practice browsers exhibit for e.g. eval code. In the future, this may become specified in detail when the JavaScript spec specifies error stack traces in detail.

• The optional script will be used to determine muting (and not to determine the filename). If it is not supplied, assume the error is not muted.

• As noted above, incorporate the handled/not handled action directly into the algorithm.

Then, update all call sites of these two algorithms. Only do this for HTML for now, and it would be done in the same PR that updates the algorithm.

Port these changes to rejected promises

Now tracked in #10515.

This might be best done as part of the previous PR. Essentially, get rid of the "handled" and "not handled" concepts, and incorporate the developer console part directly into the algorithm.

Change how web developer code invocation works in Web IDL

Now tracked in whatwg/webidl#1423.

We add a new optional named parameter rethrowExceptions (a boolean) to the following three algorithms in the Web IDL spec:

• https://webidl.spec.whatwg.org/#call-a-user-objects-operation
• https://webidl.spec.whatwg.org/#invoke-a-callback-function
• https://webidl.spec.whatwg.org/#construct-a-callback-function

It defaults to false. When it is false, we introduce the following new behavior: any exceptions thrown by web developer code get caught and routed to HTML's new "report the exception". ("Web developer code" is roughly everything between the "prepare" and "cleanup" steps, including both argument conversion and the actual code invocation.)

The exact global and script to pass for these is unclear at this time. We can initially leave them as <span class="XXX">unclear which global</span>, <span class="XXX">unclear which script</span> and work on that in later steps.

Fix all call sites

Tracked as part of whatwg/webidl#1423 and #10516.

Now we need to audit all the call sites of the algorithms we've changed:

• Report the exception
• Invoke
• Call a user object's operation
• Construct

In most cases, the pattern is that another spec is using invoke (or friends), then catching exceptions and routing them to "report the exception". After the above changes, this can be simplified to just calling invoke; the exception will be reported automatically through the shared infrastructure.

If another spec is using invoke and not catching exceptions, this might be an unintentional bug. In that case, we can leave it as-is. Or it might be intentional that they want the exception to bubble out, or be caught with a different reaction than reporting the exception. In that case the spec will need to be updated to pass the new rethrowExceptions parameter.

If another spec is directly using "report the exception", it will need to be given the appropriate new arguments, and possibly update its return value handling.

Further work on nailing down the specifics

Tracked in #10514 for muting and to-be-filed for which global.

At this point we've got good infrastructure, centralized in HTML and Web IDL. But a few core specifics are still unclear:

• Which global do we report exceptions to?
• How does error muting work?

The way to resolve these involves writing web platform tests, and/or code inspection of browser codebases. There are likely interop problems, especially for muting as discussed downthread. A test suite to illustrate the problems is step 1; after that we can discuss the desired resolution.

Any work on this sub-task feels like a bonus to me. If we accomplish the mostly-editorial refactoring above, we can split this work into new issues.