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=throw #33021

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 1 addition & 5 deletions doc/api/cli.md
Expand Up @@ -1007,15 +1007,11 @@ 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:

* `throw`: Emit [`unhandledRejection`][]. If this hook is not set, raise the
unhandled rejection as an uncaught exception.
unhandled rejection as an uncaught exception. This is the default.
* `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.
Expand Down
27 changes: 1 addition & 26 deletions lib/internal/process/promises.js
Expand Up @@ -57,11 +57,6 @@ const kThrowUnhandledRejections = 3;

const kWarnWithErrorCodeUnhandledRejections = 4;

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

let unhandledRejectionsMode;

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

Expand Down Expand Up @@ -175,15 +170,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 @@ -241,17 +227,6 @@ function processPromiseRejections() {
}
break;
}
case kDefaultUnhandledRejections: {
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) {
emitUnhandledRejectionWarning(uid, reason);
if (!deprecationWarned) {
emitDeprecationWarning();
deprecationWarned = true;
}
}
break;
}
}
maybeScheduledTicksOrMicrotasks = true;
}
Expand Down
5 changes: 1 addition & 4 deletions test/common/index.js
Expand Up @@ -599,11 +599,8 @@ function getBufferSources(buf) {
return [...getArrayBufferViews(buf), new Uint8Array(buf).buffer];
}

// Crash the process on unhandled rejections.
const crashOnUnhandledRejection = (err) => { throw err; };
process.on('unhandledRejection', crashOnUnhandledRejection);
function disableCrashOnUnhandledRejection() {
process.removeListener('unhandledRejection', crashOnUnhandledRejection);
process.on('unhandledRejection', () => {});
}

function getTTYfd() {
Expand Down
7 changes: 3 additions & 4 deletions test/es-module/test-esm-cjs-load-error-note.mjs
Expand Up @@ -63,8 +63,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, expectedCode);
assert.ok(!pImport2Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport2Stderr}`);
}));
Expand Down Expand Up @@ -94,15 +93,15 @@ pImport4.on('close', mustCall((code) => {
`${expectedNote} not found in ${pImport4Stderr}`);
}));

// Must exit with zero and show note
// Must exit non-zero 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, expectedCode);
assert.ok(!pImport5Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport5Stderr}`);
}));
4 changes: 1 addition & 3 deletions test/message/promise_unhandled_warn_with_error.js
@@ -1,10 +1,8 @@
// Flags: --unhandled-rejections=warn-with-error-code
'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');

common.disableCrashOnUnhandledRejection();

Promise.reject(new Error('alas'));
process.on('exit', assert.strictEqual.bind(null, 1));
5 changes: 2 additions & 3 deletions test/message/unhandled_promise_trace_warnings.js
@@ -1,6 +1,5 @@
// Flags: --trace-warnings
// Flags: --trace-warnings --unhandled-rejections=warn
'use strict';
const common = require('../common');
common.disableCrashOnUnhandledRejection();
require('../common');
const p = Promise.reject(new Error('This was rejected'));
setImmediate(() => p.catch(() => {}));
4 changes: 0 additions & 4 deletions test/message/unhandled_promise_trace_warnings.out
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
52 changes: 52 additions & 0 deletions test/parallel/test-promise-unhandled-default.js
@@ -0,0 +1,52 @@
'use strict';

const common = require('../common');
const Countdown = require('../common/countdown');
const assert = require('assert');

// Verify that unhandled rejections always trigger uncaught exceptions instead
// of triggering unhandled rejections.

const err1 = new Error('One');
const err2 = new Error(
'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(). The promise rejected with the' +
' reason "null".'
);
err2.code = 'ERR_UNHANDLED_REJECTION';
Object.defineProperty(err2, 'name', {
value: 'UnhandledPromiseRejection',
writable: true,
configurable: true
});

const errors = [err1, err2];
const identical = [true, false];

const ref = new Promise(() => {
throw err1;
});
// Explicitly reject `null`.
Promise.reject(null);

process.on('warning', common.mustNotCall('warning'));
// If we add an unhandledRejection handler, the exception won't be thrown
// process.on('unhandledRejection', common.mustCall(2));
process.on('rejectionHandled', common.mustNotCall('rejectionHandled'));
process.on('exit', assert.strictEqual.bind(null, 0));

const timer = setTimeout(() => console.log(ref), 1000);

const counter = new Countdown(2, () => {
clearTimeout(timer);
});

process.on('uncaughtException', common.mustCall((err, origin) => {
counter.dec();
assert.strictEqual(origin, 'unhandledRejection', err);
const knownError = errors.shift();
assert.deepStrictEqual(err, knownError);
// Check if the errors are reference equal.
assert(identical.shift() ? err === knownError : err !== knownError);
}, 2));
2 changes: 0 additions & 2 deletions test/parallel/test-promise-unhandled-throw.js
Expand Up @@ -5,8 +5,6 @@ const common = require('../common');
const Countdown = require('../common/countdown');
const assert = require('assert');

common.disableCrashOnUnhandledRejection();

// Verify that unhandled rejections always trigger uncaught exceptions instead
// of triggering unhandled rejections.

Expand Down
5 changes: 1 addition & 4 deletions test/parallel/test-promise-unhandled-warn-no-hook.js
Expand Up @@ -3,10 +3,7 @@

const common = require('../common');

common.disableCrashOnUnhandledRejection();

// Verify that ignoring unhandled rejection works fine and that no warning is
// logged.
// Verify that --unhandled-rejections=warn works fine

new Promise(() => {
throw new Error('One');
Expand Down
20 changes: 1 addition & 19 deletions test/parallel/test-promises-unhandled-proxy-rejections.js
Expand Up @@ -3,21 +3,6 @@ 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 ' +
'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)'];

function throwErr() {
throw new Error('Error from proxy');
}
Expand All @@ -38,10 +23,7 @@ const thorny = new Proxy({}, {
construct: throwErr
});

common.expectWarning({
DeprecationWarning: expectedDeprecationWarning,
UnhandledPromiseRejectionWarning: expectedPromiseWarning,
});
process.on('warning', common.mustNotCall());

// Ensure this doesn't crash
Promise.reject(thorny);
9 changes: 4 additions & 5 deletions test/parallel/test-promises-unhandled-rejections.js
Expand Up @@ -668,6 +668,7 @@ asyncTest('Throwing an error inside a rejectionHandled handler goes to' +
' unhandledException, and does not cause .catch() to throw an ' +
'exception', function(done) {
clean();
common.disableCrashOnUnhandledRejection();
const e = new Error();
const e2 = new Error();
const tearDownException = setupException(function(err) {
Expand Down Expand Up @@ -702,19 +703,17 @@ asyncTest('Rejected promise inside unhandledRejection allows nextTick loop' +
});

asyncTest(
'Unhandled promise rejection emits a warning immediately',
'Promise rejection triggers unhandledRejection immediately',
function(done) {
clean();
Promise.reject(0);
const { emitWarning } = process;
process.emitWarning = common.mustCall((...args) => {
process.on('unhandledRejection', common.mustCall((err) => {
if (timer) {
clearTimeout(timer);
timer = null;
done();
}
emitWarning(...args);
}, 2);
}));

let timer = setTimeout(common.mustNotCall(), 10000);
},
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-promises-unhandled-symbol-rejections.js
@@ -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
14 changes: 4 additions & 10 deletions test/parallel/test-promises-warning-on-unhandled-rejection.js
@@ -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 All @@ -7,8 +7,6 @@
const common = require('../common');
const assert = require('assert');

common.disableCrashOnUnhandledRejection();

let b = 0;

process.on('warning', common.mustCall((warning) => {
Expand All @@ -27,14 +25,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 +37,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
3 changes: 1 addition & 2 deletions test/sequential/test-inspector-async-call-stack-abort.js
Expand Up @@ -9,7 +9,6 @@ const { strictEqual } = require('assert');
const eyecatcher = 'nou, houdoe he?';

if (process.argv[2] === 'child') {
common.disableCrashOnUnhandledRejection();
const { Session } = require('inspector');
const { promisify } = require('util');
const { internalBinding } = require('internal/test/binding');
Expand All @@ -31,7 +30,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, 1);
ChALkeR marked this conversation as resolved.
Show resolved Hide resolved
strictEqual(proc.signal, null);
strictEqual(proc.stderr.includes(eyecatcher), true);
}