process,worker: fix process.exitCode handling for fatalException

doc/api/process.md

@@ -151,9 +151,13 @@ added: v0.1.18
 
 The `'uncaughtException'` event is emitted when an uncaught JavaScript
 exception bubbles all the way back to the event loop. By default, Node.js
-handles such exceptions by printing the stack trace to `stderr` and exiting.
+handles such exceptions by printing the stack trace to `stderr` and exiting
+with code 1, overriding any previously set [`process.exitCode`][].
 Adding a handler for the `'uncaughtException'` event overrides this default
-behavior.
+behavior. You may also change the [`process.exitCode`][] in
+`'uncaughtException'` handler which will result in process exiting with
+provided exit code, otherwise in the presence of such handler the process will
+exit with 0.
 
 The listener function is called with the `Error` object passed as the only
 argument.

lib/internal/bootstrap/node.js

@@ -475,6 +475,7 @@
         try {
           if (!process._exiting) {
             process._exiting = true;
+            process.exitCode = 1;
             process.emit('exit', 1);
           }
         } catch {

lib/internal/worker.js

@@ -454,9 +454,6 @@ function setupChild(evalScript) {
     debug(`[${threadId}] fatal exception caught = ${caught}`);
 
     if (!caught) {
-      // set correct code (uncaughtException) for [kOnExit](code) handler
-      process.exitCode = 1;
-
       let serialized;
       try {
         serialized = serializeError(error);
@@ -470,6 +467,8 @@ function setupChild(evalScript) {
       else
         port.postMessage({ type: messageTypes.COULD_NOT_SERIALIZE_ERROR });
       clearAsyncIdStack();
+
+      process.exit();
     }
   }
 }

src/node.cc

@@ -1438,7 +1438,16 @@ void FatalException(Isolate* isolate,
       exit(7);
     } else if (caught->IsFalse()) {
       ReportException(env, error, message);
-      exit(1);
+
+      // fatal_exception_function call before may have set a new exit code ->
+      // read it again, otherwise use default for uncaughtException 1
+      Local<String> exit_code = env->exit_code_string();
+      Local<Value> code;
+      if (!process_object->Get(env->context(), exit_code).ToLocal(&code) ||
+          !code->IsInt32()) {
+        exit(1);
+      }
+      exit(code.As<v8::Int32>()->Value());
     }
   }
 }

test/fixtures/process-exit-code-cases.js

@@ -0,0 +1,113 @@
+'use strict';
+
+const assert = require('assert');
+
+const cases = [];
+module.exports = cases;
+
+function exitsOnExitCodeSet() {
+  process.exitCode = 42;
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 42);
+    assert.strictEqual(code, 42);
+  });
+}
+cases.push({ func: exitsOnExitCodeSet, result: 42 });
+
+function changesCodeViaExit() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 42);
+    assert.strictEqual(code, 42);
+  });
+  process.exit(42);
+}
+cases.push({ func: changesCodeViaExit, result: 42 });
+
+function changesCodeZeroExit() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 0);
+    assert.strictEqual(code, 0);
+  });
+  process.exit(0);
+}
+cases.push({ func: changesCodeZeroExit, result: 0 });
+
+function exitWithOneOnUncaught() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    // cannot use assert because it will be uncaughtException -> 1 exit code
+    // that will render this test useless
+    if (code !== 1 || process.exitCode !== 1) {
+      console.log('wrong code! expected 1 for uncaughtException');
+      process.exit(99);
+    }
+  });
+  throw new Error('ok');
+}
+cases.push({
+  func: exitWithOneOnUncaught,
+  result: 1,
+  error: /^Error: ok$/,
+});
+
+function changeCodeInsideExit() {
+  process.exitCode = 95;
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 95);
+    assert.strictEqual(code, 95);
+    process.exitCode = 99;
+  });
+}
+cases.push({ func: changeCodeInsideExit, result: 99 });
+
+function zeroExitWithUncaughtHandler() {
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 0);
+    assert.strictEqual(code, 0);
+  });
+  process.on('uncaughtException', () => {});
+  throw new Error('ok');
+}
+cases.push({ func: zeroExitWithUncaughtHandler, result: 0 });
+
+function changeCodeInUncaughtHandler() {
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 97);
+    assert.strictEqual(code, 97);
+  });
+  process.on('uncaughtException', () => {
+    process.exitCode = 97;
+  });
+  throw new Error('ok');
+}
+cases.push({ func: changeCodeInUncaughtHandler, result: 97 });
+
+function changeCodeInExitWithUncaught() {
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 1);
+    assert.strictEqual(code, 1);
+    process.exitCode = 98;
+  });
+  throw new Error('ok');
+}
+cases.push({
+  func: changeCodeInExitWithUncaught,
+  result: 98,
+  error: /^Error: ok$/,
+});
+
+function exitWithZeroInExitWithUncaught() {
+  process.on('exit', (code) => {
+    assert.strictEqual(process.exitCode, 1);
+    assert.strictEqual(code, 1);
+    process.exitCode = 0;
+  });
+  throw new Error('ok');
+}
+cases.push({
+  func: exitWithZeroInExitWithUncaught,
+  result: 0,
+  error: /^Error: ok$/,
+});

test/parallel/test-process-exit-code.js

@@ -23,63 +23,18 @@
 require('../common');
 const assert = require('assert');
 
-switch (process.argv[2]) {
-  case 'child1':
-    return child1();
-  case 'child2':
-    return child2();
-  case 'child3':
-    return child3();
-  case 'child4':
-    return child4();
-  case 'child5':
-    return child5();
-  case undefined:
-    return parent();
-  default:
-    throw new Error('invalid');
-}
-
-function child1() {
-  process.exitCode = 42;
-  process.on('exit', function(code) {
-    assert.strictEqual(code, 42);
-  });
-}
-
-function child2() {
-  process.exitCode = 99;
-  process.on('exit', function(code) {
-    assert.strictEqual(code, 42);
-  });
-  process.exit(42);
-}
-
-function child3() {
-  process.exitCode = 99;
-  process.on('exit', function(code) {
-    assert.strictEqual(code, 0);
-  });
-  process.exit(0);
-}
-
-function child4() {
-  process.exitCode = 99;
-  process.on('exit', function(code) {
-    if (code !== 1) {
-      console.log('wrong code! expected 1 for uncaughtException');
-      process.exit(99);
-    }
-  });
-  throw new Error('ok');
-}
-
-function child5() {
-  process.exitCode = 95;
-  process.on('exit', function(code) {
-    assert.strictEqual(code, 95);
-    process.exitCode = 99;
-  });
+const testCases = require('../fixtures/process-exit-code-cases');
+
+if (!process.argv[2]) {
+  parent();
+} else {
+  const i = parseInt(process.argv[2]);
+  if (Number.isNaN(i)) {
+    console.log('Invalid test case index');
+    process.exit(100);
+    return;
+  }
+  testCases[i].func();
 }
 
 function parent() {
@@ -88,18 +43,14 @@ function parent() {
   const f = __filename;
   const option = { stdio: [ 0, 1, 'ignore' ] };
 
-  const test = (arg, exit) => {
+  const test = (arg, name = 'child', exit) => {
     spawn(node, [f, arg], option).on('exit', (code) => {
       assert.strictEqual(
         code, exit,
-        `wrong exit for ${arg}\nexpected:${exit} but got:${code}`);
+        `wrong exit for ${arg}-${name}\nexpected:${exit} but got:${code}`);
       console.log(`ok - ${arg} exited with ${exit}`);
     });
   };
 
-  test('child1', 42);
-  test('child2', 42);
-  test('child3', 0);
-  test('child4', 1);
-  test('child5', 99);
+  testCases.forEach((tc, i) => test(i, tc.func.name, tc.result));
 }