Skip to content

Commit

Permalink
process: allow monitoring uncaughtException
Browse files Browse the repository at this point in the history
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. To avoid
conflicts with other events it exports a symbol on process which owns
this special meaning.
  • Loading branch information
Flarna committed Jan 8, 2020
1 parent b9e6c1b commit 9039128
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 2 deletions.
28 changes: 28 additions & 0 deletions doc/api/process.md
Expand Up @@ -262,6 +262,20 @@ 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 listener using the symbol
`uncaughtExceptionMonitor`.

```js
process.on(process.uncaughtExceptionMonitor, (err, origin) => {
MyMonitoringTool.logSync(err, origin);
});

// Intentionally cause an exception, but don't catch it.
nonexistentFunc();
// Still crashes Node.js
```

#### Warning: Using `'uncaughtException'` correctly

`'uncaughtException'` is a crude mechanism for exception handling
Expand Down Expand Up @@ -2320,6 +2334,20 @@ documentation for the [`'warning'` event][process_warning] and the
[`emitWarning()` method][process_emit_warning] for more information about this
flag's behavior.

## `process.uncaughtExceptionMonitor`
<!-- YAML
added: REPLACEME
-->

This symbol shall be used to install a listener for only monitoring
`'uncaughtException'` events. Listeners installed using this symbol are called
before the regular `'uncaughtException'` listeners and before a hook
installed via [`process.setUncaughtExceptionCaptureCallback()`][].

Installing a listener using this symbol does not change the behavior once an
`'uncaughtException'` event is emitted, therefore the process will still crash
if no regular `'uncaughtException'` listener is installed.

## `process.umask([mask])`
<!-- YAML
added: v0.1.19
Expand Down
9 changes: 8 additions & 1 deletion lib/internal/bootstrap/node.js
Expand Up @@ -206,7 +206,8 @@ ObjectDefineProperty(process, 'features', {
const {
onGlobalUncaughtException,
setUncaughtExceptionCaptureCallback,
hasUncaughtExceptionCaptureCallback
hasUncaughtExceptionCaptureCallback,
kUncaughtExceptionMonitor
} = require('internal/process/execution');

// For legacy reasons this is still called `_fatalException`, even
Expand All @@ -220,6 +221,12 @@ ObjectDefineProperty(process, 'features', {
setUncaughtExceptionCaptureCallback;
process.hasUncaughtExceptionCaptureCallback =
hasUncaughtExceptionCaptureCallback;
ObjectDefineProperty(process, 'uncaughtExceptionMonitor', {
value: kUncaughtExceptionMonitor,
writable: false,
configurable: true,
enumerable: true
});
}

const { emitWarning } = require('internal/process/warning');
Expand Down
7 changes: 6 additions & 1 deletion lib/internal/process/execution.js
Expand Up @@ -3,6 +3,7 @@
const {
JSONStringify,
PromiseResolve,
Symbol,
} = primordials;

const path = require('path');
Expand All @@ -27,6 +28,8 @@ const {
// communication with JS.
const { shouldAbortOnUncaughtToggle } = internalBinding('util');

const kUncaughtExceptionMonitor = Symbol('process.uncaughtExceptionMonitor');

function tryGetCwd() {
try {
return process.cwd();
Expand Down Expand Up @@ -159,6 +162,7 @@ function createOnGlobalUncaughtException() {
}

const type = fromPromise ? 'unhandledRejection' : 'uncaughtException';
process.emit(kUncaughtExceptionMonitor, er, type);
if (exceptionHandlerState.captureFn !== null) {
exceptionHandlerState.captureFn(er);
} else if (!process.emit('uncaughtException', er, type)) {
Expand Down Expand Up @@ -214,5 +218,6 @@ module.exports = {
evalScript,
onGlobalUncaughtException: createOnGlobalUncaughtException(),
setUncaughtExceptionCaptureCallback,
hasUncaughtExceptionCaptureCallback
hasUncaughtExceptionCaptureCallback,
kUncaughtExceptionMonitor
};
35 changes: 35 additions & 0 deletions test/parallel/test-process-uncaught-exception-monitor.js
@@ -0,0 +1,35 @@
'use strict';

const common = require('../common');
const assert = require('assert');

const theErr = new Error('MyError');

process.on(
process.uncaughtExceptionMonitor,
common.mustCall(function onUncaughtExceptionMonitor(err, origin) {
assert.strictEqual(err, theErr);
assert.strictEqual(origin, 'uncaughtException');
}, 2)
);

process.on('uncaughtException', common.mustCall(
function onUncaughtException(err, origin) {
assert.strictEqual(origin, 'uncaughtException');
assert.strictEqual(err, theErr);
})
);

process.nextTick(common.mustCall(
function withExceptionCaptureCallback() {
process.setUncaughtExceptionCaptureCallback(common.mustCall(
function uncaughtExceptionCaptureCallback(err) {
assert.strictEqual(err, theErr);
})
);

throw theErr;
})
);

throw theErr;

0 comments on commit 9039128

Please sign in to comment.