lib,src: exit process on unhandled promise rejection cleanup by Fishrock123 · Pull Request #12010 · nodejs/node (original) (raw)

We should crash by default in the absence of user-installed unhandledRejection handlers. This is a controversial stance (some notable promise-users disagree with me!), but I believe it's the best solution for Node's users in the long run. I will describe why I think this is a good behavior first, followed by why I think it's valid route for Node to take.

• Crashing on unhandled rejection is more useful than logging: it prevents the program from taking further action (modulo nextTick and microtask calls — it's better behavior than we have now, not perfect).
• It allows the program to more quickly & reliably enter a known-good state via restarting. Crashing on unhandled rejection aligns with Node's exception handling behavior, which is to terminate the program on unexpected behavior (notably: in that case we diverge from the browser, which continues the execution of the program!)
  • It is not perfect. We do not get usable core dumps from crashing on unhandled rejection. It is an incremental improvement in this case.
• It is an acknowledgement that it is unsafe to publish modules that cause the execution of an externally-installed catch-all as a by-product of normal use (Would you use a module that relied on Node being able to survive an unhandled exception?)

Crashing on unhandled rejection is a valid path:

• It does not break Promise.reject() — which will only trip unhandledRejection if a handler is not installed by next RunMicrotasks() call.
• There's always a way to write a JavaScript program such that it doesn't trip the unhandled rejection handler. If you want to hang onto a rejection & handle it later, call myPromise.catch(() => {}). Later, when you want to handle the error, attaching another .catch will cause the error to propagate through that route.
• As a migration path, we can add a --allow-unhandled-rejections flag to the Node release to retain old-mode behavior.

I don't think we should move forward with crash-on-GC as a default behavior. As a habitual promise user, it worries me:

• The base promise implementation will hit this. Other implementations will not. In practice, in development, it's not uncommon to have a mix of promise implementations interoperating. Having one behavior for some of them and another behavior for others is confusing and bad. Adding crash-on-GC exacerbates this situation.
• Crash-on-GC adds a perf hit for folks adopting safe behavior — if you've installed a crash-on-GC rejection handler, you still pay the tracking cost per promise. Were it an optional flag, I would not use it — with a number of different promise implementations in play, I would not find crash-on-GC useful to diagnose bugs with.
• Crash-on-GC responds to memory pressure — which makes crashes subject to other conditions in the system.

As someone who develops & deploys production promise-based services, I would hesitate before migrating to a version of node that included crash-on-GC behavior by default. As a former Node CTC member, I'd advise caution before taking onboard the complexity of introducing it as a debugging tool.