Introduce self.reportError()

[annevk]

Fixes whatwg/console#50.

• At least two implementers are interested (and none opposed):
  • Chrome
  • Firefox
• Tests are written and can be reviewed and commented upon at:
  • HTML: self.reportError() web-platform-tests/wpt#29738
• Implementation bugs are filed:
  • Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1233383
  • Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1722448
  • Safari: https://bugs.webkit.org/show_bug.cgi?id=228316

(See WHATWG Working Mode: Changes for more details.)

---

/webappapis.html ( diff )

[domenic]

So based on following the links it looks like @bzbarsky is in favor of implementing this in Gecko. Any other implementers interested?

[terinjokes]

Will the be hooks for scripts to hook into this (ie, for New Relic or Sentry error reporting?), or will they still have to trampoline through window.onerror?

[domenic]

What do you mean by "trampoline through window.onerror?" What is the use case that window.addEventListener("error", ...) currently doesn't solve?

[terinjokes]

I didn't know that window.addEventListener("error", …) was a thing, tbqh.

[zcorpan]

Should this do nothing if it's called inside window.onerror (like runtime script errors in onerror are no-ops)? (Can still report something to devtools though.)

[annevk]

It does nothing, right? Target would be in error reporting mode.

[zcorpan]

Right, sorry. I misremembered the logic.

[annevk]

To be clear, the reason we want this is for JavaScript to be able to emulate the behavior of dispatchEvent(), the behavior of custom element reactions, and maybe a few other features I'm forgetting.

[annevk]

It seems this is still the right approach even after "report an exception" is fixed in #958. We don't handle muting here, but I don't think we need to.

[rniwa]

Here's one feedback: have we considered adding this on console instead?

[domenic]

@rniwa we did; that was actually the original suggestion. However, it fires an Error event at self, so it seems better to put it there.

[annevk]

Surely Blink's "explain the platform" folks are interested in this?

[annevk]

@rniwa any more thoughts? Shall we do this? @mathiasbynens perhaps you can figure out Chrome support for this?

[annevk]

@rniwa @mathiasbynens ping.

[foolip]

@clelland can you take a look at this for Chrome? I suspect there is a connection to https://w3c.github.io/reporting/ in that things which are reported through self.reportException() should feed into the Reporting API as well. Right?

[domenic]

I found https://github.com/ReactiveX/rxjs/blob/master/src/internal/util/reportUnhandledError.ts showing how at least one popular library is doing this today: indeed, it's with setTimeout(() => { throw e; }, 0) hacks. @benlesh, would you be interested in a native self.reportException(e) which would allow you to do this without setTimeout hacks, in the same way that the browser does internally for implementing EventTarget?

https://github.com/kriskowal/asap seems to do something similar, except hidden behind a few layers of abstraction. Any thoughts, @kriskowal?

[gaearon]

Not sure if this is the same thing as you're asking, but we use this trick in React:

https://github.com/facebook/react/blob/1b7b3592f4713b3a53709554aefd1d20e9a0a6a5/packages/shared/invokeGuardedCallbackImpl.js#L33-L237

Essentially, when we catch a user error in our try / catch (which we need to keep), we want to rethrow it, but we want the browser to present it as unhandled ("pause on uncaught" should break on it, the error message should say "Uncaught", and so on.)

Our trick is to call user code inside the context of a fake DOM event dispatch, and listen to the error event to see if it errored. This way it's treated by the browser as a top-level error even though technically there's a try / catch higher up in the stack.

[kriskowal]

ASAP would certainly have benefited from a mechanism to surface an error as if it had been thrown but without throwing. Much of the complexity in ASAP flows from preserving execution order and amortizing timers. Any time it needs to report an error, ASAP stops flushing its event queue, schedules a flush, and throws the error. With, reportError or some such, that would just be a for loop.

[benlesh]

would you be interested in a native self.reportException(e) which would allow you to do this without setTimeout hacks, in the same way that the browser does internally for implementing EventTarget?

@domenic Absolutely! Whenever we implemented this, I was honestly curious why something like that didn't already exist.

Although, we would probably still end up using setTimeout and throwing, because RxJS has to run in node.js as well. Unless this same mechanism existed in node.js, I'm not sure we would use it unless there were tangible benefits to doing so. Maybe @MylesBorins or @benjamingr have thoughts about an API like this existing in node.js? (If so I guess a discussion should start there, as it's off topic here)

[benjamingr]

Opened a discussion in Node.js :) nodejs/node#38947

[annevk]

This seems like something we should also expose in worklets, right? cc @padenot

[domenic]

Since nobody has had further strong opinions on the name I'm going to change it to reportError().

[padenot]

This seems like something we should also expose in worklets, right? cc @padenot

Each worklet implement their own error reporting, if any, and for the AudioWorklet, the error reporting is outside the scope, so it's fairly different.

[annevk]

I cannot approve my own PR, but I still think this is okay. I'll update OP with the PR template as I don't think we have tests for this yet.

[annevk]

While writing tests I briefly wondered if we wanted to expose whether the exception was handled in some way, but I could not think of a reason why a library would want control over that. Especially as the use case is emulating what happens if an exception is thrown in certain contexts. (If an exception is not handled, the browser will report it in the console.)

[domenic]

The tests made me realize we probably want optional any instead of any because try { x(); } catch (e) { reportError(e); } could end up with e === undefined.

[domenic]

Oh, hmm, re-reading https://heycam.github.io/webidl/#es-any it looks like undefined works fine for an any value, so self.reportError(undefined) works fine.

I guess the difference between optional any and any is that self.reportError() would only work with optional any? Hmmmm.