Fix uncaughtException exit

Author: ehmicky

README.md

@@ -26,8 +26,6 @@ logProcessErrors(options)
 `options` is an optional object with the following properties:
 
 - [`log` `{function}`](#custom-logging)
-- [`exitOnExceptions` `{boolean}`](#exiting-on-uncaught-exceptions) (default:
-  `true`)
 - [`getLevel` `{function}`](#log-level)
 - [`getMessage` `{function}`](#log-message)
 - [`colors` `{boolean}`](#log-message) (default: `false`)
@@ -80,21 +78,6 @@ The function's arguments are:
       `rejectionHandled` and `multipleResolves`
     - `secondPromiseState`, `secondPromiseValue`: only on `multipleResolves`
 
-# Exiting on uncaught exceptions
-
-By default `uncaughtException` will fire `process.exit(1)`. This is the
-recommended behavior according to the
-[Node.js documentation](https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly).
-
-This can disabled by setting the `exitOnExceptions` option to `false`:
-
-<!-- eslint-disable no-empty-function, no-unused-vars, node/no-missing-require,
-import/no-unresolved, unicorn/filename-case, strict, no-undef -->
-
-```js
-logProcessErrors({ exitOnExceptions: false })
-```
-
 # Log level
 
 By default the log level will be `warn` for `warning` events and `error` for
@@ -150,6 +133,16 @@ logProcessErrors({
 })
 ```
 
+# Uncaught exceptions
+
+`uncaughtException` events will fire `process.exit(1)`. This is the recommended
+behavior according to the
+[Node.js documentation](https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly).
+
+`process.exit(1)` will only be fired after the `uncaughtException` event has
+been logged. If an custom `log` option is used and it is asynchronous, the
+function should return a promise (or use `async`/`await`).
+
 # Stop logging
 
 Logging can be stopped by firing the function returned by `logProcessErrors()`

src/default.js

@@ -9,7 +9,6 @@ const DEFAULT_OPTS = {
   getLevel: defaultGetLevel,
   getMessage: defaultGetMessage,
   log: defaultLog,
-  exitOnExceptions: true,
   colors: true,
 }
 

src/handle.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const { exit } = require('process')
+const { promisify } = require('util')
 
 const { getInfo } = require('./info')
 const { getColors } = require('./colors')
@@ -34,22 +35,32 @@ const handleEvent = async function({
   const level = getLevel({ opts, info })
   const message = getMessage({ opts, info, level, colors })
 
-  opts.log(message, level, info)
+  // We need to `await` it in case we are logging an `uncaughtException` which
+  // will make the process exit.
+  // Without `await` Node.js would still wait until most async tasks (including
+  // stream draining for logging libraries like Winston) have completed.
+  // But there are some cases where it will not. In those cases, `opts.log()`
+  // should be either synchronous or return a promise.
+  await opts.log(message, level, info)
 
-  exitProcess({ eventName, opts })
+  await exitProcess({ eventName })
 }
 
 // Exit process on `uncaughtException`
 // See https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly
-// Can be disabled with `opts.exitOnExceptions: false`
-const exitProcess = function({ eventName, opts: { exitOnExceptions } }) {
-  if (eventName !== 'uncaughtException' || !exitOnExceptions) {
+const exitProcess = async function({ eventName }) {
+  if (eventName !== 'uncaughtException') {
     return
   }
 
+  // This is only needed as a safety measure
+  await promisify(setTimeout)(EXIT_TIMEOUT)
+
   exit(1)
 }
 
+const EXIT_TIMEOUT = 3e3
+
 module.exports = {
   handleEvent,
 }