Skip to content

Commit

Permalink
events: allow monitoring error events
Browse files Browse the repository at this point in the history
Installing an error listener has a side effect that emitted errors are
considered as handled. 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.

There are some workarounds in the wild like monkey patching emit or
remit the error 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 errors with the side
effect to consume the error. To avoid conflicts with other events it
exports a symbol on EventEmitter which owns this special meaning.

Refs: open-telemetry/opentelemetry-js#225

PR-URL: #30932
Refs: open-telemetry/opentelemetry-js#225
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
Flarna authored and BridgeAR committed Jan 3, 2020
1 parent 6b65caf commit f570de8
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 2 deletions.
25 changes: 25 additions & 0 deletions doc/api/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,18 @@ myEmitter.emit('error', new Error('whoops!'));
// Prints: whoops! there was an error
```

It is possible to monitor `'error'` events without consuming the emitted error
by installing a listener using the symbol `errorMonitor`.

```js
const myEmitter = new MyEmitter();
myEmitter.on(EventEmitter.errorMonitor, (err) => {
MyMonitoringTool.log(err);
});
myEmitter.emit('error', new Error('whoops!'));
// Still throws and crashes Node.js
```

## Capture Rejections of Promises

> Stability: 1 - captureRejections is experimental.
Expand Down Expand Up @@ -348,6 +360,19 @@ the event emitter instance, the event’s name and the number of attached
listeners, respectively.
Its `name` property is set to `'MaxListenersExceededWarning'`.

### EventEmitter.errorMonitor
<!-- YAML
added: REPLACEME
-->

This symbol shall be used to install a listener for only monitoring `'error'`
events. Listeners installed using this symbol are called before the regular
`'error'` listeners are called.

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

### emitter.addListener(eventName, listener)
<!-- YAML
added: v0.1.26
Expand Down
14 changes: 12 additions & 2 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const {
} = require('internal/util/inspect');

const kCapture = Symbol('kCapture');
const kErrorMonitor = Symbol('events.errorMonitor');

function EventEmitter(opts) {
EventEmitter.init.call(this, opts);
Expand Down Expand Up @@ -83,6 +84,13 @@ ObjectDefineProperty(EventEmitter, 'captureRejections', {
enumerable: true
});

ObjectDefineProperty(EventEmitter, 'errorMonitor', {
value: kErrorMonitor,
writable: false,
configurable: true,
enumerable: true
});

// The default for captureRejections is false
ObjectDefineProperty(EventEmitter.prototype, kCapture, {
value: false,
Expand Down Expand Up @@ -256,9 +264,11 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
let doError = (type === 'error');

const events = this._events;
if (events !== undefined)
if (events !== undefined) {
if (doError && events[kErrorMonitor] !== undefined)
this.emit(kErrorMonitor, ...args);
doError = (doError && events.error === undefined);
else if (!doError)
} else if (!doError)
return false;

// If there is no 'error' event listener then throw.
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-event-emitter-error-monitor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const EventEmitter = require('events');

const EE = new EventEmitter();
const theErr = new Error('MyError');

EE.on(
EventEmitter.errorMonitor,
common.mustCall(function onErrorMonitor(e) {
assert.strictEqual(e, theErr);
}, 3)
);

// Verify with no error listener
common.expectsError(
() => EE.emit('error', theErr), theErr
);

// Verify with error listener
EE.once('error', common.mustCall((e) => assert.strictEqual(e, theErr)));
EE.emit('error', theErr);


// Verify it works with once
process.nextTick(() => EE.emit('error', theErr));
assert.rejects(EventEmitter.once(EE, 'notTriggered'), theErr);

// Only error events trigger error monitor
EE.on('aEvent', common.mustCall());
EE.emit('aEvent');

0 comments on commit f570de8

Please sign in to comment.