reportException(ex) / reportError(ex)

[benjamingr]

Refs: whatwg/html#1196

The following API was suggested for HTML:

self.reportError(ex); // reports an exception - HTML's "report the error"
// Typically, beforehand to invoke an uncaught exception users did/do:
setTimeout(() => { throw e; }, 0);

From the top of my head, some people with opinions on these topics:
cc @ronag @mcollina @jasnell @Linkgoron @addaleax from Node.js
cc @annevk @domenic from whatwg about whether or not this makes sense from that angle
cc @benlesh for general interesting feedback and a library author PoV and for suggesting this

---

One big consideration here is that Node.js error handling model is different, if reportError does setTimeout(() => { throw e; }, 0); it's less a "report an error" and more a "crash the process with this uncauight exception".

One upside is that this API can let us provide better developer experience than a throw in a setTimeout since we can (in theory) preserve the stack and more information about the error state.

[mcollina]

I don't understand what would be the semantics of that inside a Node.js process. What would that do?

[benjamingr]

I don't understand what would be the semantics of that inside a Node.js process. What would that do?

That's a good question and why I opened an issue. The two "obvious" options are:

• Crash the process with an uncaught exception.
• Log the error and carry on.

[Linkgoron]

Killing the process would basically be making a public "next tick" process._fatalException method. I'm not sure I'm actually against such a method, I'm just not sure that reportError creates a "The process is going to crash" feeling (as opposed to it calling an error monitor).

[addaleax]

as opposed to it calling an error monitor

I guess the question here is, when would you, practically speaking, use this method over console.error() in a browser?

[benjamingr]

I guess the question here is, when would you, practically speaking, use this method over console.error() in a browser?

One difference is that calling this would trigger the inspector's "pause on uncaught exception".

[domenic]

Killing the process would basically be making a public "next tick" process._fatalException method

It would actually be same-tick. The setTimeout(,0) in the OP is saying what people currently do to work around the lack of such method, but the workaround is not really correct since it delays the uncaught exception handling by a tick (which is not the correct semantics, at least for browser cases).

---

Overall my initial feeling was that this API doesn't make sense in Node since Node has made a choice to handle uncaught exceptions so differently than browsers do. I.e., trying to write isomorphic code around exceptions seems like something you'd want to actively discourage.

However @benlesh's comments in whatwg/html#1196 (comment) indicate that at least some library authors want to have their libraries do different things in Node (crash the process) versus browsers (report the exception). I'm guessing such library authors are following a rule that they want to have the "idiomatic" behavior in each environment. So maybe it makes sense to supply such a feature after all, as long as isomorphic-code authors understand how the behavior differs and consciously choose to use the feature with that in mind.

[benlesh]

This is correct. If the crash behavior was same-tick (yet uncatchable) in node, that might be beneficial for people debugging dumps, etc.

As it stands now, RxJS will throw all unhandled errors in setTimeout(,0). Which is done primarily to prevent unwinding the stack and causing "producer interference" which leads to extremely hard to debug issues in multicasting scenarios.

[benlesh]

Thinking more about this, this almost sounds like a process.panic more than a reportError, I guess. But I would expect it to fire process.on('error', fn) so maybe I'm wrong about that. Either way, it would be great if this API existed, and we'd use it, because it would be semantically more "correct" than throwing in setTimeout(,0). It may help with testing, (could mock or spy on the method) etc.

[mcollina]

I would like to step back and think about our exiting procedures.

I think adding a process.cleanShutdown(err?) might be a good addition if it was paired with a process.onShutdown = function (err, cb) {} (note that this is a async).

In some mission-critical cases servers would like to shut down connections and current operations before exiting/crashing, performing a "clean" shutdown. I wrote my own module for this, but there are plenty of others: https://github.com/mcollina/close-with-grace.

[benjamingr]

This has stalled a while ago - so closing (with the usual "if anyone feels differently feel free to reopen").

[getify]

I just found out about reportError(..) in the browser, and after some initial confusion as to its utility (over console.error(..)), I now think it's a useful addition. I just released a patch update to one of my libraries to detect and use it, if it's present in the JS environment.

The main attraction to it, over console.error(..), is that if someone's app is using an exception monitoring service, which captures things via window.onerror / window.addEventListener("error", ..), then these reported errors make their way into that machinery. That's almost certainly what you'd want to have happen, so it's nice this can happen cleanly/semantically without the setTimeout(..0) hack.

After releasing it in my library, the first thing I did was google to see if Node had it, or was thinking about adding it, and ended up here. My thought process was: I bet there are some Node users who use similar node-exception-tracking libs/services, and they'd like my library's uncaught exceptions to bubble into that machinery, just like they would in the browser.

So I'd say there is interest from library authors (at least RxJS and me) for consistent error-reporting APIs across browser and Node. I mean, that's why it's nice that both browser and Node have console.* functions, so similarly it'd be nice if Node also has reportError(..).

I'm not sure I have an opinion on whether such an error should crash the Node process or not. But at a minimum, it would definitely be nice for it to push such a reported-error into process.on("error" ..) and process.on("uncaughtException" ..) handlers.

As for crashing the process, it seems like the "ergonomic" thing (from my exposure to Node) is, default to crashing, UNLESS some CLI flag or other config has overridden it, OR someone has taken the effort to register the "uncaughtException" handler, in which case I think they're programmatically saying, "it's cool, I got the error, no need to crash". Alternatively, perhaps Node's version of reportError(..) could take a second argument to indicate if the error should be seen as a crashing signal or not. Isomorphic code wise, in the browser, such an argument would be irrelevant and ignored.

In any case, I'd love to revive this topic and see if it could push forward in some fashion.

[benjamingr]

So I'd say there is interest from library authors (at least RxJS and me)

Ben was the initial person who pinged me about this :)

Alternatively, perhaps Node's version of reportError(..) could take a second argument to indicate if the error should be seen as a crashing signal or not. Isomorphic code wise, in the browser, such an argument would be irrelevant and ignored.

I feel strongly that whatever API we pick if it's a web standard API we should align with browsers. WHATWG has been nothing but cooperative/helpful with this sort of thing and I don't think they'd object to a second parameter (severity?) if it makes sense for browsers (which I believe it does?).

That said your other suggestion (adding an unhandledRejection handler) seems very reasonable to me.

In any case, I'd love to revive this topic and see if it could push forward in some fashion.

This only stalled because of lack of feedback/interest expressed - so absolutely.

[benjamingr]

I'll open a PR to get feedback on timing/semantics

[benlesh]

For my part, right now we're doing the setTimeout(() => { throw error; }) thing, which will crash the process. If reportError did something different than that, it would be a breaking change for us, but I guess I don't really care that much about that, so long as it's idiomatic for the platform.

reportError in Node's case could take a second argument to configure the call as well. reportError(error, { panic: true }) or reportError(error, { logOnly: true }) in the other case. Just a thought.

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[github-actions]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.