From 018e4e0b1a629c807135df32264b1d92818965db Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 2 Nov 2015 17:31:28 -0800 Subject: [PATCH] domains: fix handling of uncaught exceptions Fix node exiting due to an exception being thrown rather than emitting an 'uncaughtException' event on the process object when: 1. no error handler is set on the domain within which an error is thrown 2. an 'uncaughtException' event listener is set on the process Also fix an issue where the process would not abort in the proper function call if an error is thrown within a domain with no error handler and --abort-on-uncaught-exception is used. Fixes #3607 and #3653. PR: #3885 PR-URL: https://github.com/nodejs/node/pull/3885 Reviewed-By: James M Snell --- lib/domain.js | 33 ++- src/env.h | 1 + src/node.cc | 54 +++- test/common.js | 34 +++ test/simple/test-domain-abort-on-uncaught.js | 243 ++++++++++++++++++ ...main-no-error-handler-abort-on-uncaught.js | 190 ++++++++++++++ ...n-throw-from-uncaught-exception-handler.js | 102 ++++++++ test/simple/test-domain-uncaught-exception.js | 205 +++++++++++++++ ...domain-with-abort-on-uncaught-exception.js | 58 +---- 9 files changed, 854 insertions(+), 66 deletions(-) create mode 100644 test/simple/test-domain-abort-on-uncaught.js create mode 100644 test/simple/test-domain-no-error-handler-abort-on-uncaught.js create mode 100644 test/simple/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js create mode 100644 test/simple/test-domain-uncaught-exception.js diff --git a/lib/domain.js b/lib/domain.js index 87a47076dd2d25..c896eb8a4f76c2 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -45,8 +45,13 @@ Object.defineProperty(process, 'domain', { // between js and c++ w/o much overhead var _domain_flag = {}; +// it's possible to enter one domain while already inside +// another one. the stack is each entered domain. +var stack = []; +exports._stack = stack; + // let the process know we're using domains -process._setupDomainUse(_domain, _domain_flag); +process._setupDomainUse(_domain, _domain_flag, stack); exports.Domain = Domain; @@ -54,10 +59,6 @@ exports.create = exports.createDomain = function() { return new Domain(); }; -// it's possible to enter one domain while already inside -// another one. the stack is each entered domain. -var stack = []; -exports._stack = stack; // the active domain is always the one that we're currently in. exports.active = null; @@ -114,14 +115,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 (EventEmitter.listenerCount(self, 'error') > 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/src/env.h b/src/env.h index 2768388f3e6b0a..733e21d5f96f30 100644 --- a/src/env.h +++ b/src/env.h @@ -258,6 +258,7 @@ namespace node { V(buffer_constructor_function, v8::Function) \ V(context, v8::Context) \ V(domain_array, v8::Array) \ + V(domains_stack_array, v8::Array) \ V(fs_stats_constructor_function, v8::Function) \ V(gc_info_callback_function, v8::Function) \ V(module_load_list_array, v8::Array) \ diff --git a/src/node.cc b/src/node.cc index 88550f7a414e58..2335988bcd6b62 100644 --- a/src/node.cc +++ b/src/node.cc @@ -910,19 +910,53 @@ Local WinapiErrnoException(Isolate* isolate, } #endif +static bool DomainHasErrorHandler(const Environment* env, + const Local& domain) { + HandleScope scope(env->isolate()); -static bool IsDomainActive(const Environment* env) { - if (!env->using_domains()) { + Local domain_event_listeners_v = domain->Get(env->events_string()); + if (!domain_event_listeners_v->IsObject()) return false; - } - Local domain_array = env->domain_array().As(); - uint32_t domains_array_length = domain_array->Length(); - if (domains_array_length == 0) + Local domain_event_listeners_o = + domain_event_listeners_v.As(); + + Local domain_error_listeners_v = + domain_event_listeners_o->Get(env->error_string()); + + if (domain_error_listeners_v->IsFunction() || + (domain_error_listeners_v->IsArray() && + domain_error_listeners_v.As()->Length() > 0)) + return true; + + return false; +} + +static bool TopDomainHasErrorHandler(const Environment* env) { + HandleScope scope(env->isolate()); + + if (!env->using_domains()) return false; - Local domain_v = domain_array->Get(0); - return !domain_v->IsNull(); + Local domains_stack_array = env->domains_stack_array().As(); + if (domains_stack_array->Length() == 0) + return false; + + uint32_t domains_stack_length = domains_stack_array->Length(); + if (domains_stack_length == 0) + return false; + + Local top_domain_v = + domains_stack_array->Get(domains_stack_length - 1); + + if (!top_domain_v->IsObject()) + return false; + + Local top_domain = top_domain_v.As(); + if (DomainHasErrorHandler(env, top_domain)) + return true; + + return false; } @@ -934,7 +968,7 @@ bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) { bool isEmittingTopLevelDomainError = process_object->Get(emitting_top_level_domain_error_key)->BooleanValue(); - return !IsDomainActive(env) || isEmittingTopLevelDomainError; + return isEmittingTopLevelDomainError || !TopDomainHasErrorHandler(env); } @@ -962,8 +996,10 @@ void SetupDomainUse(const FunctionCallbackInfo& args) { assert(args[0]->IsArray()); assert(args[1]->IsObject()); + assert(args[2]->IsArray()); env->set_domain_array(args[0].As()); + env->set_domains_stack_array(args[2].As()); Local domain_flag_obj = args[1].As(); Environment::DomainFlag* domain_flag = env->domain_flag(); diff --git a/test/common.js b/test/common.js index 478e2e92a595ed..f5c1bacb36176b 100644 --- a/test/common.js +++ b/test/common.js @@ -347,3 +347,37 @@ exports.busyLoop = function busyLoop(time) { ; } }; + +// Returns true if the exit code "exitCode" and/or signal name "signal" +// represent the exit code and/or signal name of a node process that aborted, +// false otherwise. +exports.nodeProcessAborted = function nodeProcessAborted(exitCode, signal) { + // Depending on the compiler used, node will exit with either + // exit code 132 (SIGILL), 133 (SIGTRAP) or 134 (SIGABRT). + var expectedExitCodes = [132, 133, 134]; + + // On platforms using KSH as the default shell (like SmartOS), + // when a process aborts, KSH exits with an exit code that is + // greater than 256, and thus the exit code emitted with the 'exit' + // event is null and the signal is set to either SIGILL, SIGTRAP, + // or SIGABRT (depending on the compiler). + var expectedSignals = ['SIGILL', 'SIGTRAP', 'SIGABRT']; + + // On Windows, v8's base::OS::Abort triggers an access violation, + // which corresponds to exit code 3221225477 (0xC0000005) + if (process.platform === 'win32') + expectedExitCodes = [3221225477]; + + // When using --abort-on-uncaught-exception, V8 will use + // base::OS::Abort to terminate the process. + // Depending on the compiler used, the shell or other aspects of + // the platform used to build the node binary, this will actually + // make V8 exit by aborting or by raising a signal. In any case, + // one of them (exit code or signal) needs to be set to one of + // the expected exit codes or signals. + if (signal !== null) { + return expectedSignals.indexOf(signal) > -1; + } else { + return expectedExitCodes.indexOf(exitCode) > -1; + } +}; diff --git a/test/simple/test-domain-abort-on-uncaught.js b/test/simple/test-domain-abort-on-uncaught.js new file mode 100644 index 00000000000000..bfe9460241f874 --- /dev/null +++ b/test/simple/test-domain-abort-on-uncaught.js @@ -0,0 +1,243 @@ +'use strict'; + +// This test makes sure that when using --abort-on-uncaught-exception and +// when throwing an error from within a domain that has an error handler +// setup, the process _does not_ abort. + +var common = require('../common'); +var assert = require('assert'); +var domain = require('domain'); +var child_process = require('child_process'); + +var errorHandlerCalled = false; + +var tests = [ + function nextTick() { + var d = domain.create(); + + d.once('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + process.nextTick(function() { + throw new Error('exceptional!'); + }); + }); + }, + + function timer() { + var d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + setTimeout(function() { + throw new Error('exceptional!'); + }, 33); + }); + }, + + function immediate() { + console.log('starting test'); + var d = domain.create(); + + d.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }, + + function timerPlusNextTick() { + var d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + setTimeout(function() { + process.nextTick(function() { + throw new Error('exceptional!'); + }); + }, 33); + }); + }, + + function firstRun() { + var d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + throw new Error('exceptional!'); + }); + }, + + function fsAsync() { + var d = domain.create(); + + d.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }, + + function netServer() { + var net = require('net'); + var d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + var server = net.createServer(function(conn) { + conn.pipe(conn); + }); + server.listen(common.PORT, '0.0.0.0', function() { + var conn = net.connect(common.PORT, '0.0.0.0'); + conn.once('data', function() { + throw new Error('ok'); + }); + conn.end('ok'); + server.close(); + }); + }); + }, + + function firstRunNestedWithErrorHandler() { + var d = domain.create(); + var d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + function timeoutNestedWithErrorHandler() { + var d = domain.create(); + var d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + setTimeout(function() { + console.log('foo'); + throw new Error('boom!'); + }, 33); + }); + }); + }, + + function setImmediateNestedWithErrorHandler() { + var d = domain.create(); + var d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function nextTickNestedWithErrorHandler() { + var d = domain.create(); + var d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function fsAsyncNestedWithErrorHandler() { + var d = domain.create(); + var d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }); + } +]; + +if (process.argv[2] === 'child') { + var testIndex = +process.argv[3]; + + tests[testIndex](); + + process.on('exit', function onExit() { + assert.equal(errorHandlerCalled, true); + }); +} else { + + tests.forEach(function(test, testIndex) { + var testCmd = ''; + if (process.platform !== 'win32') { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines + testCmd += 'ulimit -c 0 && '; + } + + testCmd += process.argv[0]; + testCmd += ' ' + '--abort-on-uncaught-exception'; + testCmd += ' ' + process.argv[1]; + testCmd += ' ' + 'child'; + testCmd += ' ' + testIndex; + + var child = child_process.exec(testCmd); + + child.on('exit', function onExit(code, signal) { + assert.equal(code, 0, 'Test at index ' + testIndex + + ' should have exited with exit code 0 but instead exited with code ' + + code + ' and signal ' + signal); + }); + + }); +} diff --git a/test/simple/test-domain-no-error-handler-abort-on-uncaught.js b/test/simple/test-domain-no-error-handler-abort-on-uncaught.js new file mode 100644 index 00000000000000..287fac521f1c98 --- /dev/null +++ b/test/simple/test-domain-no-error-handler-abort-on-uncaught.js @@ -0,0 +1,190 @@ +'use strict'; + +/* + * This test makes sure that when using --abort-on-uncaught-exception and + * when throwing an error from within a domain that does not have an error + * handler setup, the process aborts. + */ +var common = require('../common'); +var assert = require('assert'); +var domain = require('domain'); +var child_process = require('child_process'); + +var tests = [ + function() { + var d = domain.create(); + + d.run(function() { + throw new Error('boom!'); + }); + }, + + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + /* + * In this case, only the domain at the top of the stack "d" has an error + * handler, but the domain from which the error is thrown doesn't have an + * error handler. To be consistent with the way domains bubble up errors, + * this test case _should not_ make the process abort. However it was + * determined that making such a change in v0.12.x might be a breaking + * change, so instead this test case makes sure the process aborts. + */ + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + var d = domain.create(); + + d.run(function() { + setTimeout(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + var d = domain.create(); + + d.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + var d = domain.create(); + + d.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + var d = domain.create(); + + d.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }, + + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + setTimeout(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + var d = domain.create(); + var d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }); + }, +]; + +if (process.argv[2] === 'child') { + var testIndex = +process.argv[3]; + tests[testIndex](); +} else { + + tests.forEach(function(test, testIndex) { + var testCmd = ''; + if (process.platform !== 'win32') { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines + testCmd += 'ulimit -c 0 && '; + } + + testCmd += process.argv[0]; + testCmd += ' ' + '--abort-on-uncaught-exception'; + testCmd += ' ' + process.argv[1]; + testCmd += ' ' + 'child'; + testCmd += ' ' + testIndex; + + var child = child_process.exec(testCmd); + + child.on('exit', function onExit(exitCode, signal) { + var errMsg = 'Test at index ' + testIndex + ' should have aborted ' + + 'but instead exited with exit code ' + exitCode + ' and signal ' + + signal; + assert(common.nodeProcessAborted(exitCode, signal), errMsg); + }); + }); +} diff --git a/test/simple/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js b/test/simple/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js new file mode 100644 index 00000000000000..5339af332bb62c --- /dev/null +++ b/test/simple/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js @@ -0,0 +1,102 @@ +'use strict'; + +// This test makes sure that when throwing an error from a domain, and then +// handling that error in an uncaughtException handler by throwing an error +// again, the exit code, signal and error messages are the ones we expect with +// and without using --abort-on-uncaught-exception. + +var common = require('../common'); +var assert = require('assert'); +var child_process = require('child_process'); +var domain = require('domain'); + +var uncaughtExceptionHandlerErrMsg = 'boom from uncaughtException handler'; +var domainErrMsg = 'boom from domain'; + +var RAN_UNCAUGHT_EXCEPTION_HANDLER_EXIT_CODE = 42; +var d; + +if (process.argv[2] === 'child') { + process.on('uncaughtException', common.mustCall(function onUncaught() { + if (process.execArgv.indexOf('--abort-on-uncaught-exception') !== -1) { + // When passing --abort-on-uncaught-exception to the child process, + // we want to make sure that this handler (the process' uncaughtException + // event handler) wasn't called. Unfortunately we can't parse the child + // process' output to do that, since on Windows the standard error output + // is not properly flushed in V8's Isolate::Throw right before the + // process aborts due to an uncaught exception, and thus the error + // message representing the error that was thrown cannot be read by the + // parent process. So instead of parsing the child process' stdandard + // error, the parent process will check that in the case + // --abort-on-uncaught-exception was passed, the process did not exit + // with exit code RAN_UNCAUGHT_EXCEPTION_HANDLER_EXIT_CODE. + process.exit(RAN_UNCAUGHT_EXCEPTION_HANDLER_EXIT_CODE); + } else { + // On the other hand, when not passing --abort-on-uncaught-exception to + // the node process, we want to throw in this event handler to make sure + // that the proper error message, exit code and signal are the ones we + // expect. + throw new Error(uncaughtExceptionHandlerErrMsg); + } + })); + + d = domain.create(); + d.run(common.mustCall(function() { + throw new Error(domainErrMsg); + })); +} else { + runTestWithoutAbortOnUncaughtException(); + runTestWithAbortOnUncaughtException(); +} + +function runTestWithoutAbortOnUncaughtException() { + child_process.exec(createTestCmdLine(), + function onTestDone(err, stdout, stderr) { + // When _not_ passing --abort-on-uncaught-exception, the process' + // uncaughtException handler _must_ be called, and thus the error + // message must include only the message of the error thrown from the + // process' uncaughtException handler. + assert(stderr.indexOf(uncaughtExceptionHandlerErrMsg) !== -1, + 'stderr output must include proper uncaughtException handler\'s ' + + 'error\'s message'); + assert(stderr.indexOf(domainErrMsg) === -1, 'stderr output must not ' + + 'include domain\'s error\'s message'); + + assert.notEqual(err.code, 0, + 'child process should have exited with a non-zero exit code, ' + + 'but did not'); + }); +} + +function runTestWithAbortOnUncaughtException() { + child_process.exec(createTestCmdLine({ + withAbortOnUncaughtException: true + }), function onTestDone(err, stdout, stderr) { + assert.notEqual(err.code, RAN_UNCAUGHT_EXCEPTION_HANDLER_EXIT_CODE, + 'child process should not have run its uncaughtException event ' + + 'handler'); + assert(common.nodeProcessAborted(err.code, err.signal), + 'process should have aborted, but did not'); + }); +} + +function createTestCmdLine(options) { + var testCmd = ''; + + if (process.platform !== 'win32') { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines + testCmd += 'ulimit -c 0 && '; + } + + testCmd += process.argv[0]; + + if (options && options.withAbortOnUncaughtException) { + testCmd += ' ' + '--abort-on-uncaught-exception'; + } + + testCmd += ' ' + process.argv[1]; + testCmd += ' ' + 'child'; + + return testCmd; +} diff --git a/test/simple/test-domain-uncaught-exception.js b/test/simple/test-domain-uncaught-exception.js new file mode 100644 index 00000000000000..105f7b1ab0a6c8 --- /dev/null +++ b/test/simple/test-domain-uncaught-exception.js @@ -0,0 +1,205 @@ +'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. + */ + +var common = require('../common'); +var assert = require('assert'); +var domain = require('domain'); +var child_process = require('child_process'); + +var uncaughtExceptions = {}; + +var 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. + */ + var 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. + */ + var 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 + * an 'uncaughtException' event should _not_ be emitted. + */ + var d3 = domain.create(); + var 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. + */ + var d5 = domain.create(); + var 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 d8. d8 _does_ register an + * error handler, so throwing within that domain should not emit an uncaught + * exception. + */ + var d7 = domain.create(); + var 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. + */ + var d9 = domain.create(); + var 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') { + var 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 test with the actual received messages. + tests.forEach(function doTest(test, testIndex) { + var 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('disconnect', common.mustCall(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 not have sent message: ' + receivedMessage + + ' but did'); + } + }); + } + })); + }); +} diff --git a/test/simple/test-domain-with-abort-on-uncaught-exception.js b/test/simple/test-domain-with-abort-on-uncaught-exception.js index 147540e4c3a686..adf18f63bb31ff 100644 --- a/test/simple/test-domain-with-abort-on-uncaught-exception.js +++ b/test/simple/test-domain-with-abort-on-uncaught-exception.js @@ -21,6 +21,8 @@ var assert = require('assert'); +var common = require('../common'); + /* * The goal of this test is to make sure that: * @@ -139,59 +141,27 @@ if (process.argv[2] === 'child') { }); child.on('exit', function onChildExited(exitCode, signal) { - var expectedExitCode = 0; - // We use an array of values since the actual signal can differ across - // compilers. - var expectedSignal = [null]; - // When throwing errors from the top-level domain error handler // outside of a try/catch block, the process should not exit gracefully if (!options.useTryCatch && options.throwInDomainErrHandler) { // If the top-level domain's error handler does not throw, // the process must exit gracefully, whether or not // --abort_on_uncaught_exception was passed on the command line - expectedExitCode = 7; if (cmdLineOption === '--abort_on_uncaught_exception') { - // If the top-level domain's error handler throws, and only if - // --abort_on_uncaught_exception is passed on the command line, - // the process must abort. - // - // We use an array of values since the actual exit code can differ - // across compilers. - expectedExitCode = [132, 134]; - - // On Linux, v8 raises SIGTRAP when aborting because - // the "debug break" flag is on by default - if (process.platform === 'linux') - expectedExitCode.push(133); - - // On some platforms with KSH being the default shell - // (like SmartOS), when a process aborts, KSH exits with an exit - // code that is greater than 256, and thus the exit code emitted - // with the 'exit' event is null and the signal is set to either - // SIGABRT or SIGILL. - if (process.platform === 'sunos') { - expectedExitCode = null; - expectedSignal = ['SIGABRT', 'SIGILL']; - } - - // On Windows, v8's base::OS::Abort also triggers a debug breakpoint - // which makes the process exit with code -2147483645 - if (process.platform === 'win32') - expectedExitCode = [-2147483645, 3221225477]; + assert(common.nodeProcessAborted(exitCode, signal), + 'process should have aborted, but did not'); + } else { + // By default, uncaught exceptions make node exit with an exit + // code of 7. + assert.equal(exitCode, 7); + assert.equal(signal, null); } - } - - if (Array.isArray(expectedSignal)) { - assert.ok(expectedSignal.indexOf(signal) > -1); - } else { - assert.equal(signal, expectedSignal); - } - - if (Array.isArray(expectedExitCode)) { - assert.ok(expectedExitCode.indexOf(exitCode) > -1); } else { - assert.equal(exitCode, expectedExitCode); + // If the top-level domain's error handler does not throw, + // the process must exit gracefully, whether or not + // --abort_on_uncaught_exception was passed on the command line + assert.equal(exitCode, 0); + assert.equal(signal, null); } }); }