Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

worker: fix exit code for error thrown in uncaughtException handler #38012

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,17 +199,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