Skip to content

Commit

Permalink
events: change EventTarget handler exception behavior
Browse files Browse the repository at this point in the history
Change the behavior of EventTarget, instead of emitting
'error' on handler exception, emit 'uncaughtException'.

Fixes: nodejs#36770
  • Loading branch information
Nitzan Uziely committed Feb 5, 2021
1 parent fe43bd8 commit 1847f38
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
9 changes: 6 additions & 3 deletions doc/api/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,9 @@ setMaxListeners(5, target, emitter);
<!-- YAML
added: v14.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37237
description: changed EventTarget error handling.
- version: v15.4.0
pr-url: https://github.com/nodejs/node/pull/35949
description: No longer experimental.
Expand Down Expand Up @@ -1247,9 +1250,9 @@ target.addEventListener('foo', handler4, { once: true });
### `EventTarget` error handling

When a registered event listener throws (or returns a Promise that rejects),
by default the error is forwarded to the `process.on('error')` event
on `process.nextTick()`. Throwing within an event listener will *not* stop
the other registered handlers from being invoked.
by default the error is forwarded to the `process.on('uncaughtException')`
event on `process.nextTick()`. Throwing within an event listener will *not*
stop the other registered handlers from being invoked.

The `EventTarget` does not implement any special default handling for
`'error'` type events.
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/event_target.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,9 @@ class EventTarget {
}
const result = FunctionPrototypeCall(handler.callback, this, arg);
if (result !== undefined && result !== null)
addCatch(this, result, createEvent());
addCatch(result);
} catch (err) {
emitUnhandledRejectionOrErr(this, err, createEvent());
emitUncaughtException(err);
}

handler = next;
Expand Down Expand Up @@ -582,19 +582,19 @@ function isEventTarget(obj) {
return obj?.constructor?.[kIsEventTarget];
}

function addCatch(that, promise, event) {
function addCatch(promise) {
const then = promise.then;
if (typeof then === 'function') {
FunctionPrototypeCall(then, promise, undefined, function(err) {
// The callback is called with nextTick to avoid a follow-up
// rejection from this promise.
process.nextTick(emitUnhandledRejectionOrErr, that, err, event);
emitUncaughtException(err);
});
}
}

function emitUnhandledRejectionOrErr(that, err, event) {
process.emit('error', err, event);
function emitUncaughtException(err) {
process.nextTick(() => { throw err; });
}

function makeEventHandler(handler) {
Expand Down
12 changes: 7 additions & 5 deletions test/parallel/test-eventtarget.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,16 @@ let asyncTest = Promise.resolve();
}

{
const uncaughtException = common.mustCall((err, event) => {
const uncaughtException = common.mustCall((err, origin) => {
strictEqual(err.message, 'boom');
strictEqual(event.type, 'foo');
strictEqual(origin, 'uncaughtException');
}, 4);

// Whether or not the handler function is async or not, errors
// are routed to uncaughtException
process.on('error', uncaughtException);
// Make sure that we no longer call 'error' on error.
process.on('error', common.mustNotCall());
// Don't call rejection even for async handlers.
process.on('unhandledRejection', common.mustNotCall());
process.on('uncaughtException', uncaughtException);

const eventTarget = new EventTarget();

Expand Down

0 comments on commit 1847f38

Please sign in to comment.