Skip to content

Commit

Permalink
Fixes #1468, stabilize unhandled rejection tests
Browse files Browse the repository at this point in the history
  • Loading branch information
petkaantonov committed May 29, 2019
1 parent 8991667 commit 60ef7a0
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 35 deletions.
129 changes: 119 additions & 10 deletions src/debuggability.js
Expand Up @@ -31,6 +31,77 @@ var longStackTraces = !!(util.env("BLUEBIRD_LONG_STACK_TRACES") != 0 &&
var wForgottenReturn = util.env("BLUEBIRD_W_FORGOTTEN_RETURN") != 0 &&
(warnings || !!util.env("BLUEBIRD_W_FORGOTTEN_RETURN"));

var deferUnhandledRejectionCheck;
(function() {
var promises = [];

function unhandledRejectionCheck() {
for (var i = 0; i < promises.length; ++i) {
promises[i]._notifyUnhandledRejection();
}
unhandledRejectionClear();
}

function unhandledRejectionClear() {
promises.length = 0;
}

if (util.isNode) {
deferUnhandledRejectionCheck = (function() {
var timers = require("timers");

This comment has been minimized.

Copy link
@benjamingr

benjamingr May 29, 2019

Collaborator

libraries like Jest do override this sometimes.

This comment has been minimized.

Copy link
@petkaantonov

This comment has been minimized.

Copy link
@benjamingr

benjamingr May 29, 2019

Collaborator

I... don't think that's a safe assumption to make but I concede this is better than nothing.

Sinon does both, but admittedly almost everyone who I know using lolex does .install which overrides the globals.

var timerSetTimeout = timers.setTimeout;
var timer = timerSetTimeout(unhandledRejectionCheck, 1);
timer.unref();

return function(promise) {
promises.push(promise);
if (typeof timer.refresh === "function") {
timer.refresh();
} else {
timerSetTimeout(unhandledRejectionCheck, 1).unref();
}
};
})();
} else if (typeof document === "object" && document.createElement) {
deferUnhandledRejectionCheck = (function() {
var iframeSetTimeout;

function checkIframe() {
if (document.body) {
var iframe = document.createElement("iframe");
document.body.appendChild(iframe);
if (iframe.contentWindow &&
iframe.contentWindow.setTimeout) {
iframeSetTimeout = iframe.contentWindow.setTimeout;
}
document.body.removeChild(iframe);
}
}
checkIframe();
return function(promise) {
promises.push(promise);
if (iframeSetTimeout) {
iframeSetTimeout(unhandledRejectionCheck, 1);
} else {
checkIframe();
}
};
})();
} else {
deferUnhandledRejectionCheck = function(promise) {
promises.push(promise);
setTimeout(unhandledRejectionCheck, 1);
};
}

es5.defineProperty(Promise, "_unhandledRejectionCheck", {
value: unhandledRejectionCheck
});
es5.defineProperty(Promise, "_unhandledRejectionClear", {
value: unhandledRejectionClear
});
})();

Promise.prototype.suppressUnhandledRejections = function() {
var target = this._target();
target._bitField = ((target._bitField & (~IS_REJECTION_UNHANDLED)) |
Expand All @@ -40,10 +111,7 @@ Promise.prototype.suppressUnhandledRejections = function() {
Promise.prototype._ensurePossibleRejectionHandled = function () {
if ((this._bitField & IS_REJECTION_IGNORED) !== 0) return;
this._setRejectionIsUnhandled();
var self = this;
setTimeout(function() {
self._notifyUnhandledRejection();
}, 1);
deferUnhandledRejectionCheck(this);
};

Promise.prototype._notifyUnhandledRejectionIsHandled = function () {
Expand Down Expand Up @@ -144,46 +212,87 @@ Promise.hasLongStackTraces = function () {
return config.longStackTraces && longStackTracesIsSupported();
};


var legacyHandlers = {
unhandledrejection: {
before: function() {
var ret = util.global.onunhandledrejection;
util.global.onunhandledrejection = null;
return ret;
},
after: function(fn) {
util.global.onunhandledrejection = fn;
}
},
rejectionhandled: {
before: function() {
var ret = util.global.onrejectionhandled;
util.global.onrejectionhandled = null;
return ret;
},
after: function(fn) {
util.global.onrejectionhandled = fn;
}
}
};

var fireDomEvent = (function() {
var dispatch = function(legacy, e) {
if (legacy) {
var fn;
try {
fn = legacy.before();
return !util.global.dispatchEvent(e);
} finally {
legacy.after(fn);
}
} else {
return !util.global.dispatchEvent(e);
}
};
try {
if (typeof CustomEvent === "function") {
var event = new CustomEvent("CustomEvent");
util.global.dispatchEvent(event);
return function(name, event) {
name = name.toLowerCase();
var eventData = {
detail: event,
cancelable: true
};
var domEvent = new CustomEvent(name.toLowerCase(), eventData);
var domEvent = new CustomEvent(name, eventData);
es5.defineProperty(
domEvent, "promise", {value: event.promise});
es5.defineProperty(
domEvent, "reason", {value: event.reason});
return !util.global.dispatchEvent(domEvent);

return dispatch(legacyHandlers[name], domEvent);
};
// In Firefox < 48 CustomEvent is not available in workers but
// Event is.
} else if (typeof Event === "function") {
var event = new Event("CustomEvent");
util.global.dispatchEvent(event);
return function(name, event) {
var domEvent = new Event(name.toLowerCase(), {
name = name.toLowerCase();
var domEvent = new Event(name, {
cancelable: true
});
domEvent.detail = event;
es5.defineProperty(domEvent, "promise", {value: event.promise});
es5.defineProperty(domEvent, "reason", {value: event.reason});
return !util.global.dispatchEvent(domEvent);
return dispatch(legacyHandlers[name], domEvent);
};
} else {
var event = document.createEvent("CustomEvent");
event.initCustomEvent("testingtheevent", false, true, {});
util.global.dispatchEvent(event);
return function(name, event) {
name = name.toLowerCase();
var domEvent = document.createEvent("CustomEvent");
domEvent.initCustomEvent(name.toLowerCase(), false, true,
domEvent.initCustomEvent(name, false, true,
event);
return !util.global.dispatchEvent(domEvent);
return dispatch(legacyHandlers[name], domEvent);
};
}
} catch (e) {}
Expand Down
5 changes: 3 additions & 2 deletions test/browser/mocha.js
Expand Up @@ -4215,7 +4215,7 @@ function Runnable(title, fn) {
this.fn = fn;
this.async = fn && fn.length;
this.sync = ! this.async;
this._timeout = 2000;
this._timeout = 1000 * 60 * 5;
this._slow = 75;
this._enableTimeouts = true;
this.timedOut = false;
Expand Down Expand Up @@ -4331,6 +4331,7 @@ Runnable.prototype.resetTimeout = function(){

if (!this._enableTimeouts) return;
this.clearTimeout();

this.timer = setTimeout(function(){
if (!self._enableTimeouts) return;
self.callback(new Error('timeout of ' + ms + 'ms exceeded'));
Expand Down Expand Up @@ -5185,7 +5186,7 @@ function Suite(title, parentContext) {
this._afterEach = [];
this._afterAll = [];
this.root = !title;
this._timeout = 2000;
this._timeout = 1000 * 60 * 5;
this._enableTimeouts = true;
this._slow = 75;
this._bail = false;
Expand Down
4 changes: 4 additions & 0 deletions test/mocha/bluebird-multiple-instances.js
Expand Up @@ -57,6 +57,10 @@ if (isNodeJS) {
setTimeout(function(){
d1.fulfill();
d2.fulfill();
setTimeout(function() {
Promise1._unhandledRejectionCheck();
Promise2._unhandledRejectionCheck();
}, 100);
}, 1);
return Promise.all([spy1.promise, spy2.promise]);
});
Expand Down
19 changes: 11 additions & 8 deletions test/mocha/helpers/util.js
Expand Up @@ -99,7 +99,7 @@ module.exports = {
var promise = new Promise(function() {
resolve = arguments[0];
reject = arguments[1];
}).timeout(500);
});
var ret = function(fn) {
ret.callback = fn;
return ret.node;
Expand All @@ -122,7 +122,7 @@ module.exports = {
var promise = new Promise(function() {
resolve = arguments[0];
reject = arguments[1];
}).timeout(500);
});
domain = require('domain').create();
domain.on("error", function(e) {
try {
Expand All @@ -136,18 +136,18 @@ module.exports = {
return promise;
},

//Since there is only a single handler possible at a time, older
//tests that are run just before this file could affect the results
//that's why there is 500ms limit in grunt file between each test
//beacuse the unhandled rejection handler will run within 100ms right now
onUnhandledFail: function(testFunction) {
Promise._unhandledRejectionClear();
return new Promise(function(resolve, reject) {
var err = new Error("Reporting handled rejection as unhandled from: " +
testFunction);
Promise.onPossiblyUnhandledRejection(function() {
reject(err);
});
Promise.delay(25).then(resolve);
Promise.delay(150).then(function() {
Promise._unhandledRejectionCheck();
resolve();
});
}).lastly(function() {
Promise.onPossiblyUnhandledRejection(null);
});
Expand Down Expand Up @@ -183,7 +183,10 @@ module.exports = {
resolve(e);
}
});
Promise.delay(200).then(function() {
Promise.delay(50).then(function() {
Promise._unhandledRejectionCheck();
return Promise.delay(1);
}).then(function() {
var message = "Expected onPossiblyUnhandledRejection to be called " +
total + " times but it was only called " + cur + " times";
reject(new Error(message));
Expand Down
29 changes: 17 additions & 12 deletions test/mocha/unhandled_rejections.js
Expand Up @@ -517,9 +517,11 @@ describe("Promise.onUnhandledRejectionHandled", function() {
var a = new Promise(function(){
throw reason;
});
setTimeout(function(){

setTimeout(function() {
Promise._unhandledRejectionCheck();
a.then(assert.fail, function(){});
}, 200);
}, 1);

return Promise.all([spy1.promise, spy2.promise]);
});
Expand Down Expand Up @@ -564,9 +566,10 @@ describe("global events", function() {

var promise = new Promise(function() {throw err;});
setTimeout(function() {
Promise._unhandledRejectionCheck();
promise.then(assert.fail, function(){});
}, 150);
}).timeout(500);
}, 1);
});
});

specify("are fired with local events", function() {
Expand Down Expand Up @@ -603,9 +606,10 @@ describe("global events", function() {

var promise = new Promise(function() {throw err;});
setTimeout(function() {
Promise._unhandledRejectionCheck();
promise.then(assert.fail, function(){});
}, 150);
}).timeout(500);
}, 1);
});

});
});
Expand Down Expand Up @@ -661,27 +665,28 @@ if (windowDomEventSupported) {
assert.strictEqual(e.detail.promise, promise);
assert.strictEqual(e.detail.reason, undefined);
assert.strictEqual(e.promise, promise);
assert.strictEqual(e.reason, err);
assert.strictEqual(e.reason, undefined);
order.push(3);
});
attachEvent("rejectionhandled", function(e) {
assert.strictEqual(e.detail.promise, promise);
assert.strictEqual(e.detail.reason, undefined);
assert.strictEqual(e.promise, promise);
assert.strictEqual(e.reason, err);
assert.strictEqual(e.reason, undefined);
assert.strictEqual(e.defaultPrevented, true);
order.push(4);
resolve();
});

setTimeout(function() {
Promise._unhandledRejectionCheck();
promise.then(assert.fail, function(r) {
order.push(5);
assert.strictEqual(r, err);
assert.deepEqual(order, [1,2,3,4,5]);
});
}, 150);
}).timeout(500);
}, 1);
});

})
});
Expand Down Expand Up @@ -722,7 +727,7 @@ if (windowDomEventSupported) {
worker.postMessage("reject");
}).then(function () {
assert.deepEqual(order, [1, 2]);
}).timeout(500);
});
});
});
}
Expand Down Expand Up @@ -796,7 +801,7 @@ describe("issues", function () {

specify("GH-1501-2", function testFunction() {
var ret = onUnhandledFail(testFunction);
Promise.reduce([Promise.delay(100), Promise.reject(new Error("reason"))],
Promise.reduce([Promise.delay(1), Promise.reject(new Error("reason"))],
function() {},
{}).caught(function() {});
return ret;
Expand Down
2 changes: 1 addition & 1 deletion tools/build.js
Expand Up @@ -212,7 +212,7 @@ function buildBrowser(sources, dir, tmpDir, depsRequireCode, minify, npmPackage,
entries: entries,
detectGlobals: false,
standalone: "Promise"
}).exclude('async_hooks');
}).exclude('async_hooks').exclude("timers");
return Promise.promisify(b.bundle, b)().then(function(src) {
var alias = "\
;if (typeof window !== 'undefined' && window !== null) { \
Expand Down
4 changes: 2 additions & 2 deletions tools/mocha_runner.js
Expand Up @@ -94,8 +94,8 @@ module.exports = function mochaRun(progress) {
return Promise.each(testGroup, function(test, index, length) {
var mocha = new Mocha({
reporter: "spec",
timeout: 50000, //200 caused non-deterministic test failures
//when a test uses timeouts just barely under 200 ms
timeout: "300s",
enableTimeouts: true,
slow: Infinity,
bail: true
});
Expand Down

1 comment on commit 60ef7a0

@Trott
Copy link

@Trott Trott commented on 60ef7a0 Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit makes the tests unreliable for me locally on macOS using Node.js 12.11.1. It doesn't fail every time but it fails most of the time if I use npm test and less often if I use node tools/test --run=unhandled_rejections.js:

55 passing (222ms)
  1 failing

  1) issues GH-1487-8:
     Error: Reporting handled rejection as unhandled from: function testFunction() {
        var ret = onUnhandledFail(testFunction);
        var arr = [ Promise.reject( new Error('foo') ) ];
        var p = Promise.resolve( arr );
        p.filter( function() {} ).caught( function() {} );
        return ret;
    }
      at /Users/trott/temp/bluebird/test/mocha/helpers/util.js:142:23
  From previous event:
      at onUnhandledFail (/Users/trott/temp/bluebird/test/mocha/helpers/util.js:141:16)
      at context.testFunction (/Users/trott/temp/bluebird/test/mocha/unhandled_rejections.js:869:19)
      at callFn (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:251:21)
      at Test.Runnable.run (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:244:7)
      at Runner.runTest (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:374:10)
      at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:452:12
      at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:299:14)
      at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:309:7
      at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:248:23)
      at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:271:7
      at done (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:207:5)
      at callFn (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:262:7)
      at Hook.Runnable.run (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:244:7)
      at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:259:10)
      at Immediate.<anonymous> (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:276:5)
      at processImmediate (internal/timers.js:439:21)



Error: Reporting handled rejection as unhandled from: function testFunction() {
        var ret = onUnhandledFail(testFunction);
        var arr = [ Promise.reject( new Error('foo') ) ];
        var p = Promise.resolve( arr );
        p.filter( function() {} ).caught( function() {} );
        return ret;
    }
    at /Users/trott/temp/bluebird/test/mocha/helpers/util.js:142:23
    at onUnhandledFail (/Users/trott/temp/bluebird/test/mocha/helpers/util.js:141:16)
    at context.testFunction (/Users/trott/temp/bluebird/test/mocha/unhandled_rejections.js:869:19)
    at callFn (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:251:21)
    at Test.Runnable.run (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:244:7)
    at Runner.runTest (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:374:10)
    at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:452:12
    at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:299:14)
    at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:309:7
    at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:248:23)
    at /Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:271:7
    at done (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:207:5)
    at callFn (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:262:7)
    at Hook.Runnable.run (/Users/trott/temp/bluebird/node_modules/mocha/lib/runnable.js:244:7)
    at next (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:259:10)
    at Immediate.<anonymous> (/Users/trott/temp/bluebird/node_modules/mocha/lib/runner.js:276:5)
From previous event:
    at Object.run (/Users/trott/temp/bluebird/tools/job-runner/job-runner.js:141:27)
    at runTestGroup (/Users/trott/temp/bluebird/tools/test.js:123:22)
    at /Users/trott/temp/bluebird/tools/test.js:261:16
    at processImmediate (internal/timers.js:439:21)
From previous event:
    at Object.<anonymous> (/Users/trott/temp/bluebird/tools/test.js:254:27)
    at Module._compile (internal/modules/cjs/loader.js:945:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:962:10)
    at Module.load (internal/modules/cjs/loader.js:798:32)
    at Function.Module._load (internal/modules/cjs/loader.js:711:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:1014:10)
    at internal/main/run_main_module.js:17:11

Please sign in to comment.