From de3603f0a6a7a9e56cc4897b67aa6b11f0f76e2a Mon Sep 17 00:00:00 2001
From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com>
Date: Wed, 8 Jan 2020 10:22:00 +0100
Subject: [PATCH] process: allow monitoring uncaughtException

Installing an uncaughtException listener has a side effect that process
is not aborted. This is quite bad for monitoring/logging tools which
tend to be interested in errors but don't want to cause side effects
like swallow an exception or change the output on console.

There are some workarounds in the wild like monkey patching emit or
rethrow in the exception if monitoring tool detects that it is the only
listener but this is error prone and risky.

This PR allows to install a listener to monitor uncaughtException
without the side effect to consider the exception has handled.

PR-URL: https://github.com/nodejs/node/pull/31257
Refs: https://github.com/nodejs/node/pull/30932
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/process.md                            | 32 +++++++++
 lib/internal/process/execution.js             |  1 +
 .../uncaught-exceptions/uncaught-monitor1.js  | 10 +++
 .../uncaught-exceptions/uncaught-monitor2.js  | 11 +++
 ...test-process-uncaught-exception-monitor.js | 69 +++++++++++++++++++
 5 files changed, 123 insertions(+)
 create mode 100644 test/fixtures/uncaught-exceptions/uncaught-monitor1.js
 create mode 100644 test/fixtures/uncaught-exceptions/uncaught-monitor2.js
 create mode 100644 test/parallel/test-process-uncaught-exception-monitor.js

diff --git a/doc/api/process.md b/doc/api/process.md
index 72fb416373fa12..6fdd40068417c3 100644
--- a/doc/api/process.md
+++ b/doc/api/process.md
@@ -262,6 +262,10 @@ nonexistentFunc();
 console.log('This will not run.');
 ```
 
+It is possible to monitor `'uncaughtException'` events without overriding the
+default behavior to exit the process by installing a
+`'uncaughtExceptionMonitor'` listener.
+
 #### Warning: Using `'uncaughtException'` correctly
 
 `'uncaughtException'` is a crude mechanism for exception handling
@@ -289,6 +293,34 @@ To restart a crashed application in a more reliable way, whether
 in a separate process to detect application failures and recover or restart as
 needed.
 
+### Event: `'uncaughtExceptionMonitor'`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `err` {Error} The uncaught exception.
+* `origin` {string} Indicates if the exception originates from an unhandled
+  rejection or from synchronous errors. Can either be `'uncaughtException'` or
+  `'unhandledRejection'`.
+
+The `'uncaughtExceptionMonitor'` event is emitted before an
+`'uncaughtException'` event is emitted or a hook installed via
+[`process.setUncaughtExceptionCaptureCallback()`][] is called.
+
+Installing an `'uncaughtExceptionMonitor'` listener does not change the behavior
+once an `'uncaughtException'` event is emitted. The process will
+still crash if no `'uncaughtException'` listener is installed.
+
+```js
+process.on('uncaughtExceptionMonitor', (err, origin) => {
+  MyMonitoringTool.logSync(err, origin);
+});
+
+// Intentionally cause an exception, but don't catch it.
+nonexistentFunc();
+// Still crashes Node.js
+```
+
 ### Event: `'unhandledRejection'`
 <!-- YAML
 added: v1.4.1
diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
index d9567e643d7c85..a6d6d2325a7f79 100644
--- a/lib/internal/process/execution.js
+++ b/lib/internal/process/execution.js
@@ -159,6 +159,7 @@ function createOnGlobalUncaughtException() {
     }
 
     const type = fromPromise ? 'unhandledRejection' : 'uncaughtException';
+    process.emit('uncaughtExceptionMonitor', er, type);
     if (exceptionHandlerState.captureFn !== null) {
       exceptionHandlerState.captureFn(er);
     } else if (!process.emit('uncaughtException', er, type)) {
diff --git a/test/fixtures/uncaught-exceptions/uncaught-monitor1.js b/test/fixtures/uncaught-exceptions/uncaught-monitor1.js
new file mode 100644
index 00000000000000..9724539a253bc1
--- /dev/null
+++ b/test/fixtures/uncaught-exceptions/uncaught-monitor1.js
@@ -0,0 +1,10 @@
+'use strict';
+
+// Keep the event loop alive.
+setTimeout(() => {}, 1e6);
+
+process.on('uncaughtExceptionMonitor', (err) => {
+  console.log(`Monitored: ${err.message}`);
+});
+
+throw new Error('Shall exit');
diff --git a/test/fixtures/uncaught-exceptions/uncaught-monitor2.js b/test/fixtures/uncaught-exceptions/uncaught-monitor2.js
new file mode 100644
index 00000000000000..743a5611008ba2
--- /dev/null
+++ b/test/fixtures/uncaught-exceptions/uncaught-monitor2.js
@@ -0,0 +1,11 @@
+'use strict';
+
+// Keep the event loop alive.
+setTimeout(() => {}, 1e6);
+
+process.on('uncaughtExceptionMonitor', (err) => {
+  console.log(`Monitored: ${err.message}, will throw now`);
+  missingFunction();
+});
+
+throw new Error('Shall exit');
diff --git a/test/parallel/test-process-uncaught-exception-monitor.js b/test/parallel/test-process-uncaught-exception-monitor.js
new file mode 100644
index 00000000000000..fc6c2637f6978e
--- /dev/null
+++ b/test/parallel/test-process-uncaught-exception-monitor.js
@@ -0,0 +1,69 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { execFile } = require('child_process');
+const fixtures = require('../common/fixtures');
+
+{
+  // Verify exit behavior is unchanged
+  const fixture = fixtures.path('uncaught-exceptions', 'uncaught-monitor1.js');
+  execFile(
+    process.execPath,
+    [fixture],
+    common.mustCall((err, stdout, stderr) => {
+      assert.strictEqual(err.code, 1);
+      assert.strictEqual(Object.getPrototypeOf(err).name, 'Error');
+      assert.strictEqual(stdout, 'Monitored: Shall exit\n');
+      const errLines = stderr.trim().split(/[\r\n]+/);
+      const errLine = errLines.find((l) => /^Error/.exec(l));
+      assert.strictEqual(errLine, 'Error: Shall exit');
+    })
+  );
+}
+
+{
+  // Verify exit behavior is unchanged
+  const fixture = fixtures.path('uncaught-exceptions', 'uncaught-monitor2.js');
+  execFile(
+    process.execPath,
+    [fixture],
+    common.mustCall((err, stdout, stderr) => {
+      assert.strictEqual(err.code, 7);
+      assert.strictEqual(Object.getPrototypeOf(err).name, 'Error');
+      assert.strictEqual(stdout, 'Monitored: Shall exit, will throw now\n');
+      const errLines = stderr.trim().split(/[\r\n]+/);
+      const errLine = errLines.find((l) => /^ReferenceError/.exec(l));
+      assert.strictEqual(
+        errLine,
+        'ReferenceError: missingFunction is not defined'
+      );
+    })
+  );
+}
+
+const theErr = new Error('MyError');
+
+process.on(
+  'uncaughtExceptionMonitor',
+  common.mustCall((err, origin) => {
+    assert.strictEqual(err, theErr);
+    assert.strictEqual(origin, 'uncaughtException');
+  }, 2)
+);
+
+process.on('uncaughtException', common.mustCall((err, origin) => {
+  assert.strictEqual(origin, 'uncaughtException');
+  assert.strictEqual(err, theErr);
+}));
+
+process.nextTick(common.mustCall(() => {
+  // Test with uncaughtExceptionCaptureCallback installed
+  process.setUncaughtExceptionCaptureCallback(common.mustCall(
+    (err) => assert.strictEqual(err, theErr))
+  );
+
+  throw theErr;
+}));
+
+throw theErr;