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

`Promise.join` calls callback synchronously #1153

Open
overlookmotel opened this Issue Jul 3, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@overlookmotel
Contributor

overlookmotel commented Jul 3, 2016

Please answer the questions the best you can:

  1. What version of bluebird is the issue happening on?

v2.10.2, v3.4.1

  1. What platform and version? (For example Node.js 0.12 or Google Chrome 32)

Node v6.2.1, OS X 10.9.5

  1. Did this issue happen with earlier version of bluebird?

Yes, consistent across v2.x and v3.x


Promise.join's behavior in when it calls the callback (final argument) depends on the state of the promises it's passed. If all promises are resolved, it runs the callback synchronously; but if any of the promises are pending, it runs callback async.

Promise.join(
    Promise.resolve(1),
    Promise.resolve(2),
    Promise.resolve(3),
    function(a, b, c) {
        console.log( {a: a, b: b, c: c} );
    }
);
console.log('next sync statement');

outputs:

{ a: 1, b: 2, c: 3 }
next sync statement

But:

Promise.join(
    Promise.resolve(1),
    Promise.resolve(2).tap( function() {} ),
    Promise.resolve(3),
    function(a, b, c) {
        console.log( {a: a, b: b, c: c} );
    }
);
console.log('next sync statement');
next sync statement
{ a: 1, b: 2, c: 3 }

I'm not completely sure of the intended behavior, but I tend to think that this unpredictability isn't ideal and the callback should never be called synchronously.

@overlookmotel

This comment has been minimized.

Show comment
Hide comment
@overlookmotel

overlookmotel Jul 3, 2016

Contributor

Also, if no promises are provided, the callback doesn't run at all:

Promise.join( function(a, b, c) {
    console.log( {a: a, b: b, c: c} );
} );
console.log('next sync statement');
next sync statement

It would be pretty pointless for someone to call Promise.join with no promises to join! But still you'd expect the callback to be called or an error to be thrown.

Contributor

overlookmotel commented Jul 3, 2016

Also, if no promises are provided, the callback doesn't run at all:

Promise.join( function(a, b, c) {
    console.log( {a: a, b: b, c: c} );
} );
console.log('next sync statement');
next sync statement

It would be pretty pointless for someone to call Promise.join with no promises to join! But still you'd expect the callback to be called or an error to be thrown.

@overlookmotel

This comment has been minimized.

Show comment
Hide comment
@overlookmotel

overlookmotel Aug 31, 2016

Contributor

Any plans to backport this to bluebird v2.x? The issue remains in v2.10.2.

Contributor

overlookmotel commented Aug 31, 2016

Any plans to backport this to bluebird v2.x? The issue remains in v2.10.2.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov
Owner

petkaantonov commented Aug 31, 2016

Sure

@overlookmotel

This comment has been minimized.

Show comment
Hide comment
@overlookmotel

overlookmotel Aug 31, 2016

Contributor

OK great. I've raised a separate issue for my second point about the callback not being called if no promises are provided. #1218

Contributor

overlookmotel commented Aug 31, 2016

OK great. I've raised a separate issue for my second point about the callback not being called if no promises are provided. #1218

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Aug 31, 2016

Collaborator

I'm not sure if we should fix this in 2.0, if there is someone in the wild who is doing this (for whatever weird use case) it'd be a behavior change for them.

Collaborator

benjamingr commented Aug 31, 2016

I'm not sure if we should fix this in 2.0, if there is someone in the wild who is doing this (for whatever weird use case) it'd be a behavior change for them.

@Venryx

This comment has been minimized.

Show comment
Hide comment
@Venryx

Venryx Apr 11, 2017

Just thought I'd mention myself as such a person -- and yes, it's a kind-of weird use case. (though it's only for debugging code, of course)

Anyway, for how to restore this synchronous behavior (in my case, for a stack-trace processing library with external source-maps), see here: stacktracejs/stacktrace.js#188

Venryx commented Apr 11, 2017

Just thought I'd mention myself as such a person -- and yes, it's a kind-of weird use case. (though it's only for debugging code, of course)

Anyway, for how to restore this synchronous behavior (in my case, for a stack-trace processing library with external source-maps), see here: stacktracejs/stacktrace.js#188

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