From bb645c2a176673e259d90754fa4523fa1d8ef741 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 2 Nov 2015 17:56:24 -0800 Subject: [PATCH] domains: emit uncaughtException when appropriate Fix node exiting due to an exception being thrown rather than emitting an `'uncaughtException'` event on the process object when no error handler is set on the domain within which an error is thrown and an `'uncaughtException'` event listener is set on the process. Fixes #3607. --- lib/domain.js | 22 +-- test/parallel/test-domain-errors.js | 203 ++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-domain-errors.js diff --git a/lib/domain.js b/lib/domain.js index b6321a20f80db6..bda3b223fd0287 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -96,14 +96,20 @@ Domain.prototype._errorHandler = function errorHandler(er) { // that these exceptions are caught, and thus would prevent it from // aborting in these cases. if (stack.length === 1) { - try { - // Set the _emittingTopLevelDomainError so that we know that, even - // if technically the top-level domain is still active, it would - // be ok to abort on an uncaught exception at this point - process._emittingTopLevelDomainError = true; - caught = emitError(); - } finally { - process._emittingTopLevelDomainError = false; + // If there's no error handler, do not emit an 'error' event + // as this would throw an error, make the process exit, and thus + // prevent the process 'uncaughtException' event from being emitted + // if a listener is set. + if (self.listeners('error').length > 0) { + try { + // Set the _emittingTopLevelDomainError so that we know that, even + // if technically the top-level domain is still active, it would + // be ok to abort on an uncaught exception at this point + process._emittingTopLevelDomainError = true; + caught = emitError(); + } finally { + process._emittingTopLevelDomainError = false; + } } } else { // wrap this in a try/catch so we don't get infinite throwing diff --git a/test/parallel/test-domain-errors.js b/test/parallel/test-domain-errors.js new file mode 100644 index 00000000000000..3dda0fb131f6a1 --- /dev/null +++ b/test/parallel/test-domain-errors.js @@ -0,0 +1,203 @@ +'use strict'; + +/* + * The goal of this test is to make sure that errors thrown within domains + * are handled correctly. It checks that the process 'uncaughtException' event + * is emitted when appropriate, and not emitted when it shouldn't. It also + * checks that the proper domain error handlers are called when they should + * be called, and not called when they shouldn't. + */ + +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); +const child_process = require('child_process'); + +const uncaughtExceptions = {}; + +const tests = []; + +function test1() { + /* + * Throwing from an async callback from within a domain that doesn't have + * an error handler must result in emitting the process' uncaughtException + * event. + */ + const d = domain.create(); + d.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); +} + +tests.push({ + fn: test1, + expectedMessages: ['uncaughtException'] +}); + +function test2() { + /* + * Throwing from from within a domain that doesn't have an error handler must + * result in emitting the process' uncaughtException event. + */ + const d2 = domain.create(); + d2.run(function() { + throw new Error('boom!'); + }); +} + +tests.push({ + fn: test2, + expectedMessages: ['uncaughtException'] +}); + +function test3() { + /* + * This test creates two nested domains: d3 and d4. d4 doesn't register an + * error handler, but d3 does. The error is handled by the d3 domain and thus + * and 'uncaughtException' event should _not_ be emitted. + */ + const d3 = domain.create(); + const d4 = domain.create(); + + d3.on('error', function onErrorInD3Domain(err) { + process.send('errorHandledByDomain'); + }); + + d3.run(function() { + d4.run(function() { + throw new Error('boom!'); + }); + }); +} + +tests.push({ + fn: test3, + expectedMessages: ['errorHandledByDomain'] +}); + +function test4() { + /* + * This test creates two nested domains: d5 and d6. d6 doesn't register an + * error handler. When the timer's callback is called, because async + * operations like timer callbacks are bound to the domain that was active + * at the time of their creation, and because both d5 and d6 domains have + * exited by the time the timer's callback is called, its callback runs with + * only d6 on the domains stack. Since d6 doesn't register an error handler, + * the process' uncaughtException event should be emitted. + */ + const d5 = domain.create(); + const d6 = domain.create(); + + d5.on('error', function onErrorInD2Domain(err) { + process.send('errorHandledByDomain'); + }); + + d5.run(function() { + d6.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); + }); +} + +tests.push({ + fn: test4, + expectedMessages: ['uncaughtException'] +}); + +function test5() { + /* + * This test creates two nested domains: d7 and d4. d8 _does_ register an + * error handler, so throwing within that domain should not emit an uncaught + * exception. + */ + const d7 = domain.create(); + const d8 = domain.create(); + + d8.on('error', function onErrorInD3Domain(err) { + process.send('errorHandledByDomain'); + }); + + d7.run(function() { + d8.run(function() { + throw new Error('boom!'); + }); + }); +} +tests.push({ + fn: test5, + expectedMessages: ['errorHandledByDomain'] +}); + +function test6() { + /* + * This test creates two nested domains: d9 and d10. d10 _does_ register an + * error handler, so throwing within that domain in an async callback should + * _not_ emit an uncaught exception. + */ + const d9 = domain.create(); + const d10 = domain.create(); + + d10.on('error', function onErrorInD2Domain(err) { + process.send('errorHandledByDomain'); + }); + + d9.run(function() { + d10.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); + }); +} + +tests.push({ + fn: test6, + expectedMessages: ['errorHandledByDomain'] +}); + +if (process.argv[2] === 'child') { + const testIndex = process.argv[3]; + process.on('uncaughtException', function onUncaughtException() { + process.send('uncaughtException'); + }); + + tests[testIndex].fn(); +} else { + // Run each test's function in a child process. Listen on + // messages sent by each child process and compare expected + // messages defined for each ttest from the actual received messages. + tests.forEach(function doTest(test, testIndex) { + const testProcess = child_process.fork(__filename, ['child', testIndex]); + + testProcess.on('message', function onMsg(msg) { + if (test.messagesReceived === undefined) + test.messagesReceived = []; + + test.messagesReceived.push(msg); + }); + + testProcess.on('exit', function onExit() { + // Make sure that all expected messages were sent from the + // child process + test.expectedMessages.forEach(function(expectedMessage) { + if (test.messagesReceived === undefined || + test.messagesReceived.indexOf(expectedMessage) === -1) + assert(false, 'test ' + test.fn.name + ' should have sent message: ' + + expectedMessage + ' but didn\'t'); + }); + + if (test.messagesReceived) { + test.messagesReceived.forEach(function(receivedMessage) { + if (test.expectedMessages.indexOf(receivedMessage) === -1) { + assert(false, 'test ' + test.fn.name + + ' should have sent message: ' + receivedMessage + ' but did'); + } + }); + } + }); + }); +}