mirror of https://github.com/lukechilds/node.git
Browse Source
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. Finally, change the behavior of --abort-on-uncaught-exception so that, if the domain within which the error is thrown has no error handler, but a domain further up the domains stack has one, the process will not abort. Fixes #3607 and #3653. PR: #3654 PR-URL: https://github.com/nodejs/node/pull/3654 Reviewed-By: Chris Dickinson <chris@neversaw.us>process-exit-stdio-flushing
Julien Gilli
9 years ago
9 changed files with 808 additions and 126 deletions
@ -0,0 +1,168 @@ |
|||||
|
'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. |
||||
|
*/ |
||||
|
const common = require('../common'); |
||||
|
const assert = require('assert'); |
||||
|
const domain = require('domain'); |
||||
|
const child_process = require('child_process'); |
||||
|
|
||||
|
const tests = [ |
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
const d2 = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
d2.run(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
setTimeout(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
setImmediate(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
process.nextTick(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
|
||||
|
d.run(function() { |
||||
|
var fs = require('fs'); |
||||
|
fs.exists('/non/existing/file', function onExists(exists) { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
const d2 = domain.create(); |
||||
|
|
||||
|
d.on('error', function errorHandler() { |
||||
|
}); |
||||
|
|
||||
|
d.run(function() { |
||||
|
d2.run(function() { |
||||
|
setTimeout(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
const d2 = domain.create(); |
||||
|
|
||||
|
d.on('error', function errorHandler() { |
||||
|
}); |
||||
|
|
||||
|
d.run(function() { |
||||
|
d2.run(function() { |
||||
|
setImmediate(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
const d2 = domain.create(); |
||||
|
|
||||
|
d.on('error', function errorHandler() { |
||||
|
}); |
||||
|
|
||||
|
d.run(function() { |
||||
|
d2.run(function() { |
||||
|
process.nextTick(function() { |
||||
|
throw new Error('boom!'); |
||||
|
}); |
||||
|
}); |
||||
|
}); |
||||
|
}, |
||||
|
|
||||
|
function() { |
||||
|
const d = domain.create(); |
||||
|
const 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') { |
||||
|
const 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) { |
||||
|
const errMsg = 'Test at index ' + testIndex + ' should have aborted ' + |
||||
|
'but instead exited with exit code ' + exitCode + ' and signal ' + |
||||
|
signal; |
||||
|
assert(common.nodeProcessAborted(exitCode, signal), errMsg); |
||||
|
}); |
||||
|
}); |
||||
|
} |
@ -0,0 +1,101 @@ |
|||||
|
'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.
|
||||
|
|
||||
|
const common = require('../common'); |
||||
|
const assert = require('assert'); |
||||
|
const child_process = require('child_process'); |
||||
|
const domain = require('domain'); |
||||
|
|
||||
|
const uncaughtExceptionHandlerErrMsg = 'boom from uncaughtException handler'; |
||||
|
const domainErrMsg = 'boom from domain'; |
||||
|
|
||||
|
const RAN_UNCAUGHT_EXCEPTION_HANDLER_EXIT_CODE = 42; |
||||
|
|
||||
|
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); |
||||
|
} |
||||
|
})); |
||||
|
|
||||
|
const 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.includes(uncaughtExceptionHandlerErrMsg), |
||||
|
'stderr output must include proper uncaughtException handler\'s ' + |
||||
|
'error\'s message'); |
||||
|
assert(!stderr.includes(domainErrMsg), '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; |
||||
|
} |
@ -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. |
||||
|
*/ |
||||
|
|
||||
|
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 |
||||
|
* an '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 d8. 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 test with 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('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'); |
||||
|
} |
||||
|
}); |
||||
|
} |
||||
|
})); |
||||
|
}); |
||||
|
} |
Loading…
Reference in new issue