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

process: Change default --unhandled-rejections=warn-with-error-code #33033

Closed
wants to merge 1 commit into from
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
9 changes: 3 additions & 6 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -978,20 +978,17 @@ added:
- v10.17.0
-->

By default all unhandled rejections trigger a warning plus a deprecation warning
for the very first unhandled rejection in case no [`unhandledRejection`][] hook
is used.

Using this flag allows to change what should happen when an unhandled rejection
occurs. One of the following modes can be chosen:

* `warn-with-error-code`: Emit [`unhandledRejection`][]. If this hook is not
set, trigger a warning, and set the process exit code to 66. This is the
default.
* `throw`: Emit [`unhandledRejection`][]. If this hook is not set, raise the
unhandled rejection as an uncaught exception.
* `strict`: Raise the unhandled rejection as an uncaught exception.
* `warn`: Always trigger a warning, no matter if the [`unhandledRejection`][]
hook is set or not but do not print the deprecation warning.
* `warn-with-error-code`: Emit [`unhandledRejection`][]. If this hook is not
set, trigger a warning, and set the process exit code to 1.
* `none`: Silence all warnings.

### `--use-bundled-ca`, `--use-openssl-ca`
Expand Down
37 changes: 8 additions & 29 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,11 @@ const kThrowUnhandledRejections = 3;

// --unhandled-rejections=warn-with-error-code:
// Emit 'unhandledRejection', if it's unhandled, emit
// 'UnhandledPromiseRejectionWarning', then set process exit code to 1.
// 'UnhandledPromiseRejectionWarning', then set process exit code to 66.

const kWarnWithErrorCodeUnhandledRejections = 4;

// --unhandled-rejections is unset:
// Emit 'unhandledRejection', if it's unhandled, emit
// 'UnhandledPromiseRejectionWarning', then emit deprecation warning.
const kDefaultUnhandledRejections = 5;
const unhandledRejectionExitCode = 66;

let unhandledRejectionsMode;

Expand All @@ -86,7 +83,7 @@ function getUnhandledRejectionsMode() {
case 'warn-with-error-code':
return kWarnWithErrorCodeUnhandledRejections;
default:
return kDefaultUnhandledRejections;
return kWarnWithErrorCodeUnhandledRejections;
}
}

Expand Down Expand Up @@ -151,12 +148,14 @@ function handledRejection(promise) {
}

const unhandledRejectionErrName = 'UnhandledPromiseRejectionWarning';
function emitUnhandledRejectionWarning(uid, reason) {
function emitUnhandledRejectionWarning(uid, reason, failing) {
const warning = getErrorWithoutStack(
unhandledRejectionErrName,
'Unhandled promise rejection. This error originated either by ' +
'throwing inside of an async function without a catch block, ' +
'or by rejecting a promise which was not handled with .catch(). ' +
(failing ? 'This process will terminate with exit code ' +
unhandledRejectionExitCode + '. ' : '') +
'To terminate the node process on unhandled promise ' +
'rejection, use the CLI flag `--unhandled-rejections=strict` (see ' +
'https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). ' +
Expand All @@ -175,15 +174,6 @@ function emitUnhandledRejectionWarning(uid, reason) {
process.emitWarning(warning);
}

let deprecationWarned = false;
function emitDeprecationWarning() {
process.emitWarning(
'Unhandled promise rejections are deprecated. In the future, ' +
'promise rejections that are not handled will terminate the ' +
'Node.js process with a non-zero exit code.',
'DeprecationWarning', 'DEP0018');
}

// If this method returns true, we've executed user code or triggered
// a warning to be emitted which requires the microtask and next tick
// queues to be drained again.
Expand Down Expand Up @@ -236,19 +226,8 @@ function processPromiseRejections() {
case kWarnWithErrorCodeUnhandledRejections: {
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) {
emitUnhandledRejectionWarning(uid, reason);
process.exitCode = 1;
}
break;
}
case kDefaultUnhandledRejections: {
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) {
emitUnhandledRejectionWarning(uid, reason);
if (!deprecationWarned) {
emitDeprecationWarning();
deprecationWarned = true;
}
emitUnhandledRejectionWarning(uid, reason, true /* failing */);
process.exitCode = unhandledRejectionExitCode;
}
break;
}
Expand Down
8 changes: 4 additions & 4 deletions test/es-module/test-esm-cjs-load-error-note.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const expectedNote = 'To load an ES module, ' +
'or use the .mjs extension.';

const expectedCode = 1;
const expectedUnhandledRejectionCode = 66;

const pExport1 = spawn(process.execPath, [Export1]);
let pExport1Stderr = '';
Expand Down Expand Up @@ -63,8 +64,7 @@ pImport2.stderr.on('data', (data) => {
pImport2Stderr += data;
});
pImport2.on('close', mustCall((code) => {
// As this exits normally we expect 0
assert.strictEqual(code, 0);
assert.strictEqual(code, expectedUnhandledRejectionCode);
assert.ok(!pImport2Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport2Stderr}`);
}));
Expand Down Expand Up @@ -94,15 +94,15 @@ pImport4.on('close', mustCall((code) => {
`${expectedNote} not found in ${pImport4Stderr}`);
}));

// Must exit with zero and show note
// Must exit with 66 and show note
const pImport5 = spawn(process.execPath, [Import5]);
let pImport5Stderr = '';
pImport5.stderr.setEncoding('utf8');
pImport5.stderr.on('data', (data) => {
pImport5Stderr += data;
});
pImport5.on('close', mustCall((code) => {
assert.strictEqual(code, 0);
assert.strictEqual(code, expectedUnhandledRejectionCode);
assert.ok(!pImport5Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport5Stderr}`);
}));
2 changes: 1 addition & 1 deletion test/message/promise_unhandled_warn_with_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ const assert = require('assert');
common.disableCrashOnUnhandledRejection();

Promise.reject(new Error('alas'));
process.on('exit', assert.strictEqual.bind(null, 1));
process.on('exit', assert.strictEqual.bind(null, 66));
2 changes: 1 addition & 1 deletion test/message/promise_unhandled_warn_with_error.out
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
at *
at *
(Use `node --trace-warnings ...` to show where the warning was created)
*UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
*UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). This process will terminate with exit code 66. To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
4 changes: 0 additions & 4 deletions test/message/unhandled_promise_trace_warnings.out
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
at *
at *
at *
(node:*) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
at *
at *
at *
(node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
at handledRejection (internal/process/promises.js:*)
at promiseRejectHandler (internal/process/promises.js:*)
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-async-wrap-pop-id-during-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const ret = spawnSync(
['--stack_size=150', __filename, 'async'],
{ maxBuffer: Infinity }
);
assert.strictEqual(ret.status, 0,
assert.strictEqual(ret.status, 66,
`EXIT CODE: ${ret.status}, STDERR:\n${ret.stderr}`);
const stderr = ret.stderr.toString('utf8', 0, 2048);
assert.ok(!/async.*hook/i.test(stderr));
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-promises-unhandled-proxy-rejections.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
// Flags: --unhandled-rejections=warn
'use strict';
const common = require('../common');

common.disableCrashOnUnhandledRejection();

const expectedDeprecationWarning = ['Unhandled promise rejections are ' +
'deprecated. In the future, promise ' +
'rejections that are not handled will ' +
'terminate the Node.js process with a ' +
'non-zero exit code.', 'DEP0018'];
const expectedPromiseWarning = ['Unhandled promise rejection. ' +
'This error originated either by throwing ' +
'inside of an async function without a catch ' +
Expand Down Expand Up @@ -39,7 +35,6 @@ const thorny = new Proxy({}, {
});

common.expectWarning({
DeprecationWarning: expectedDeprecationWarning,
UnhandledPromiseRejectionWarning: expectedPromiseWarning,
});

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-promises-unhandled-rejections.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --unhandled-rejections=warn
'use strict';
const common = require('../common');
const assert = require('assert');
Expand Down Expand Up @@ -714,7 +715,7 @@ asyncTest(
done();
}
emitWarning(...args);
}, 2);
}, 4);

let timer = setTimeout(common.mustNotCall(), 10000);
},
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-promises-unhandled-symbol-rejections.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// Flags: --unhandled-rejections=warn
'use strict';
const common = require('../common');

common.disableCrashOnUnhandledRejection();

const expectedValueWarning = ['Symbol()'];
const expectedDeprecationWarning = ['Unhandled promise rejections are ' +
'deprecated. In the future, promise ' +
'rejections that are not handled will ' +
'terminate the Node.js process with a ' +
'non-zero exit code.', 'DEP0018'];
const expectedPromiseWarning = ['Unhandled promise rejection. ' +
'This error originated either by throwing ' +
'inside of an async function without a catch ' +
Expand All @@ -20,7 +16,6 @@ const expectedPromiseWarning = ['Unhandled promise rejection. ' +
'(rejection id: 1)'];

common.expectWarning({
DeprecationWarning: expectedDeprecationWarning,
UnhandledPromiseRejectionWarning: [
expectedValueWarning,
expectedPromiseWarning
Expand Down
12 changes: 4 additions & 8 deletions test/parallel/test-promises-warning-on-unhandled-rejection.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --no-warnings
// Flags: --no-warnings --unhandled-rejections=warn
'use strict';

// Test that warnings are emitted when a Promise experiences an uncaught
Expand Down Expand Up @@ -27,14 +27,10 @@ process.on('warning', common.mustCall((warning) => {
);
break;
case 2:
// One time deprecation warning, first unhandled rejection
assert.strictEqual(warning.name, 'DeprecationWarning');
break;
case 3:
// Number rejection error displayed. Note it's been stringified
assert.strictEqual(warning.message, '42');
break;
case 4:
case 3:
// Unhandled rejection warning (won't be handled next tick)
assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning');
assert(
Expand All @@ -43,13 +39,13 @@ process.on('warning', common.mustCall((warning) => {
`but did not. Had "${warning.message}" instead.`
);
break;
case 5:
case 4:
// Rejection handled asynchronously.
assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning');
assert(/Promise rejection was handled asynchronously/
.test(warning.message));
}
}, 6));
}, 5));

const p = Promise.reject('This was rejected'); // Reject with a string
setImmediate(common.mustCall(() => p.catch(() => { })));
Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-inspector-async-call-stack-abort.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if (process.argv[2] === 'child') {
const options = { encoding: 'utf8' };
const proc = spawnSync(
process.execPath, ['--expose-internals', __filename, 'child'], options);
strictEqual(proc.status, 0);
strictEqual(proc.status, 66);
strictEqual(proc.signal, null);
strictEqual(proc.stderr.includes(eyecatcher), true);
}