Handle exceptions when calling callbacks by default

[domenic]

What is the issue with the Web IDL Standard?

Splitting from https://github.com/whatwg/html/issues/958#issuecomment-226309871.

After https://github.com/whatwg/html/pull/10404 lands, HTML will have a solid foundation for reporting exceptions.

It turns out that most cases, when spec writers use Web IDL to construct or invoke callback functions, or to call a user object's operation, they want errors to be reported to the global error handler; they don't want them to propagate out. Such reporting is a safer default.

We should add a new optional named parameter rethrowExceptions (default false) to these definitions. 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.)

Which global do we propagate to? @jeremyroman's research in https://github.com/whatwg/html/pull/10404 settled on the JavaScript function object's realm's global object.

---

Before merging this change, however, we need to have PRs ready for all call sites of construct/invoke/call a user object's operation, to prepare them for the new default. In most cases this will be deleting some spec code that try/catches the exception and reports it. So, this will be a multi-spec project.

We can use these links to find all callers:

• Invoke
• Call a user object's operation
• Construct

[jeremyroman]

A survey of the current uses of these algorithms indexed by webdex. Note that promise-returning callback types cannot actually throw, because WebIDL turns this into a promise rejection internally.

algorithm	spec	topic	catches and reports	rethrows	is promise	other	notes
invoke	DOM	mutation observers	•
invoke	HTML	canvas toBlob				•	should report (doesn't currently)
invoke	HTML	custom element reactions	•
invoke	HTML	navigate event			•
invoke	HTML	event handler attribute		•
invoke	HTML	timers	•
invoke	HTML	microtask queuing	•
invoke	HTML	animation frames	•
invoke	HTML	worklet registration example				•	probably intended to rethrow but unclear
invoke	Notifications	request	•
invoke	Streams			•
invoke	Files and Directories					•	unclear, but likely should report (assuming browsers do)
invoke	Prioritized Task Scheduling	postTask				•	catches and rejects a promise; might be able to change to use a Promise<any> return type instead of any to have WebIDL handle this
invoke	Shared Storage	select-url result index				•	catches and rejects a promise (in a task)
invoke	Shared Storage	run()			•		not entirely certain this is true; the callback type is oddly abstract
invoke	Video rVFC		•
invoke	Web Smart Card				•
invoke	CSS Layout API	intrinsic sizes				•	unclear, but likely should report
invoke	CSS Layout API	layout callback				•	unclear, but likely should report
invoke	CSS Paint API					•	unclear
invoke	CSS View Transitions	update			•
invoke	Geolocation	acquire				•	unclear, but likely should report
invoke	Reporting API		•
invoke	Web Locks				•
invoke	WebXR	XR animation frame	•
call a user object's operation	DOM	handleEvent				•	reports but also sets a special flag for IndexedDB
construct	DOM	sync create custom element				•	reports but also does other things
construct	HTML	upgrade custom element				•	reports but also does other things
construct	SharedStorage	register				•	unclear but should probably rethrow
construct	Web Audio	instantiate processor				•	special handling to fire processorerror

Excluded from this list are references which do not invoke these algorithms (but mention or modify it).

The "catch and report" cases naturally don't specify the global (except HTML, where this was part of whatwg/html#10404). It's likely that the callback's realm's global is appropriate, though for each a cursory check of existing browser behavior would be nice.

WebIDL also doesn't have many normative references to HTML. This wouldn't be the first, though.

Do you still think this should be a boolean defaulting to reporting, or should this be a required parameter? Or maybe some kind of conditionally required parameter, where it must be specified unless it's a promise-returning callback?

[domenic]

Wow, amazing work; thank you.

canvas toBlob

This needs to catch and report, I think? Right now the error just goes to the top level of the task, which is... undefined behavior?

It's likely that the callback's realm's global is appropriate, though for each a cursory check of existing browser behavior would be nice.

I'd really like us to be able to use the same answer for all cases and not have this be configurable per call site. But yeah, agreed that a check would be good.

Do you still think this should be a boolean defaulting to reporting, or should this be a required parameter? Or maybe some kind of conditionally required parameter, where it must be specified unless it's a promise-returning callback?

I hadn't thought a conditionally-required parameter before. Maybe that's the way to go, if we're changing all the call sites anyway.

Reporting-by-default makes "invoke" and friends require fewer brains to use; if you told me there was a Web IDL algorithm called "invoke" for callbacks I might just call it without thinking to look up if it had extra arguments. But if we can assume that all spec writers look up the signatures of the algorithms they use, then probably it's better. And I think that's a ... reasonable? ... assumption.

In that case I'd change from a boolean to something like "report" vs. "rethrow".

I was wondering if we could do "report-and-rethrow" for the cases that report but also do custom behavior. But I feel like probably in those cases you want to do the custom behavior first, before potentially running author code? (Although at least one place does not do that right now... I wonder if that's a bug.) I don't know; if there were a way to make it easier to ensure that people always send it to the right global for a callback after doing their custom processing, that would be nice. Let me know if you have an yideas.

For how to do "conditionally-required", I'd do something like

To invoke a callback function type value callable with a Web IDL arguments list args, exception behavior exceptionBehavior, and an optional callback this value thisArg, perform the following steps. These steps will either return an IDL value or throw an exception. The exceptionBehavior argument must be supplied if and only if callable's return type is not a promise type.

The other strategy I thought of was declaring it as optional with a default of "promise", and then using an Assert to state that it's not left as the default for non-promise _callable_s. However, this has the disadvantage that someone skimming the algorithm declaration might think it was actually optional.

[jeremyroman]

canvas toBlob

This needs to catch and report, I think? Right now the error just goes to the top level of the task, which is... undefined behavior?

Agreed, it should probably report. Updated the table.

It's likely that the callback's realm's global is appropriate, though for each a cursory check of existing browser behavior would be nice.

I'd really like us to be able to use the same answer for all cases and not have this be configurable per call site. But yeah, agreed that a check would be good.

Agreed, though if that is not what browsers do then we have to get vendors to change it, assuming doing so would be web-compatible. I'm hoping that it's either already doing that everywhere or it's easy to fix.

Do you still think this should be a boolean defaulting to reporting, or should this be a required parameter? Or maybe some kind of conditionally required parameter, where it must be specified unless it's a promise-returning callback?

I hadn't thought a conditionally-required parameter before. Maybe that's the way to go, if we're changing all the call sites anyway.

Reporting-by-default makes "invoke" and friends require fewer brains to use; if you told me there was a Web IDL algorithm called "invoke" for callbacks I might just call it without thinking to look up if it had extra arguments. But if we can assume that all spec writers look up the signatures of the algorithms they use, then probably it's better. And I think that's a ... reasonable? ... assumption.

In that case I'd change from a boolean to something like "report" vs. "rethrow".

I was wondering if we could do "report-and-rethrow" for the cases that report but also do custom behavior. But I feel like probably in those cases you want to do the custom behavior first, before potentially running author code? (Although at least one place does not do that right now... I wonder if that's a bug.) I don't know; if there were a way to make it easier to ensure that people always send it to the right global for a callback after doing their custom processing, that would be nice. Let me know if you have an yideas.

There are few enough sites and they're special enough already that I think complicating WebIDL to handle them is probably not worthwhile. And hopefully the number of such sites doesn't grow too much.

For how to do "conditionally-required", I'd do something like

To invoke a callback function type value callable with a Web IDL arguments list args, exception behavior exceptionBehavior, and an optional callback this value thisArg, perform the following steps. These steps will either return an IDL value or throw an exception. The exceptionBehavior argument must be supplied if and only if callable's return type is not a promise type.

The other strategy I thought of was declaring it as optional with a default of "promise", and then using an Assert to state that it's not left as the default for non-promise _callable_s. However, this has the disadvantage that someone skimming the algorithm declaration might think it was actually optional.

I had those exact same two thoughts and typed both of them in a response before noticing you'd described exactly the same things.

(also, bikeshed: exceptionBehavior vs exceptionHandling?)

[jeremyroman]

Another quick question -- in the report case, what happens if the return type is not undefined? I'm inclined to assert that never happens, but we could instead try to convert undefined to whatever IDL type is expected, which might be better since that's what we do if IsCallable(F) is false, in this same algorithm.

Edit: Converting undefined to a type can itself throw, and that isn't handled above. Fun.

[jeremyroman]

Also, since all call sites of construct and call a user object's operation do something special, I'm leaving them alone for now. Construct, in particular, can't just report and return nothing because callers are definitely going to be expecting a result.

[domenic]

(Agreed on all the decisions you made regarding return types, construct, and call a user object's operation.)

@jeremyroman are you up for turning your table in https://github.com/whatwg/webidl/issues/1423#issuecomment-2258993403 into a checklist, now that we know which cases need mandatory updates? Probably create a new tracking issue in this repo.