worker: fix exit code for error thrown in handler

Change worker exit code when the unhandled exception handler throws from 0 to 7 fixes: #37996 PR-URL: #38012 Fixes: #37996 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · May 1, 2021 · b6f4d790d313fed0b942b752775958eaeba707ff · b6f4d79
1 parent f1aae43
commit b6f4d790d313fed0b942b752775958eaeba707ff
Showing with 125 additions and 97 deletions.

+12 −0 lib/internal/main/worker_thread.js

+109 −95 test/fixtures/process-exit-code-cases.js

+2 −1 test/parallel/test-process-exit-code.js

+2 −1 test/parallel/test-worker-exit-code.js
diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -193,17 +193,29 @@ port.on('message', (message) => {
 function workerOnGlobalUncaughtException(error, fromPromise) {
   debug(`[${threadId}] gets uncaught exception`);
   let handled = false;
+  let handlerThrew = false;
   try {
     handled = onGlobalUncaughtException(error, fromPromise);
   } catch (e) {
     error = e;
+    handlerThrew = true;
   }
   debug(`[${threadId}] uncaught exception handled = ${handled}`);
 
   if (handled) {
     return true;
   }
 
+  if (!process._exiting) {
+    try {
+      process._exiting = true;
+      process.exitCode = 1;
+      if (!handlerThrew) {
+        process.emit('exit', process.exitCode);
+      }
+    } catch {}
+  }
+
   let serialized;
   try {
     const { serializeError } = require('internal/error_serdes');

diff --git a/test/fixtures/process-exit-code-cases.js b/test/fixtures/process-exit-code-cases.js
@@ -2,112 +2,126 @@
 
 const assert = require('assert');
 
-const cases = [];
-module.exports = cases;
+function getTestCases(isWorker = false) {
+  const 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 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 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 changesCodeZeroExit() {
-  process.exitCode = 99;
-  process.on('exit', (code) => {
-    assert.strictEqual(process.exitCode, 0);
-    assert.strictEqual(code, 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$/,
   });
-  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 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 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 changeCodeInUncaughtHandler() {
-  process.on('exit', (code) => {
-    assert.strictEqual(process.exitCode, 97);
-    assert.strictEqual(code, 97);
-  });
-  process.on('uncaughtException', () => {
-    process.exitCode = 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$/,
   });
-  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;
+  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$/,
   });
-  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;
+  function exitWithThrowInUncaughtHandler() {
+    process.on('uncaughtException', () => {
+      throw new Error('ok')
+    });
+    throw new Error('bad');
+  }
+  cases.push({
+    func: exitWithThrowInUncaughtHandler,
+    result: isWorker ? 1 : 7,
+    error: /^Error: ok$/,
   });
-  throw new Error('ok');
+  return cases;
 }
-cases.push({
-  func: exitWithZeroInExitWithUncaught,
-  result: 0,
-  error: /^Error: ok$/,
-});
+exports.getTestCases = getTestCases;
diff --git a/test/parallel/test-process-exit-code.js b/test/parallel/test-process-exit-code.js
@@ -24,7 +24,8 @@ require('../common');
 const assert = require('assert');
 const debug = require('util').debuglog('test');
 
-const testCases = require('../fixtures/process-exit-code-cases');
+const { getTestCases } = require('../fixtures/process-exit-code-cases');
+const testCases = getTestCases(false);
 
 if (!process.argv[2]) {
   parent();

diff --git a/test/parallel/test-worker-exit-code.js b/test/parallel/test-worker-exit-code.js
@@ -8,7 +8,8 @@ const assert = require('assert');
 const worker = require('worker_threads');
 const { Worker, parentPort } = worker;
 
-const testCases = require('../fixtures/process-exit-code-cases');
+const { getTestCases } = require('../fixtures/process-exit-code-cases');
+const testCases = getTestCases(true);
 
 // Do not use isMainThread so that this test itself can be run inside a Worker.
 if (!process.env.HAS_STARTED_WORKER) {