nodejs · targos · Jul 17, 2018 · Jul 19, 2018 · richardlau · Jul 23, 2018
diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md
@@ -225,34 +225,24 @@ countdown.dec(); // The countdown callback will be invoked now.
 
 #### Testing promises
 
-When writing tests involving promises, either make sure that the
-`onFulfilled` or the `onRejected` handler is wrapped in
-`common.mustCall()` or `common.mustNotCall()` accordingly, or
-call `common.crashOnUnhandledRejection()` in the top level of the
-test to make sure that unhandled rejections would result in a test
-failure. For example:
+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.
 
 ```javascript
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs').promises;
 
-// Use `common.crashOnUnhandledRejection()` to make sure unhandled rejections
-// will fail the test.
-common.crashOnUnhandledRejection();
-
-// Or, wrap the `onRejected` handler in `common.mustNotCall()`.
-fs.writeFile('test-file', 'test').catch(common.mustNotCall());
-
-// Or, wrap the `onFulfilled` handler in `common.mustCall()`.
-// If there are assertions in the `onFulfilled` handler, wrap
-// the next `onRejected` handler in `common.mustNotCall()`
-// to handle potential failures.
+// Wrap the `onFulfilled` handler in `common.mustCall()`.
 fs.readFile('test-file').then(
  common.mustCall(
  (content) => assert.strictEqual(content.toString(), 'test2')
- ))
- .catch(common.mustNotCall());
+ ));
 ```
 
 ### Flags

diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js
@@ -7,8 +7,6 @@ const common = require('../../common');
 constassert=require('assert');
 consttest_promise=require(`./build/${common.buildType}/test_promise`);
 
-common.crashOnUnhandledRejection();
-
 // A resolution
 {
 constexpected_result=42;

diff --git a/test/addons-napi/test_threadsafe_function/test.js b/test/addons-napi/test_threadsafe_function/test.js
@@ -12,8 +12,6 @@ const expectedArray = (function(arrayLength) {
 returnresult;
 })(binding.ARRAY_LENGTH);
 
-common.crashOnUnhandledRejection();
-
 // Handle the rapid teardown test case as the child process. We unref the
 // thread-safe function after we have received two values. This causes the
 // process to exit and the environment cleanup handler to be invoked.

diff --git a/test/addons/callback-scope/test-resolve-async.js b/test/addons/callback-scope/test-resolve-async.js
@@ -4,8 +4,6 @@ const common = require('../../common');
 const assert = require('assert');
 const { testResolveAsync } = require(`./build/${common.buildType}/binding`);
 
-common.crashOnUnhandledRejection();
-
 let called = false;
 testResolveAsync().then(() => { called = true; });
 

diff --git a/test/addons/make-callback-recurse/test.js b/test/addons/make-callback-recurse/test.js
@@ -9,8 +9,6 @@ const makeCallback = binding.makeCallback;
 // Make sure this is run in the future.
 const mustCallCheckDomains = common.mustCall(checkDomains);
 
-common.crashOnUnhandledRejection();
-
 // Make sure that using MakeCallback allows the error to propagate.
 assert.throws(function() {
  makeCallback({}, function() {

diff --git a/test/async-hooks/test-promise.chain-promise-before-init-hooks.js b/test/async-hooks/test-promise.chain-promise-before-init-hooks.js
@@ -8,8 +8,6 @@ const { checkInvocations } = require('./hook-checks');
 if (!common.isMainThread)
  common.skip('Worker bootstrapping works differently -> different async IDs');
 
-common.crashOnUnhandledRejection();
-
 const p = new Promise(common.mustCall(function executor(resolve, reject) {
  resolve(5);
 }));

diff --git a/test/async-hooks/test-promise.js b/test/async-hooks/test-promise.js
@@ -9,8 +9,6 @@ const { checkInvocations } = require('./hook-checks');
 if (!common.isMainThread)
  common.skip('Worker bootstrapping works differently -> different async IDs');
 
-common.crashOnUnhandledRejection();
-
 const hooks = initHooks();
 
 hooks.enable();

diff --git a/test/common/README.md b/test/common/README.md
@@ -55,18 +55,19 @@ symlinks
 ([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716(v=vs.85).aspx)).
 On non-Windows platforms, this always returns `true`.
 
-### crashOnUnhandledRejection()
-
-Installs a `process.on('unhandledRejection')` handler that crashes the process
-after a tick. This is useful for tests that use Promises and need to make sure
-no unexpected rejections occur, because currently they result in silent
-failures.
-
 ### ddCommand(filename, kilobytes)
 * return [&lt;Object>]
 
 Platform normalizes the `dd` command
 
+### 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.
+
 ### enoughTestMem
 * [&lt;boolean>]
 

diff --git a/test/common/index.js b/test/common/index.js
@@ -814,9 +814,10 @@ exports.getBufferSources = function getBufferSources(buf) {
 };
 
 // Crash the process on unhandled rejections.
-exports.crashOnUnhandledRejection = function() {
- process.on('unhandledRejection',
- (err) => process.nextTick(() => { throw err; }));
+const crashOnUnhandledRejection = (err) => { throw err; };
+process.on('unhandledRejection', crashOnUnhandledRejection);
+exports.disableCrashOnUnhandledRejection = function() {
+ process.removeListener('unhandledRejection', crashOnUnhandledRejection);
 };
 
 exports.getTTYfd = function getTTYfd() {

diff --git a/test/common/index.mjs b/test/common/index.mjs
@@ -52,7 +52,7 @@ const {
  skipIf32Bits,
  getArrayBufferViews,
  getBufferSources,
-crashOnUnhandledRejection,
+disableCrashOnUnhandledRejection,
  getTTYfd,
  runWithInvalidFD,
  hijackStdout,
@@ -112,7 +112,7 @@ export {
 skipIf32Bits,
 getArrayBufferViews,
 getBufferSources,
-crashOnUnhandledRejection,
+disableCrashOnUnhandledRejection,
 getTTYfd,
 runWithInvalidFD,
 hijackStdout,

diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js
@@ -25,6 +25,7 @@ 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/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js
@@ -5,8 +5,6 @@ const assert = require('assert');
 const { URL } = require('url');
 const vm = require('vm');
 
-common.crashOnUnhandledRejection();
-
 const relativePath = '../fixtures/es-modules/test-esm-ok.mjs';
 const absolutePath = require.resolve('../fixtures/es-modules/test-esm-ok.mjs');
 const targetURL = new URL('file:///');

diff --git a/test/es-module/test-esm-error-cache.js b/test/es-module/test-esm-error-cache.js
@@ -2,11 +2,9 @@
 
 // Flags: --experimental-modules
 
-constcommon=require('../common');
+require('../common');
 constassert=require('assert');
 
-common.crashOnUnhandledRejection();
-
 constfile='../fixtures/syntax/bad_syntax.js';
 
 leterror;

diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs
@@ -1,11 +1,6 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs
 
-import{
-crashOnUnhandledRejection,
-expectsError
-}from'../common';
-
-crashOnUnhandledRejection();
+import{expectsError}from'../common';
 
 import('test').catch(expectsError({
 code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK',

diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs
@@ -1,6 +1,5 @@
 // Flags: --experimental-modules
-/* eslint-disable node-core/required-modules */
-importcommonfrom'../common/index.js';
+import'../common';
 importassertfrom'assert';
 
 asyncfunctiondoTest(){
@@ -12,5 +11,4 @@ async function doTest() {
 );
 }
 
-common.crashOnUnhandledRejection();
 doTest();
diff --git a/test/internet/test-dns-any.js b/test/internet/test-dns-any.js
@@ -9,8 +9,6 @@ const net = require('net');
 let running = false;
 const queue = [];
 
-common.crashOnUnhandledRejection();
-
 const dnsPromises = dns.promises;
 const isIPv4 = net.isIPv4;
 const isIPv6 = net.isIPv6;

diff --git a/test/internet/test-dns-ipv4.js b/test/internet/test-dns-ipv4.js
@@ -7,8 +7,6 @@ const net = require('net');
 constutil=require('util');
 constisIPv4=net.isIPv4;
 
-common.crashOnUnhandledRejection();
-
 constdnsPromises=dns.promises;
 letrunning=false;
 constqueue=[];

diff --git a/test/internet/test-dns-ipv6.js b/test/internet/test-dns-ipv6.js
@@ -4,8 +4,6 @@ const { addresses } = require('../common/internet');
 if(!common.hasIPv6)
 common.skip('this test, no IPv6 support');
 
-common.crashOnUnhandledRejection();
-
 constassert=require('assert');
 constdns=require('dns');
 constnet=require('net');

diff --git a/test/internet/test-dns-txt-sigsegv.js b/test/internet/test-dns-txt-sigsegv.js
@@ -1,11 +1,9 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const dns = require('dns');
 const dnsPromises = dns.promises;
 
-common.crashOnUnhandledRejection();
-
 (async function() {
  const result = await dnsPromises.resolveTxt('www.microsoft.com');
  assert.strictEqual(result.length, 0);

diff --git a/test/internet/test-dns.js b/test/internet/test-dns.js
@@ -30,8 +30,6 @@ const isIPv6 = net.isIPv6;
 constutil=require('util');
 constdnsPromises=dns.promises;
 
-common.crashOnUnhandledRejection();
-
 letexpected=0;
 letcompleted=0;
 letrunning=false;

diff --git a/test/known_issues/test-inspector-cluster-port-clash.js b/test/known_issues/test-inspector-cluster-port-clash.js
@@ -23,8 +23,6 @@ if (process.config.variables.v8_enable_inspector === 0) {
 const cluster = require('cluster');
 const net = require('net');
 
-common.crashOnUnhandledRejection();
-
 const ports = [process.debugPort];
 const clashPort = process.debugPort + 2;
 function serialFork() {

diff --git a/test/message/unhandled_promise_trace_warnings.js b/test/message/unhandled_promise_trace_warnings.js
@@ -1,5 +1,6 @@
 // Flags: --trace-warnings
 'use strict';
-require('../common');
+const common = require('../common');
+common.disableCrashOnUnhandledRejection();
 const p = Promise.reject(new Error('This was rejected'));
 setImmediate(() => p.catch(() => {}));
diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
@@ -5,8 +5,6 @@ const assert = require('assert');
 // Test assert.rejects() and assert.doesNotReject() by checking their
 // expected output and by verifying that they do not work sync
 
-common.crashOnUnhandledRejection();
-
 // Run all tests in parallel and check their outcome at the end.
 constpromises=[];
 

diff --git a/test/parallel/test-async-hooks-disable-during-promise.js b/test/parallel/test-async-hooks-disable-during-promise.js
@@ -1,7 +1,6 @@
 'use strict';
 constcommon=require('../common');
 constasync_hooks=require('async_hooks');
-common.crashOnUnhandledRejection();
 
 if(!common.isMainThread)
 common.skip('Worker bootstrapping works differently -> different AsyncWraps');

diff --git a/test/parallel/test-async-hooks-enable-during-promise.js b/test/parallel/test-async-hooks-enable-during-promise.js
@@ -2,8 +2,6 @@
 const common = require('../common');
 const async_hooks = require('async_hooks');
 
-common.crashOnUnhandledRejection();
-
 Promise.resolve(1).then(common.mustCall(() => {
  async_hooks.createHook({
  init: common.mustCall(),

diff --git a/test/parallel/test-async-hooks-promise-enable-disable.js b/test/parallel/test-async-hooks-promise-enable-disable.js
@@ -8,8 +8,6 @@ let p_resource = null;
 letp_er=null;
 letp_inits=0;
 
-common.crashOnUnhandledRejection();
-
 // Not useful to place common.mustCall() around 'exit' event b/c it won't be
 // able to check it anyway.
 process.on('exit',(code)=>{

diff --git a/test/parallel/test-async-hooks-promise-triggerid.js b/test/parallel/test-async-hooks-promise-triggerid.js
@@ -6,8 +6,6 @@ const async_hooks = require('async_hooks');
 if(!common.isMainThread)
 common.skip('Worker bootstrapping works differently -> different async IDs');
 
-common.crashOnUnhandledRejection();
-
 constpromiseAsyncIds=[];
 
 async_hooks.createHook({

diff --git a/test/parallel/test-async-wrap-pop-id-during-load.js b/test/parallel/test-async-wrap-pop-id-during-load.js
@@ -1,8 +1,9 @@
 'use strict';
 
-require('../common');
+constcommon=require('../common');
 
 if(process.argv[2]==='async'){
+common.disableCrashOnUnhandledRejection();
 asyncfunctionfn(){
 fn();
 thrownewError();

diff --git a/test/parallel/test-async-wrap-promise-after-enabled.js b/test/parallel/test-async-wrap-promise-after-enabled.js
@@ -12,8 +12,6 @@ const async_hooks = require('async_hooks');
 
 const seenEvents = [];
 
-common.crashOnUnhandledRejection();
-
 const p = new Promise((resolve) => resolve(1));
 p.then(() => seenEvents.push('then'));
 

diff --git a/test/parallel/test-c-ares.js b/test/parallel/test-c-ares.js
@@ -23,8 +23,6 @@
 const common = require('../common');
 const assert = require('assert');
 
-common.crashOnUnhandledRejection();
-
 const dns = require('dns');
 const dnsPromises = dns.promises;
 

diff --git a/test/parallel/test-child-process-promisified.js b/test/parallel/test-child-process-promisified.js
@@ -4,8 +4,6 @@ const assert = require('assert');
 constchild_process=require('child_process');
 const{ promisify }=require('util');
 
-common.crashOnUnhandledRejection();
-
 constexec=promisify(child_process.exec);
 constexecFile=promisify(child_process.execFile);
 

diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js
@@ -5,8 +5,6 @@ const cares = process.binding('cares_wrap');
 const dns = require('dns');
 const dnsPromises = dns.promises;
 
-common.crashOnUnhandledRejection();
-
 // Stub `getaddrinfo` to *always* error.
 cares.getaddrinfo = () => process.binding('uv').UV_ENOENT;
 

diff --git a/test/parallel/test-dns-promises-resolve.js b/test/parallel/test-dns-promises-resolve.js
@@ -4,8 +4,6 @@ const assert = require('assert');
 
 constdnsPromises=require('dns').promises;
 
-common.crashOnUnhandledRejection();
-
 // Error when rrtype is invalid.
 {
 constrrtype='DUMMY';

diff --git a/test/parallel/test-dns-resolveany-bad-ancount.js b/test/parallel/test-dns-resolveany-bad-ancount.js
@@ -6,8 +6,6 @@ const assert = require('assert');
 const dgram = require('dgram');
 const dnsPromises = dns.promises;
 
-common.crashOnUnhandledRejection();
-
 const server = dgram.createSocket('udp4');
 
 server.on('message', common.mustCall((msg, { address, port }) => {

diff --git a/test/parallel/test-dns-resolveany.js b/test/parallel/test-dns-resolveany.js
@@ -6,8 +6,6 @@ const assert = require('assert');
 const dgram = require('dgram');
 const dnsPromises = dns.promises;
 
-common.crashOnUnhandledRejection();
-
 const answers = [
  { type: 'A', address: '1.2.3.4', ttl: 123 },
  { type: 'AAAA', address: '::42', ttl: 123 },