WICG / webcomponents

Web Components specifications
Other
4.38k stars 371 forks source link

Clarify "report an exception" for Custom Elements #635

Open EdgarChen opened 7 years ago

EdgarChen commented 7 years ago

The latest plan in https://github.com/whatwg/html/issues/958 doesn't cover all custom element cases, so I filed this issue to clarify them. Please let me know if I should move this discussion to https://github.com/whatwg/html/issues/958, or other places.

custom element constructor

There are two places will call custom element constructor:

  1. Step 7.1 of upgrade an element.
  2. Step 6.2. of create an element.

The exception thrown from the constructor will get taken care of automatically by the guarded flag, right?

upgrade reactions

Operations, attributes, setters, or deleters annotated with the [CEReactions] extended attribute will possibly invoke upgrade reaction after executing the listed actions. e.g.,

How to report the exception thrown from upgrade (i.e., Step 7.2) is not defined in latest plan.

Custom element upgrade reactions will need a bit of care; will have to think on it a bit more.

create an element

There are some places that run create an element with the synchronous custom elements flag set,

  1. document.createElement() (Step 5)
  2. document.createElementNS() (Step 4 of internal createElementNS steps)
  3. create element for token with execute script is true (Step 7)

For 3), the exceptions will be reported to the document's window (since it is during parsing), right? What about the 1) and 2)?

/cc @domenic.

domenic commented 7 years ago

The exception thrown from the constructor will get taken care of automatically by the guarded flag, right?

In this case we will unset the guarded flag, because as noted in those algorithms, we catch and handle exceptions specifically.

https://html.spec.whatwg.org/multipage/scripting.html#invoke-custom-element-reactions

This will work as today; "If this throws any exception, then report the exception." It's not like we're removing the ability to report exceptions...

For 3), the exceptions will be reported to the document's window (since it is during parsing), right? What about the 1) and 2)?

Again, the spec is very clear on this today. The "create an element" algorithm explicitly catches exceptions and reports them. That'll still work fine.


In general, nothing about the plan on whatwg/html#958 will change the behavior of the custom elements spec. It might let us shorten some of the steps by e.g. using the guarded flag, but probably we'll just leave them alone.

EdgarChen commented 7 years ago

Thanks for your replying and sorry for not being clearer. I didn't mean to remove the ability to report exceptions. My intention is to clarify which global will be passed to "reporting an exception" regarding the latest plan.

domenic commented 7 years ago

Oh, I see, sorry for not understanding. I agree this is not specified well and not clear from the plan in whatwg/html#958.

Unfortunately I don't have a great answer, because I still haven't become comfortable with @bzbarsky's suggestion that we use entry global as the default for error reporting. This is simply because I haven't had time to get back to that general problem area and come up with any alternatives that I can compare with his proposal.

If we end up going with his suggestion, then probably we will use entry global in those places that are called from JS APIs, and the node document's global object for the parser case. But I am still not sure that entry global is the right way to go for such things... it might be the way to go for now though, at least in Firefox, since it sounds like that is what usually happens in Firefox from previous discussions with @bzbarsky.

bzbarsky commented 7 years ago

If I were implementing it, I would use the entry global of the reaction invocation (as in, the global of the reaction callback) or so...

bzbarsky commented 7 years ago

Or put another way, reporting an exception on the global of the caller of the JS API without giving code from that global a chance to catch that exception is pretty odd behavior. So if someone does appendChild we don't want to use the entry global of the appendChild call, imo. We want to use the entry global for the call back out into JS to handle the CE reaction.

bzbarsky commented 7 years ago

There's on complication around this, which is the exception that https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element step 7.2 throws. This is happening outside the normal callback machinery, so wouldn't just automatically pick up webidl callback error reporting. But other than that complication, is there a reason we can't use normal webidl callback error reporting for everything else?

For that SameValue check, ideally we would treat it like we would treat an exception from C itself, in terms of reporting...