Skip to content

Commit

Permalink
worker: fix exit code for error thrown in handler
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Linkgoron authored and targos committed May 1, 2021
1 parent f1aae43 commit b6f4d79
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 97 deletions.
12 changes: 12 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
204 changes: 109 additions & 95 deletions test/fixtures/process-exit-code-cases.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
3 changes: 2 additions & 1 deletion test/parallel/test-process-exit-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-worker-exit-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b6f4d79

Please sign in to comment.