Skip to content

Commit 6986fa0

Browse files
Linkgoronjasnell
authored andcommitted
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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 1e4a2bc commit 6986fa0

File tree

4 files changed

+125
-97
lines changed

4 files changed

+125
-97
lines changed

lib/internal/main/worker_thread.js

+12
Original file line numberDiff line numberDiff line change
@@ -199,17 +199,29 @@ port.on('message', (message) => {
199199
function workerOnGlobalUncaughtException(error, fromPromise) {
200200
debug(`[${threadId}] gets uncaught exception`);
201201
let handled = false;
202+
let handlerThrew = false;
202203
try {
203204
handled = onGlobalUncaughtException(error, fromPromise);
204205
} catch (e) {
205206
error = e;
207+
handlerThrew = true;
206208
}
207209
debug(`[${threadId}] uncaught exception handled = ${handled}`);
208210

209211
if (handled) {
210212
return true;
211213
}
212214

215+
if (!process._exiting) {
216+
try {
217+
process._exiting = true;
218+
process.exitCode = 1;
219+
if (!handlerThrew) {
220+
process.emit('exit', process.exitCode);
221+
}
222+
} catch {}
223+
}
224+
213225
let serialized;
214226
try {
215227
const { serializeError } = require('internal/error_serdes');

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

+109-95
Original file line numberDiff line numberDiff line change
@@ -2,112 +2,126 @@
22

33
const assert = require('assert');
44

5-
const cases = [];
6-
module.exports = cases;
5+
function getTestCases(isWorker = false) {
6+
const cases = [];
7+
function exitsOnExitCodeSet() {
8+
process.exitCode = 42;
9+
process.on('exit', (code) => {
10+
assert.strictEqual(process.exitCode, 42);
11+
assert.strictEqual(code, 42);
12+
});
13+
}
14+
cases.push({ func: exitsOnExitCodeSet, result: 42 });
715

8-
function exitsOnExitCodeSet() {
9-
process.exitCode = 42;
10-
process.on('exit', (code) => {
11-
assert.strictEqual(process.exitCode, 42);
12-
assert.strictEqual(code, 42);
13-
});
14-
}
15-
cases.push({ func: exitsOnExitCodeSet, result: 42 });
16+
function changesCodeViaExit() {
17+
process.exitCode = 99;
18+
process.on('exit', (code) => {
19+
assert.strictEqual(process.exitCode, 42);
20+
assert.strictEqual(code, 42);
21+
});
22+
process.exit(42);
23+
}
24+
cases.push({ func: changesCodeViaExit, result: 42 });
1625

17-
function changesCodeViaExit() {
18-
process.exitCode = 99;
19-
process.on('exit', (code) => {
20-
assert.strictEqual(process.exitCode, 42);
21-
assert.strictEqual(code, 42);
22-
});
23-
process.exit(42);
24-
}
25-
cases.push({ func: changesCodeViaExit, result: 42 });
26+
function changesCodeZeroExit() {
27+
process.exitCode = 99;
28+
process.on('exit', (code) => {
29+
assert.strictEqual(process.exitCode, 0);
30+
assert.strictEqual(code, 0);
31+
});
32+
process.exit(0);
33+
}
34+
cases.push({ func: changesCodeZeroExit, result: 0 });
2635

27-
function changesCodeZeroExit() {
28-
process.exitCode = 99;
29-
process.on('exit', (code) => {
30-
assert.strictEqual(process.exitCode, 0);
31-
assert.strictEqual(code, 0);
36+
function exitWithOneOnUncaught() {
37+
process.exitCode = 99;
38+
process.on('exit', (code) => {
39+
// cannot use assert because it will be uncaughtException -> 1 exit code
40+
// that will render this test useless
41+
if (code !== 1 || process.exitCode !== 1) {
42+
console.log('wrong code! expected 1 for uncaughtException');
43+
process.exit(99);
44+
}
45+
});
46+
throw new Error('ok');
47+
}
48+
cases.push({
49+
func: exitWithOneOnUncaught,
50+
result: 1,
51+
error: /^Error: ok$/,
3252
});
33-
process.exit(0);
34-
}
35-
cases.push({ func: changesCodeZeroExit, result: 0 });
3653

37-
function exitWithOneOnUncaught() {
38-
process.exitCode = 99;
39-
process.on('exit', (code) => {
40-
// cannot use assert because it will be uncaughtException -> 1 exit code
41-
// that will render this test useless
42-
if (code !== 1 || process.exitCode !== 1) {
43-
console.log('wrong code! expected 1 for uncaughtException');
44-
process.exit(99);
45-
}
46-
});
47-
throw new Error('ok');
48-
}
49-
cases.push({
50-
func: exitWithOneOnUncaught,
51-
result: 1,
52-
error: /^Error: ok$/,
53-
});
54+
function changeCodeInsideExit() {
55+
process.exitCode = 95;
56+
process.on('exit', (code) => {
57+
assert.strictEqual(process.exitCode, 95);
58+
assert.strictEqual(code, 95);
59+
process.exitCode = 99;
60+
});
61+
}
62+
cases.push({ func: changeCodeInsideExit, result: 99 });
5463

55-
function changeCodeInsideExit() {
56-
process.exitCode = 95;
57-
process.on('exit', (code) => {
58-
assert.strictEqual(process.exitCode, 95);
59-
assert.strictEqual(code, 95);
60-
process.exitCode = 99;
61-
});
62-
}
63-
cases.push({ func: changeCodeInsideExit, result: 99 });
64+
function zeroExitWithUncaughtHandler() {
65+
process.on('exit', (code) => {
66+
assert.strictEqual(process.exitCode, 0);
67+
assert.strictEqual(code, 0);
68+
});
69+
process.on('uncaughtException', () => { });
70+
throw new Error('ok');
71+
}
72+
cases.push({ func: zeroExitWithUncaughtHandler, result: 0 });
6473

65-
function zeroExitWithUncaughtHandler() {
66-
process.on('exit', (code) => {
67-
assert.strictEqual(process.exitCode, 0);
68-
assert.strictEqual(code, 0);
69-
});
70-
process.on('uncaughtException', () => {});
71-
throw new Error('ok');
72-
}
73-
cases.push({ func: zeroExitWithUncaughtHandler, result: 0 });
74+
function changeCodeInUncaughtHandler() {
75+
process.on('exit', (code) => {
76+
assert.strictEqual(process.exitCode, 97);
77+
assert.strictEqual(code, 97);
78+
});
79+
process.on('uncaughtException', () => {
80+
process.exitCode = 97;
81+
});
82+
throw new Error('ok');
83+
}
84+
cases.push({ func: changeCodeInUncaughtHandler, result: 97 });
7485

75-
function changeCodeInUncaughtHandler() {
76-
process.on('exit', (code) => {
77-
assert.strictEqual(process.exitCode, 97);
78-
assert.strictEqual(code, 97);
79-
});
80-
process.on('uncaughtException', () => {
81-
process.exitCode = 97;
86+
function changeCodeInExitWithUncaught() {
87+
process.on('exit', (code) => {
88+
assert.strictEqual(process.exitCode, 1);
89+
assert.strictEqual(code, 1);
90+
process.exitCode = 98;
91+
});
92+
throw new Error('ok');
93+
}
94+
cases.push({
95+
func: changeCodeInExitWithUncaught,
96+
result: 98,
97+
error: /^Error: ok$/,
8298
});
83-
throw new Error('ok');
84-
}
85-
cases.push({ func: changeCodeInUncaughtHandler, result: 97 });
8699

87-
function changeCodeInExitWithUncaught() {
88-
process.on('exit', (code) => {
89-
assert.strictEqual(process.exitCode, 1);
90-
assert.strictEqual(code, 1);
91-
process.exitCode = 98;
100+
function exitWithZeroInExitWithUncaught() {
101+
process.on('exit', (code) => {
102+
assert.strictEqual(process.exitCode, 1);
103+
assert.strictEqual(code, 1);
104+
process.exitCode = 0;
105+
});
106+
throw new Error('ok');
107+
}
108+
cases.push({
109+
func: exitWithZeroInExitWithUncaught,
110+
result: 0,
111+
error: /^Error: ok$/,
92112
});
93-
throw new Error('ok');
94-
}
95-
cases.push({
96-
func: changeCodeInExitWithUncaught,
97-
result: 98,
98-
error: /^Error: ok$/,
99-
});
100113

101-
function exitWithZeroInExitWithUncaught() {
102-
process.on('exit', (code) => {
103-
assert.strictEqual(process.exitCode, 1);
104-
assert.strictEqual(code, 1);
105-
process.exitCode = 0;
114+
function exitWithThrowInUncaughtHandler() {
115+
process.on('uncaughtException', () => {
116+
throw new Error('ok')
117+
});
118+
throw new Error('bad');
119+
}
120+
cases.push({
121+
func: exitWithThrowInUncaughtHandler,
122+
result: isWorker ? 1 : 7,
123+
error: /^Error: ok$/,
106124
});
107-
throw new Error('ok');
125+
return cases;
108126
}
109-
cases.push({
110-
func: exitWithZeroInExitWithUncaught,
111-
result: 0,
112-
error: /^Error: ok$/,
113-
});
127+
exports.getTestCases = getTestCases;

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ require('../common');
2424
const assert = require('assert');
2525
const debug = require('util').debuglog('test');
2626

27-
const testCases = require('../fixtures/process-exit-code-cases');
27+
const { getTestCases } = require('../fixtures/process-exit-code-cases');
28+
const testCases = getTestCases(false);
2829

2930
if (!process.argv[2]) {
3031
parent();

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ const assert = require('assert');
88
const worker = require('worker_threads');
99
const { Worker, parentPort } = worker;
1010

11-
const testCases = require('../fixtures/process-exit-code-cases');
11+
const { getTestCases } = require('../fixtures/process-exit-code-cases');
12+
const testCases = getTestCases(true);
1213

1314
// Do not use isMainThread so that this test itself can be run inside a Worker.
1415
if (!process.env.HAS_STARTED_WORKER) {

0 commit comments

Comments
 (0)