diff --git a/doc/api/process.md b/doc/api/process.md index edf01258c3941b..df544e7f0f17b9 100644 --- a/doc/api/process.md +++ b/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. diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index e433c67aa42c8b..016c0c5e2304bc 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -475,6 +475,7 @@ try { if (!process._exiting) { process._exiting = true; + process.exitCode = 1; process.emit('exit', 1); } } catch { diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 83389d204d285f..26c16f86e3e8fc 100644 --- a/lib/internal/worker.js +++ b/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(); } } } diff --git a/src/node.cc b/src/node.cc index f9ac48c0770221..f7eae49bd0e5c2 100644 --- a/src/node.cc +++ b/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 exit_code = env->exit_code_string(); + Local code; + if (!process_object->Get(env->context(), exit_code).ToLocal(&code) || + !code->IsInt32()) { + exit(1); + } + exit(code.As()->Value()); } } } diff --git a/test/fixtures/process-exit-code-cases.js b/test/fixtures/process-exit-code-cases.js new file mode 100644 index 00000000000000..c8c4a2ebe6c7ff --- /dev/null +++ b/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$/, +}); diff --git a/test/parallel/test-process-exit-code.js b/test/parallel/test-process-exit-code.js index f5f8099c8d2439..9059b0b5c22487 100644 --- a/test/parallel/test-process-exit-code.js +++ b/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)); } diff --git a/test/parallel/test-worker-exit-code.js b/test/parallel/test-worker-exit-code.js index b621389b49ca6b..f09b834f853b12 100644 --- a/test/parallel/test-worker-exit-code.js +++ b/test/parallel/test-worker-exit-code.js @@ -9,6 +9,8 @@ const assert = require('assert'); const worker = require('worker_threads'); const { Worker, parentPort } = worker; +const testCases = require('../fixtures/process-exit-code-cases'); + // Do not use isMainThread so that this test itself can be run inside a Worker. if (!process.env.HAS_STARTED_WORKER) { process.env.HAS_STARTED_WORKER = 1; @@ -19,114 +21,27 @@ if (!process.env.HAS_STARTED_WORKER) { process.exit(100); return; } - parentPort.once('message', (msg) => { - switch (msg) { - case 'child1': - return child1(); - case 'child2': - return child2(); - case 'child3': - return child3(); - case 'child4': - return child4(); - case 'child5': - return child5(); - case 'child6': - return child6(); - case 'child7': - return child7(); - default: - throw new Error('invalid'); - } - }); -} - -function child1() { - process.exitCode = 42; - process.on('exit', (code) => { - assert.strictEqual(code, 42); - }); -} - -function child2() { - process.exitCode = 99; - process.on('exit', (code) => { - assert.strictEqual(code, 42); - }); - process.exit(42); -} - -function child3() { - process.exitCode = 99; - process.on('exit', (code) => { - assert.strictEqual(code, 0); - }); - process.exit(0); -} - -function child4() { - 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) { - console.error('wrong code! expected 1 for uncaughtException'); - process.exit(99); - } - }); - throw new Error('ok'); -} - -function child5() { - process.exitCode = 95; - process.on('exit', (code) => { - assert.strictEqual(code, 95); - process.exitCode = 99; - }); -} - -function child6() { - process.on('exit', (code) => { - assert.strictEqual(code, 0); - }); - process.on('uncaughtException', common.mustCall(() => { - // handle - })); - throw new Error('ok'); -} - -function child7() { - process.on('exit', (code) => { - assert.strictEqual(code, 97); - }); - process.on('uncaughtException', common.mustCall(() => { - process.exitCode = 97; - })); - throw new Error('ok'); + parentPort.once('message', (msg) => testCases[msg].func()); } function parent() { - const test = (arg, exit, error = null) => { + const test = (arg, name = 'worker', exit, error = null) => { const w = new Worker(__filename); w.on('exit', common.mustCall((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}`); })); if (error) { w.on('error', common.mustCall((err) => { - assert(error.test(err)); + console.log(err); + assert(error.test(err), + `wrong error for ${arg}\nexpected:${error} but got:${err}`); })); } w.postMessage(arg); }; - test('child1', 42); - test('child2', 42); - test('child3', 0); - test('child4', 1, /^Error: ok$/); - test('child5', 99); - test('child6', 0); - test('child7', 97); + testCases.forEach((tc, i) => test(i, tc.func.name, tc.result, tc.error)); } diff --git a/test/parallel/test-worker-uncaught-exception-async.js b/test/parallel/test-worker-uncaught-exception-async.js index f820976c11ebcd..862b1a66d619c3 100644 --- a/test/parallel/test-worker-uncaught-exception-async.js +++ b/test/parallel/test-worker-uncaught-exception-async.js @@ -10,6 +10,7 @@ if (!process.env.HAS_STARTED_WORKER) { const w = new Worker(__filename); w.on('message', common.mustNotCall()); w.on('error', common.mustCall((err) => { + console.log(err.message); assert(/^Error: foo$/.test(err)); })); w.on('exit', common.mustCall((code) => { @@ -17,6 +18,19 @@ if (!process.env.HAS_STARTED_WORKER) { assert.strictEqual(code, 1); })); } else { + // cannot use common.mustCall as it cannot catch this + let called = false; + process.on('exit', (code) => { + if (!called) { + called = true; + } else { + assert.fail('Exit callback called twice in worker'); + } + }); + + setTimeout(() => assert.fail('Timeout executed after uncaughtException'), + 2000); + setImmediate(() => { throw new Error('foo'); }); diff --git a/test/parallel/test-worker-uncaught-exception.js b/test/parallel/test-worker-uncaught-exception.js index 67b861e22619aa..95c142b6c70d64 100644 --- a/test/parallel/test-worker-uncaught-exception.js +++ b/test/parallel/test-worker-uncaught-exception.js @@ -10,6 +10,7 @@ if (!process.env.HAS_STARTED_WORKER) { const w = new Worker(__filename); w.on('message', common.mustNotCall()); w.on('error', common.mustCall((err) => { + console.log(err.message); assert(/^Error: foo$/.test(err)); })); w.on('exit', common.mustCall((code) => { @@ -17,5 +18,18 @@ if (!process.env.HAS_STARTED_WORKER) { assert.strictEqual(code, 1); })); } else { + // cannot use common.mustCall as it cannot catch this + let called = false; + process.on('exit', (code) => { + if (!called) { + called = true; + } else { + assert.fail('Exit callback called twice in worker'); + } + }); + + setTimeout(() => assert.fail('Timeout executed after uncaughtException'), + 2000); + throw new Error('foo'); }