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.

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>

1 files changed, 101 insertions, 0 deletions

diff --git a/test/parallel/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js b/test/parallel/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js
new file mode 100644
index 0000000000..499988107d
--- /dev/null
+++ b/test/parallel/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js
@@ -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;
+}