0

I am using Javascript Promises for the first time and ran into something I don't understand.

What I am trying to do is create a validation phase which runs around and checks things - eventually waiting for all promises to resolve.

To do this, I create a validation promise:

 validate = function(data) {
     var p = new Promise(function(resolve, reject)){

Here I define a promises array for all the different things I will be doing:

         var all_promises = Array();

Now do things like this Sequelize call while adding promises into this array (Sequelize returns promises):

         all_promises.push(resBooking);
         resBooking.count(...).then(...).catch(...);

I have logging statements that demonstrate we got through then and everything is dandy. Now all I need to do is wait!

        Promise.all(all_promises).then(function(){
            p.resolve();
        });

But this silly thing hangs - it waits for something to complete. No CPU usage. What am I doing wrong?

KateYoak
  • 1,591
  • 3
  • 18
  • 34
  • What do you mean by hang? Does execution stop or does it sit at 100% CPU? – Halcyon Jun 23 '15 at 15:10
  • What is `resBooking`? Not a promise, is it? – Bergi Jun 23 '15 at 15:15
  • 1
    Don't use the [promise constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Jun 23 '15 at 15:16
  • Yes, Sequelie returns promises. I'll edit my question. As to hanging, it just stops and waits. No CPU usage. – KateYoak Jun 23 '15 at 15:20
  • @KateYoak: `resBooking.count()` does look like a promise, `resBooking` doesn't. Is it really? – Bergi Jun 23 '15 at 15:31
  • I expect this code to generate a run-time error on `p.resolve`, since `p` won't be defined. That error will result in the promise rejecting. You just want `resolve`. Also, you need to return `p` from the function `validate`. –  Jun 23 '15 at 15:35

1 Answers1

5

What you want is

validate = function(data) {
    var p = new Promise(function(resolve, reject)){
        var all_promises = Array();
        all_promises.push(resBooking);
        resBooking.count(...).then(...).catch(...);

        Promise.all(all_promises).then(resolve);
    });
    return p;
};

In other words, call resolve, not p.resolve(). p.resolve will generate a run-time error (p doesn't exist), which will be "swallowed" by the promise constructor and cause it to fail. However, you won't even be able to see that rejected promise from the outside, since you are also not returning it from the function.

However, although this code should work now, you are still committing the "promise constructor anti-pattern". You don't need to construct a new promise when you already have one in the form of the Promise.all. So you can just write

validate = function(data) {
    var all_promises = Array();
    all_promises.push(resBooking);
    resBooking.count(...).then(...).catch(...);
    return Promise.all(all_promises);
};

I am not sure the above is exactly what you want, though. I don't know what resBooking is, or resBooking.count. In any case, you will be waiting on the resBooking promise, rather than the result of the then and catch you are hanging off it. Depending on what you're trying to accomplish, you might want

validate = function(data) {
    var all_promises = Array();
    all_promises.push(resBooking.count(...).then(...).catch(...));
    return Promise.all(all_promises);
};
  • Wow... the moment I saw it I couldn't believe I missed it. It definitely works. Thank you for the better constructor too. That explanation made more sense than the other article. Can I do 3 upvotes? ;-) – KateYoak Jun 23 '15 at 15:48
  • @KateYoak actually, you can [award a bounty](http://meta.stackexchange.com/questions/16065/how-does-the-bounty-system-work) if you feel that the answer deserves extra reward. – Benjamin Gruenbaum Jun 23 '15 at 16:00
  • BTW, I think this would look a lot cleaner with a `.map`. – Benjamin Gruenbaum Jun 23 '15 at 16:00