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

Firefox: calling resolve() don't resolve then() #1022

Closed
toverux opened this Issue Feb 26, 2016 · 9 comments

Comments

Projects
None yet
5 participants
@toverux

toverux commented Feb 26, 2016

I'm using BlueBird version 3.3.3.
Platform: Ubuntu with Firefox 45 / Chromium 48

When I call resolve(), the promise listener registered with .then() is never called on Firefox, but it is on Chromium.

Relevant code :

// Used to suppress the click event when the promise is resolved().
const makeSilent = (resolve) => {
    return () => resolve(); // This is called, after that I don't know what happens
};

Dialog = class Dialog {
    static confirm(opts) {
        return new Promise((resolve) => {
            let $confirm = findDialog('confirm');
            refreshDialog($confirm, opts);

            // We call resolve when the 'OK' button is clicked.
            getOk($confirm).click(makeSilent(resolve));

            showDialog($confirm);
        });
    }
};

// The promise consumer :
Dialog.confirm(options).then(() => thisFunctionIsNeverCalledOnFirefox());

By tracing the code with some console.logs, I see that the resolve in makeSilent is called, but after that, nothing more happens. No errors in the console.
I also tried to remove makeSilent by replacing .click(makeSilent(resolve)) with .click(resolve), but it's doesn't work too.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Feb 28, 2016

Collaborator

I just wanted to update you that although no one commented yet we're debating how to fix this.

You can work around it by calling setScheduler. The problem is that the low latency scheduler (that uses MutationObservers under the hood) probably doesn't work in that particular case.

Collaborator

benjamingr commented Feb 28, 2016

I just wanted to update you that although no one commented yet we're debating how to fix this.

You can work around it by calling setScheduler. The problem is that the low latency scheduler (that uses MutationObservers under the hood) probably doesn't work in that particular case.

@toverux

This comment has been minimized.

Show comment
Hide comment
@toverux

toverux Mar 4, 2016

Thank you, it works. I'm using Promise.setScheduler(task => task()); now. Is Bluebird exposing better asynchronous schedulers somewhere? Or is a synchronous scheduler, like mine, acceptable ?

toverux commented Mar 4, 2016

Thank you, it works. I'm using Promise.setScheduler(task => task()); now. Is Bluebird exposing better asynchronous schedulers somewhere? Or is a synchronous scheduler, like mine, acceptable ?

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Mar 4, 2016

Collaborator

@toverux a synchronus scheduler is problematic since it can break some invariants about promises like the order of things. You can use setTimeout as a scheduler or even NativePromise.prototype.resolve.

Collaborator

benjamingr commented Mar 4, 2016

@toverux a synchronus scheduler is problematic since it can break some invariants about promises like the order of things. You can use setTimeout as a scheduler or even NativePromise.prototype.resolve.

@toverux

This comment has been minimized.

Show comment
Hide comment
@toverux

toverux Mar 4, 2016

Ok, thank you for your help :)

toverux commented Mar 4, 2016

Ok, thank you for your help :)

@fimbulvetr

This comment has been minimized.

Show comment
Hide comment
@fimbulvetr

fimbulvetr Mar 25, 2016

Hello, I'm affected by this issue as well. I've adopted Bluebird because it was cross browser and I'd hate to have this be an issue. I was able to workaround by the above or using setTimeout, which I am not in favor of. I would like to see a fix for this in the library, so my code is not having to use workarounds everywhere. I understand bluebird is fast, but fast means nothing if not reliable. Workarounds throughout my codebase means i might as well use alternative libraries.

fimbulvetr commented Mar 25, 2016

Hello, I'm affected by this issue as well. I've adopted Bluebird because it was cross browser and I'd hate to have this be an issue. I was able to workaround by the above or using setTimeout, which I am not in favor of. I would like to see a fix for this in the library, so my code is not having to use workarounds everywhere. I understand bluebird is fast, but fast means nothing if not reliable. Workarounds throughout my codebase means i might as well use alternative libraries.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov Mar 26, 2016

Owner

I cannot reproduce on firefox 45

Owner

petkaantonov commented Mar 26, 2016

I cannot reproduce on firefox 45

@ritterb82

This comment has been minimized.

Show comment
Hide comment
@ritterb82

ritterb82 Mar 28, 2016

I just want to add I'm also seeing a weird bug that just started with iOS 9.3 mobile iPhone that may be related.

As soon as i updated code that was working find before stopped working. Tracing it down it seems like there is some race condition or something causing promises to not return.

Here's my basic code:

console.log("before");
return Promise.resolve().then(function() {
console.log("after");
});

I can get the "before" to consistently log, but sometimes the "after" doesn't. And as soon as it fails, there is no recovery.

In my specific situation, i'm actually using HammerJS with a swipe and taps in a Cordova App that triggers this race condition. I don't know if that related but I thought it might help.

Again, as soon as I updated to iOS9.3 it just basically stopped working correctly.

ritterb82 commented Mar 28, 2016

I just want to add I'm also seeing a weird bug that just started with iOS 9.3 mobile iPhone that may be related.

As soon as i updated code that was working find before stopped working. Tracing it down it seems like there is some race condition or something causing promises to not return.

Here's my basic code:

console.log("before");
return Promise.resolve().then(function() {
console.log("after");
});

I can get the "before" to consistently log, but sometimes the "after" doesn't. And as soon as it fails, there is no recovery.

In my specific situation, i'm actually using HammerJS with a swipe and taps in a Cordova App that triggers this race condition. I don't know if that related but I thought it might help.

Again, as soon as I updated to iOS9.3 it just basically stopped working correctly.

@ritterb82

This comment has been minimized.

Show comment
Hide comment
@ritterb82

ritterb82 Mar 29, 2016

Just a followup,

I changed my code to new Promise(function(resolve, reject) { resolve() }).then(...) but was still having issues with the promise not resolving.

As suggested above I'm now wrapping all resolve()'s in timeouts like

new Promise(function(resolve, reject) {
setTimeout(function() { resolve(); }, 0);
}).then(...)

This seems to fix the issue, but now i have wrap all the all the resolve / rejects to make sure this race condition does't happen.

ritterb82 commented Mar 29, 2016

Just a followup,

I changed my code to new Promise(function(resolve, reject) { resolve() }).then(...) but was still having issues with the promise not resolving.

As suggested above I'm now wrapping all resolve()'s in timeouts like

new Promise(function(resolve, reject) {
setTimeout(function() { resolve(); }, 0);
}).then(...)

This seems to fix the issue, but now i have wrap all the all the resolve / rejects to make sure this race condition does't happen.

@toverux

This comment has been minimized.

Show comment
Hide comment
@toverux

toverux Apr 12, 2016

@petkaantonov Will try that asap. Thank you!
PS: I think you forgot to remove some issue's labels?

toverux commented Apr 12, 2016

@petkaantonov Will try that asap. Thank you!
PS: I think you forgot to remove some issue's labels?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment