events: change EventTarget handler exception behavior · nodejs/node@83d6e63 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 633
• Pull requests 756
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 83d6e63

Browse filesBrowse the repository at this point in the historyBrowse files

Nitzan Uziely

authored and

benjamingr

committed

events: change EventTarget handler exception behavior

Change the behavior of EventTarget, instead of emitting 'error' on handler exception, emit 'uncaughtException'. Fixes: #36770 PR-URL: #37237 Fixes: #36770 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

1 parent 9803729 commit 83d6e63
Copy full SHA for 83d6e63

4 files changed

+23-18Lines changed: 23 additions & 18 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• doc/api
  • events.md
• lib/internal
  • event_target.js
• test/parallel
  • test-eventtarget.js
  • test-process-uncaught-exception-monitor.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎doc/api/events.md‎

Copy file name to clipboardExpand all lines: doc/api/events.md

+3Lines changed: 3 additions & 0 deletions

• Display the source diff
• Display the rich diff

Original file line number	Diff line number	Diff line change
@@ -1137,6 +1137,9 @@ setMaxListeners(5, target, emitter);
1137	1137	<!-- YAML
1138	1138	added: v14.5.0
1139	1139	changes:
	1140	+ - version: REPLACEME
	1141	+ pr-url: https://github.com/nodejs/node/pull/37237
	1142	+ description: changed EventTarget error handling.
1140	1143	- version: v15.4.0
1141	1144	pr-url: https://github.com/nodejs/node/pull/35949
1142	1145	description: No longer experimental.

Collapse file

‎lib/internal/event_target.js‎

Copy file name to clipboardExpand all lines: lib/internal/event_target.js

+6-6Lines changed: 6 additions & 6 deletions

Original file line number	Diff line number	Diff line change
@@ -459,9 +459,9 @@ class EventTarget {
459	459	result = FunctionPrototypeCall(callback, this, arg);
460	460	}
461	461	if (result !== undefined && result !== null)
462		- addCatch(this, result, createEvent());
	462	+ addCatch(result);
463	463	} catch (err) {
464		- emitUnhandledRejectionOrErr(this, err, createEvent());
	464	+ emitUncaughtException(err);
465	465	}
466	466
467	467	handler = next;
@@ -624,19 +624,19 @@ function isEventTarget(obj) {
624	624	return obj?.constructor?.[kIsEventTarget];
625	625	}
626	626
627		- function addCatch(that, promise, event) {
	627	+ function addCatch(promise) {
628	628	const then = promise.then;
629	629	if (typeof then === 'function') {
630	630	FunctionPrototypeCall(then, promise, undefined, function(err) {
631	631	// The callback is called with nextTick to avoid a follow-up
632	632	// rejection from this promise.
633		- process.nextTick(emitUnhandledRejectionOrErr, that, err, event);
	633	+ emitUncaughtException(err);
634	634	});
635	635	}
636	636	}
637	637
638		- function emitUnhandledRejectionOrErr(that, err, event) {
639		- process.emit('error', err, event);
	638	+ function emitUncaughtException(err) {
	639	+ process.nextTick(() => { throw err; });
640	640	}
641	641
642	642	function makeEventHandler(handler) {

Collapse file

‎test/parallel/test-eventtarget.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-eventtarget.js

+7-5Lines changed: 7 additions & 5 deletions

Original file line number	Diff line number	Diff line change
@@ -178,14 +178,16 @@ let asyncTest = Promise.resolve();
178	178	}
179	179
180	180	{
181		- const uncaughtException = common.mustCall((err, event) => {
	181	+ const uncaughtException = common.mustCall((err, origin) => {
182	182	strictEqual(err.message, 'boom');
183		- strictEqual(event.type, 'foo');
	183	+ strictEqual(origin, 'uncaughtException');
184	184	}, 4);
185	185
186		- // Whether or not the handler function is async or not, errors
187		- // are routed to uncaughtException
188		- process.on('error', uncaughtException);
	186	+ // Make sure that we no longer call 'error' on error.
	187	+ process.on('error', common.mustNotCall());
	188	+ // Don't call rejection even for async handlers.
	189	+ process.on('unhandledRejection', common.mustNotCall());
	190	+ process.on('uncaughtException', uncaughtException);
189	191
190	192	const eventTarget = new EventTarget();
191	193

Collapse file

‎test/parallel/test-process-uncaught-exception-monitor.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-process-uncaught-exception-monitor.js

+7-7Lines changed: 7 additions & 7 deletions

Original file line number	Diff line number	Diff line change
@@ -55,15 +55,15 @@ process.on(
55	55	process.on('uncaughtException', common.mustCall((err, origin) => {
56	56	assert.strictEqual(origin, 'uncaughtException');
57	57	assert.strictEqual(err, theErr);
58		- }));
59	58
60		- process.nextTick(common.mustCall(() => {
61		- // Test with uncaughtExceptionCaptureCallback installed
62		- process.setUncaughtExceptionCaptureCallback(common.mustCall(
63		- (err) => assert.strictEqual(err, theErr))
64		- );
	59	+ process.nextTick(common.mustCall(() => {
	60	+ // Test with uncaughtExceptionCaptureCallback installed
	61	+ process.setUncaughtExceptionCaptureCallback(common.mustCall(
	62	+ (err) => assert.strictEqual(err, theErr))
	63	+ );
65	64
66		- throw theErr;
	65	+ throw theErr;
	66	+ }));
67	67	}));
68	68
69	69	throw theErr;

0 commit comments

Comments

0 (0)