From 77ab281ee5f79c3de57b4fe368b3b2c7d2199f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Mon, 12 Apr 2021 16:04:44 +0200 Subject: [PATCH] test: remove common.disableCrashOnUnhandledRejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the --unhandled-rejections=none CLI flag instead. PR-URL: https://github.com/nodejs/node/pull/38210 Reviewed-By: Richard Lau Reviewed-By: Juan José Arboleda Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: James M Snell --- doc/guides/writing-tests.md | 8 +++----- test/common/README.md | 8 -------- test/common/index.js | 5 ----- test/common/index.mjs | 2 -- test/common/inspector-helper.js | 1 - test/parallel/test-async-wrap-pop-id-during-load.js | 5 ++--- test/parallel/test-no-harmony-top-level-await.mjs | 3 --- .../parallel/test-promise-handled-rejection-no-warning.js | 1 - test/parallel/test-promise-unhandled-error.js | 2 -- test/parallel/test-promise-unhandled-silent-no-hook.js | 2 -- test/parallel/test-promise-unhandled-silent.js | 2 -- test/parallel/test-promise-unhandled-throw-handler.js | 2 -- test/parallel/test-promise-unhandled-warn.js | 2 -- test/parallel/test-promises-unhandled-proxy-rejections.js | 3 +-- test/parallel/test-promises-unhandled-rejections.js | 4 +--- .../parallel/test-promises-unhandled-symbol-rejections.js | 2 -- test/parallel/test-trace-events-promises.js | 2 -- 17 files changed, 7 insertions(+), 47 deletions(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index 8424f744d16927..7d33ee15858cc5 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -250,11 +250,9 @@ countdown.dec(); // The countdown callback will be invoked now. When writing tests involving promises, it is generally good to wrap the `onFulfilled` handler, otherwise the test could successfully finish if the -promise never resolves (pending promises do not keep the event loop alive). The -`common` module automatically adds a handler that makes the process crash - and -hence, the test fail - in the case of an `unhandledRejection` event. It is -possible to disable it with `common.disableCrashOnUnhandledRejection()` if -needed. +promise never resolves (pending promises do not keep the event loop alive). +Node.js automatically crashes - and hence, the test fails - in the case of an +`unhandledRejection` event. ```js const common = require('../common'); diff --git a/test/common/README.md b/test/common/README.md index 31bf0b972a8516..2f2ee17a0f01e4 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -61,14 +61,6 @@ On non-Windows platforms, this always returns `true`. Creates a 10 MB file of all null characters. -### `disableCrashOnUnhandledRejection()` - -Removes the `process.on('unhandledRejection')` handler that crashes the process -after a tick. The handler is useful for tests that use Promises and need to make -sure no unexpected rejections occur, because currently they result in silent -failures. However, it is useful in some rare cases to disable it, for example if -the `unhandledRejection` hook is directly used by the test. - ### `enoughTestCpu` * [<boolean>][] diff --git a/test/common/index.js b/test/common/index.js index 13175195c4405c..f0d8c72d4a1017 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -628,10 +628,6 @@ function getBufferSources(buf) { return [...getArrayBufferViews(buf), new Uint8Array(buf).buffer]; } -function disableCrashOnUnhandledRejection() { - process.on('unhandledRejection', () => {}); -} - function getTTYfd() { // Do our best to grab a tty fd. const tty = require('tty'); @@ -732,7 +728,6 @@ const common = { canCreateSymLink, childShouldThrowAndAbort, createZeroFilledFile, - disableCrashOnUnhandledRejection, expectsError, expectWarning, gcUntil, diff --git a/test/common/index.mjs b/test/common/index.mjs index c0bbb9d74e1cda..73049004759869 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -46,7 +46,6 @@ const { skipIf32Bits, getArrayBufferViews, getBufferSources, - disableCrashOnUnhandledRejection, getTTYfd, runWithInvalidFD } = common; @@ -92,7 +91,6 @@ export { skipIf32Bits, getArrayBufferViews, getBufferSources, - disableCrashOnUnhandledRejection, getTTYfd, runWithInvalidFD, createRequire diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index 3ab4975ce09c30..9c98ce6252c0e3 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -25,7 +25,6 @@ function spawnChildProcess(inspectorFlags, scriptContents, scriptFile) { const handler = tearDown.bind(null, child); process.on('exit', handler); process.on('uncaughtException', handler); - common.disableCrashOnUnhandledRejection(); process.on('unhandledRejection', handler); process.on('SIGINT', handler); diff --git a/test/parallel/test-async-wrap-pop-id-during-load.js b/test/parallel/test-async-wrap-pop-id-during-load.js index bf75451817164b..14d1ed8b890e5c 100644 --- a/test/parallel/test-async-wrap-pop-id-during-load.js +++ b/test/parallel/test-async-wrap-pop-id-during-load.js @@ -1,9 +1,8 @@ 'use strict'; -const common = require('../common'); +require('../common'); if (process.argv[2] === 'async') { - common.disableCrashOnUnhandledRejection(); async function fn() { fn(); throw new Error(); @@ -16,7 +15,7 @@ const { spawnSync } = require('child_process'); const ret = spawnSync( process.execPath, - ['--stack_size=150', __filename, 'async'], + ['--unhandled-rejections=none', '--stack_size=150', __filename, 'async'], { maxBuffer: Infinity } ); assert.strictEqual(ret.status, 0, diff --git a/test/parallel/test-no-harmony-top-level-await.mjs b/test/parallel/test-no-harmony-top-level-await.mjs index d48fbbe7f45246..5805af0e5ee265 100644 --- a/test/parallel/test-no-harmony-top-level-await.mjs +++ b/test/parallel/test-no-harmony-top-level-await.mjs @@ -2,10 +2,7 @@ import { mustCall, - disableCrashOnUnhandledRejection } from '../common/index.mjs'; -disableCrashOnUnhandledRejection(); - process.on('unhandledRejection', mustCall()); Promise.reject(new Error('should not be fatal error')); diff --git a/test/parallel/test-promise-handled-rejection-no-warning.js b/test/parallel/test-promise-handled-rejection-no-warning.js index 14b06affb73825..8878d67faba6b0 100644 --- a/test/parallel/test-promise-handled-rejection-no-warning.js +++ b/test/parallel/test-promise-handled-rejection-no-warning.js @@ -2,7 +2,6 @@ const common = require('../common'); // This test verifies that DEP0018 does not occur when rejections are handled. -common.disableCrashOnUnhandledRejection(); process.on('warning', common.mustNotCall()); process.on('unhandledRejection', common.mustCall()); Promise.reject(new Error()); diff --git a/test/parallel/test-promise-unhandled-error.js b/test/parallel/test-promise-unhandled-error.js index bb906fdcbe7591..67607dd5ebfa2b 100644 --- a/test/parallel/test-promise-unhandled-error.js +++ b/test/parallel/test-promise-unhandled-error.js @@ -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. diff --git a/test/parallel/test-promise-unhandled-silent-no-hook.js b/test/parallel/test-promise-unhandled-silent-no-hook.js index db4f7a8471df7a..2fb088ed6ff58d 100644 --- a/test/parallel/test-promise-unhandled-silent-no-hook.js +++ b/test/parallel/test-promise-unhandled-silent-no-hook.js @@ -4,8 +4,6 @@ const common = require('../common'); const assert = require('assert'); -common.disableCrashOnUnhandledRejection(); - // Verify that ignoring unhandled rejection works fine and that no warning is // logged even though there is no unhandledRejection hook attached. diff --git a/test/parallel/test-promise-unhandled-silent.js b/test/parallel/test-promise-unhandled-silent.js index c9ccc0118e064c..edf5111eaec0ba 100644 --- a/test/parallel/test-promise-unhandled-silent.js +++ b/test/parallel/test-promise-unhandled-silent.js @@ -3,8 +3,6 @@ const common = require('../common'); -common.disableCrashOnUnhandledRejection(); - // Verify that ignoring unhandled rejection works fine and that no warning is // logged. diff --git a/test/parallel/test-promise-unhandled-throw-handler.js b/test/parallel/test-promise-unhandled-throw-handler.js index be441c2d34c6bd..26a1d2f85c1d02 100644 --- a/test/parallel/test-promise-unhandled-throw-handler.js +++ b/test/parallel/test-promise-unhandled-throw-handler.js @@ -5,8 +5,6 @@ const common = require('../common'); const Countdown = require('../common/countdown'); const assert = require('assert'); -common.disableCrashOnUnhandledRejection(); - // Verify that the unhandledRejection handler prevents triggering // uncaught exceptions diff --git a/test/parallel/test-promise-unhandled-warn.js b/test/parallel/test-promise-unhandled-warn.js index 62ddc69d8cd34b..aab07973be40aa 100644 --- a/test/parallel/test-promise-unhandled-warn.js +++ b/test/parallel/test-promise-unhandled-warn.js @@ -3,8 +3,6 @@ const common = require('../common'); -common.disableCrashOnUnhandledRejection(); - // Verify that ignoring unhandled rejection works fine and that no warning is // logged. diff --git a/test/parallel/test-promises-unhandled-proxy-rejections.js b/test/parallel/test-promises-unhandled-proxy-rejections.js index 099df95691d466..77f2bb653b3baa 100644 --- a/test/parallel/test-promises-unhandled-proxy-rejections.js +++ b/test/parallel/test-promises-unhandled-proxy-rejections.js @@ -1,8 +1,7 @@ +// Flags: --unhandled-rejections=none 'use strict'; const common = require('../common'); -common.disableCrashOnUnhandledRejection(); - function throwErr() { throw new Error('Error from proxy'); } diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index fc2ad945bae36b..761923c5cc30c6 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -1,10 +1,9 @@ +// Flags: --unhandled-rejections=none 'use strict'; const common = require('../common'); const assert = require('assert'); const { inspect } = require('util'); -common.disableCrashOnUnhandledRejection(); - const asyncTest = (function() { let asyncTestsEnabled = false; let asyncTestLastCheck; @@ -643,7 +642,6 @@ 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) { diff --git a/test/parallel/test-promises-unhandled-symbol-rejections.js b/test/parallel/test-promises-unhandled-symbol-rejections.js index 00da8885f13d9a..6ce6808abd2c47 100644 --- a/test/parallel/test-promises-unhandled-symbol-rejections.js +++ b/test/parallel/test-promises-unhandled-symbol-rejections.js @@ -2,8 +2,6 @@ 'use strict'; const common = require('../common'); -common.disableCrashOnUnhandledRejection(); - const expectedValueWarning = ['Symbol()']; const expectedPromiseWarning = ['Unhandled promise rejection. ' + 'This error originated either by throwing ' + diff --git a/test/parallel/test-trace-events-promises.js b/test/parallel/test-trace-events-promises.js index eec463369c119e..37e4866c6b8afc 100644 --- a/test/parallel/test-trace-events-promises.js +++ b/test/parallel/test-trace-events-promises.js @@ -6,8 +6,6 @@ const fs = require('fs'); const path = require('path'); const tmpdir = require('../common/tmpdir'); -common.disableCrashOnUnhandledRejection(); - if (process.argv[2] === 'child') { const p = Promise.reject(1); // Handled later Promise.reject(2); // Unhandled