Skip to content

Commit

Permalink
process: Change default --unhandled-rejections=warn-with-error-code
Browse files Browse the repository at this point in the history
This is a semver-major change that resolves DEP0018.

In addition to changing the default value for --unhandled-rejections,
this PR changes the exit code from 1 to 66. The warning message has
been updated to mention the new exit code, which I selected in order
to make this problem easier to search for on the web.

Users can prevent this behavior by setting a non-default value for the
--unhandled-rejections flag (throw, strict, warn, or none) or by setting
a hook for 'unhandledRejection'. This change has no effect on users who
have already set the flag or installed an 'unhandledRejection' hook; it
will only impact users who currently see a DEP0018 warning today.
  • Loading branch information
dfabulich committed Jun 16, 2020
1 parent 14d012e commit e46e34b
Show file tree
Hide file tree
Showing 12 changed files with 27 additions and 68 deletions.
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);
}

0 comments on commit e46e34b

Please sign in to comment.