120

How to correctly construct a loop to make sure the following promise call and the chained logger.log(res) runs synchronously through iteration? (bluebird)

db.getUser(email).then(function(res) { logger.log(res); }); // this is a promise

I tried the following way (method from http://blog.victorquinn.com/javascript-promise-while-loop )

var Promise = require('bluebird');

var promiseWhile = function(condition, action) {
    var resolver = Promise.defer();

    var loop = function() {
        if (!condition()) return resolver.resolve();
        return Promise.cast(action())
            .then(loop)
            .catch(resolver.reject);
    };

    process.nextTick(loop);

    return resolver.promise;
});

var count = 0;
promiseWhile(function() {
    return count < 10;
}, function() {
    return new Promise(function(resolve, reject) {
        db.getUser(email)
          .then(function(res) { 
              logger.log(res); 
              count++;
              resolve();
          });
    }); 
}).then(function() {
    console.log('all done');
}); 

Although it seems to work, but I don't think it guarantees the order of calling logger.log(res);

Any suggestions?

user2127480
  • 4,623
  • 5
  • 19
  • 17
  • 1
    The code looks fine to me (recursion with the `loop` function is the way to do synchronous loops). Why do you think there is no guarantee? – hugomg Jul 09 '14 at 17:27
  • db.getUser(email) is guaranteed to be called in order. But, since db.getUser() itself is a promise, calling it sequentially does not necessarily mean the database queries for 'email' runs sequentially due to the asynchronous feature of promise. Thus, the logger.log(res) is invoked depending on which query happens to finish first. – user2127480 Jul 09 '14 at 17:42
  • 1
    @user2127480: But the next iteration of the loop is called sequentially only after the promise has resolved, that's how that `while` code works? – Bergi Jul 09 '14 at 17:48

13 Answers13

138

If you really want a general promiseWhen() function for this and other purposes, then by all means do so, using Bergi's simplifications. However, because of the way promises work, passing callbacks in this way is generally unnecessary and forces you to jump through complex little hoops.

As far as I can tell you're trying :

  • to asynchronously fetch a series of user details for a collection of email addresses (at least, that's the only scenario that makes sense).
  • to do so by building a .then() chain via recursion.
  • to maintain the original order when handling the returned results.

Defined thus, the problem is actually the one discussed under "The Collection Kerfuffle" in Promise Anti-patterns, which offers two simple solutions :

  • parallel asynchronous calls using Array.prototype.map()
  • serial asynchronous calls using Array.prototype.reduce().

The parallel approach will (straightforwardly) give the issue that you are trying to avoid - that the order of the responses is uncertain. The serial approach will build the required .then() chain - flat - no recursion.

function fetchUserDetails(arr) {
    return arr.reduce(function(promise, email) {
        return promise.then(function() {
            return db.getUser(email).done(function(res) {
                logger.log(res);
            });
        });
    }, Promise.resolve());
}

Call as follows :

//Compose here, by whatever means, an array of email addresses.
var arrayOfEmailAddys = [...];

fetchUserDetails(arrayOfEmailAddys).then(function() {
    console.log('all done');
});

As you can see, there's no need for the ugly outer var count or it's associated condition function. The limit (of 10 in the question) is determined entirely by the length of the array arrayOfEmailAddys.

Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
  • 16
    feels like this should be the selected answer. graceful and very reusable approach. – ken Jan 29 '16 at 16:25
  • 1
    Does anyone know if a catch would propagate back to the parent? For example if db.getUser were to fail, would the (reject) error propagate back up? – wayofthefuture Sep 22 '16 at 18:42
  • @wayofthefuture, no. Think of it this way ..... you can't change history. – Roamer-1888 Sep 22 '16 at 20:38
  • I guess what I'm asking is, do I need a catch block before .done? or will the parent catch get the error? – wayofthefuture Sep 23 '16 at 01:40
  • you're mostly right but what if all of your objects won't fit into one array in memory. for example when writing a database migration script. – actual_kangaroo Oct 31 '16 at 07:16
  • @EruPenkman, if this approach gives you run into memory problems, then the fall-back pattern is recursion. – Roamer-1888 Oct 31 '16 at 22:16
  • This is a great concept. I used a similar pattern that set another method that returns a promise into the initialValue of `Array.prototype.reduce` (establishing the db connection) and just kept passing that information down the chain. Seems like sorcery, but it works beautifully. –  Jan 11 '17 at 16:17
  • The thing that baffs me the f'out is the `Promise.resolve()` at the end, where is that `Promise` coming from? – Jamie Hutber Jan 12 '17 at 11:57
  • `Promise.resolve` is a static method of `Promise`. It creates a new, resolved promise. In this case, `Promise.resolve()` is necessary to provide a "starter promise", so `promise.then(...)` has something to bite on, in the first iteration of the reduction. – Roamer-1888 Jan 12 '17 at 13:15
  • Argh nice, and its the initial value of the reduce. Tidy :D – Jamie Hutber Jan 12 '17 at 14:31
  • 4
    Thanks for the answer. This should be the accepted answer. – klvs May 13 '17 at 06:07
  • @Roamer-1888 Thanks for the reminder of using reduce, that is an elegant solution. Since the original question used a condition to stop I would like to add the possibility to create an iterable that runs until a condition is fulfilled. This way you can use map or reduce with a condition. This seems so be an elegant way to avoid recursion. – jhp Aug 07 '17 at 11:37
  • @jhp, it depends on what you want. There are two phases in my answer (1) building the promise chain (2) allowing the chain to settle. If you have something testable in the build phase, then stop building when the condition is met. If you have something testable only in the settlement phase, then perform the test in each `.then(...)` in the chain and throw when the condition is met. Throwing will send you down the chain's error path, to be caught by the next `.catch(...)` (typically at the end of the chain). – Roamer-1888 Aug 07 '17 at 12:51
  • 1
    @Roamer-1888 My mistake, I misread the original question. I (personally) was looking into a solution where the intial list you need for reduce is growing as your requests settle (its a queryMore of a DB). In this case I found the idea to use reduce with a generator a quite nice separation of (1) the conditional extension of the promise chain and (2) the consumption of the returned resuls. – jhp Aug 07 '17 at 14:23
78

I don't think it guarantees the order of calling logger.log(res);

Actually, it does. That statement is executed before the resolve call.

Any suggestions?

Lots. The most important is your use of the create-promise-manually antipattern - just do only

promiseWhile(…, function() {
    return db.getUser(email)
             .then(function(res) { 
                 logger.log(res); 
                 count++;
             });
})…

Second, that while function could be simplified a lot:

var promiseWhile = Promise.method(function(condition, action) {
    if (!condition()) return;
    return action().then(promiseWhile.bind(null, condition, action));
});

Third, I would not use a while loop (with a closure variable) but a for loop:

var promiseFor = Promise.method(function(condition, action, value) {
    if (!condition(value)) return value;
    return action(value).then(promiseFor.bind(null, condition, action));
});

promiseFor(function(count) {
    return count < 10;
}, function(count) {
    return db.getUser(email)
             .then(function(res) { 
                 logger.log(res); 
                 return ++count;
             });
}, 0).then(console.log.bind(console, 'all done'));
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Can you have a look at my reply under my post? Do I understand correctly? Thx. – user2127480 Jul 09 '14 at 17:48
  • Thanks, I'm using this successfully - except – Gordon Jul 25 '14 at 18:25
  • @Gordon: except what? – Bergi Jul 26 '14 at 08:34
  • 2
    Oops. Except that `action` takes `value` as its argument in `promiseFor`. SO wouldn't let me make such a small edit. Thanks, it's very helpful and elegant. – Gordon Jul 27 '14 at 06:37
  • Why would substituting "promiseFor" for "promiseWhile" make a `for` loop when nothing else has changed? Moreover, why was it considered to be a while loop in the first place? – Roamer-1888 Jul 27 '14 at 21:10
  • 1
    @Roamer-1888: Maybe the terminology is a bit odd, but I mean that a `while` loop does test some global state while a `for` loop has its iteration variable (counter) bound to the loop body itself. In fact I've used a more functional approach that looks more like a fixpoint iteration than a loop. Check their code again, the `value` parameter is different. – Bergi Jul 27 '14 at 21:52
  • I must be missing something here. Do your `promiseFor` and `promiseWhile` intentionally coexist? – Roamer-1888 Jul 27 '14 at 22:26
  • No, they exist in different codes. The second snippet in my answer shows an improvement to the function that the OP found in that blog post, the third snippet improves the iteration pattern by avoiding the global `count`, and adapts the loop function for that (additional argument, different name). – Bergi Jul 27 '14 at 22:33
  • That's what I thought. Agreed, avoiding the outer `count` is highly desirable, but surely `promiseFor()` needs fixing. Should `promiseWhile.bind(null, condition, action)` not read `promiseFor.bind(null, condition, action, value)`? – Roamer-1888 Jul 27 '14 at 23:09
  • No, it's supposed to get the result value from the action, to test in the condition and repeat (getting a new result value) if necessary. – Bergi Jul 27 '14 at 23:18
  • 2
    OK, I see it now. As the `.bind()` obfuscates the new `value`, I think I might choose to longhand the function out for readability. And sorry if I'm being thick but if `promiseFor` and `promiseWhile` don't coexist, then how does one call the other? – Roamer-1888 Jul 27 '14 at 23:41
  • You caught me! They shouldn't. Sorry for the confusion :-/ – Bergi Jul 28 '14 at 00:56
  • Extra points for the promiseFor, thanks. Any idea why this _very usefull_ thing is not already implemented in bluebird and the likes? – Fractalf Jan 18 '16 at 13:32
  • @Fractalf: Probably because it's too trivial and too generic. Most iteration is over collections, and the popular promise libraries do have helper methods for those already (e.g. Bluebird: `Promise.mapSeries(new Array(10), …)`). When you really need a `while`-like "loop", you'd usually just write out the recursive function in full. – Bergi Jan 18 '16 at 20:11
  • @Bergi, `Promise.method` is not available in our modern browsers. Do you have a polyfill way to implement your loop without `BlueBird` help? Thx. – herve Oct 11 '16 at 17:23
  • 2
    @herve You can basically omit it and replace the `return …` by `return Promise.resolve(…)`. If you need additional safeguards against `condition` or `action` throwing an exception (like [`Promise.method` provides it](http://bluebirdjs.com/docs/api/promise.method.html)), wrap the whole function body in a `return Promise.resolve().then(() => { … })` – Bergi Oct 11 '16 at 17:29
  • 1
    @Bergi, it works smoothly, thank you! ```var promiseWhile = function(condition, action) { if (!condition()) return Promise.resolve(); return Promise.resolve(action().then(promiseWhile.bind(null, condition, action))); }; var i = 0; function myCondition() { return i < 3; } function myPromise() { return new Promise(function(resolve, reject) { setTimeout(function() { console.log('i = ', i++); resolve(); }, 2000); }); } promiseWhile(myCondition, myPromise);``` – herve Oct 12 '16 at 08:13
  • 2
    @herve Actually that should be `Promise.resolve().then(action).…` or `Promise.resolve(action()).…`, you don't need to wrap the return value of `then` – Bergi Oct 12 '16 at 10:35
  • Made an npm package for promised-for loop https://www.npmjs.com/package/promised-for – Harsh Vakharia Jan 04 '17 at 14:51
  • Tried the for loop method. I got an error action(...).then is not a function at this line `return action(value).then(promiseFor.bind(null, condition, action));` – Flyn Sequeira Oct 03 '17 at 10:54
  • @FlynSequeira That means that your `action` function, the "body" of the loop, does not (always) return a promise. It needs to, that's the whole idea of asynchronous looping. If you can't get it to work, you might want to [ask a new question](https://stackoverflow.com/questions/ask) with your complete code. – Bergi Oct 03 '17 at 11:46
  • @Bergi I'm pretty sure there is memory leaking with `promiseFor` but your promiseWhile doesn't leak, but I doesn't know why ? – HugoPoi Aug 01 '18 at 15:22
  • @HugoPoi I don't know why either, [in theory neither should leak](https://stackoverflow.com/q/29925948/1048572). Which promise implementation are you using, Bluebird? – Bergi Aug 01 '18 at 15:29
  • @Bergi I confirm your code not leaking but my code + yours => leak, I use bluebird and I think the issue is there, I add a intermediate .then after a Promise.map fixing it but I doesn't know why it solve the issue. – HugoPoi Aug 01 '18 at 16:15
  • @HugoPoi You might want to [ask a new question](https://stackoverflow.com/questions/ask) or [open an issue](https://github.com/petkaantonov/bluebird/issues) with the exact code then – Bergi Aug 01 '18 at 16:20
40

Here's how I do it with the standard Promise object.

// Given async function sayHi
function sayHi() {
  return new Promise((resolve) => {
    setTimeout(() => {
      console.log('Hi');
      resolve();
    }, 3000);
  });
}

// And an array of async functions to loop through
const asyncArray = [sayHi, sayHi, sayHi];

// We create the start of a promise chain
let chain = Promise.resolve();

// And append each function in the array to the promise chain
for (const func of asyncArray) {
  chain = chain.then(func);
}

// Output:
// Hi
// Hi (After 3 seconds)
// Hi (After 3 more seconds)
youngwerth
  • 602
  • 5
  • 11
  • 3
    how to send params in this way? – Akash khan Dec 26 '16 at 05:50
  • 4
    @khan on the chain = chain.then(func) line, you could do either: `chain = chain.then(func.bind(null, "...your params here"));` or `chain = chain.then(() => func("your params here"));` – youngwerth Dec 27 '16 at 20:12
10

Given

  • asyncFn function
  • array of items

Required

  • promise chaining .then()'s in series (in order)
  • native es6

Solution

let asyncFn = (item) => {
  return new Promise((resolve, reject) => {
    setTimeout( () => {console.log(item); resolve(true)}, 1000 )
  })
}

// asyncFn('a')
// .then(()=>{return async('b')})
// .then(()=>{return async('c')})
// .then(()=>{return async('d')})

let a = ['a','b','c','d']

a.reduce((previous, current, index, array) => {
  return previous                                    // initiates the promise chain
  .then(()=>{return asyncFn(array[index])})      //adds .then() promise for each item
}, Promise.resolve())
4Z4T4R
  • 2,340
  • 2
  • 26
  • 45
kamran
  • 101
  • 1
  • 3
  • 2
    If `async` is about to become a reserved word in JavaScript it might add clarity to rename that function here. – hippietrail Jul 06 '16 at 08:34
  • Also, is it not the case that fat arrow functions without a body in braces simply return the what the expression there evaluates to? That would make the code more concise. I might also add a comment stating that `current` is unused. – hippietrail Jul 06 '16 at 08:40
  • 2
    this is the proper way! – teleme.io May 22 '17 at 04:37
6

There is a new way to solve this and it's by using async/await.

async function myFunction() {
  while(/* my condition */) {
    const res = await db.getUser(email);
    logger.log(res);
  }
}

myFunction().then(() => {
  /* do other stuff */
})

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function https://ponyfoo.com/articles/understanding-javascript-async-await

tomasgvivo
  • 61
  • 1
  • 1
3

Bergi's suggested function is really nice:

var promiseWhile = Promise.method(function(condition, action) {
      if (!condition()) return;
    return action().then(promiseWhile.bind(null, condition, action));
});

Still I want to make a tiny addition, which makes sense, when using promises:

var promiseWhile = Promise.method(function(condition, action, lastValue) {
  if (!condition()) return lastValue;
  return action().then(promiseWhile.bind(null, condition, action));
});

This way the while loop can be embedded into a promise chain and resolves with lastValue (also if the action() is never run). See example:

var count = 10;
util.promiseWhile(
  function condition() {
    return count > 0;
  },
  function action() {
    return new Promise(function(resolve, reject) {
      count = count - 1;
      resolve(count)
    })
  },
  count)
andrewmu
  • 14,276
  • 4
  • 39
  • 37
3

I'd make something like this:

var request = []
while(count<10){
   request.push(db.getUser(email).then(function(res) { return res; }));
   count++
};

Promise.all(request).then((dataAll)=>{
  for (var i = 0; i < dataAll.length; i++) {

      logger.log(dataAll[i]); 
  }  
});

in this way, dataAll is an ordered array of all element to log. And log operation will perform when all promises are done.

Claudio
  • 642
  • 3
  • 12
  • Promise.all will call the will call promises at the same time. So the order of completion might change. The question asks for chained promises. So the order of completion should not be changed. – canbax Apr 05 '18 at 09:47
  • Edit 1: You don't need to call Promise.all at all. As long as the promises are fired they will be executed in parallel. – canbax Apr 05 '18 at 12:51
1

First take array of promises(promise array) and after resolve these promise array using Promise.all(promisearray).

var arry=['raju','ram','abdul','kruthika'];

var promiseArry=[];
for(var i=0;i<arry.length;i++) {
  promiseArry.push(dbFechFun(arry[i]));
}

Promise.all(promiseArry)
  .then((result) => {
    console.log(result);
  })
  .catch((error) => {
     console.log(error);
  });

function dbFetchFun(name) {
  // we need to return a  promise
  return db.find({name:name}); // any db operation we can write hear
}
shriek
  • 5,605
  • 8
  • 46
  • 75
1

Use async and await (es6):

function taskAsync(paramets){
 return new Promise((reslove,reject)=>{
 //your logic after reslove(respoce) or reject(error)
})
}

async function fName(){
let arry=['list of items'];
  for(var i=0;i<arry.length;i++){
   let result=await(taskAsync('parameters'));
}

}
Eric Aya
  • 69,473
  • 35
  • 181
  • 253
0
function promiseLoop(promiseFunc, paramsGetter, conditionChecker, eachFunc, delay) {
    function callNext() {
        return promiseFunc.apply(null, paramsGetter())
            .then(eachFunc)
    }

    function loop(promise, fn) {
        if (delay) {
            return new Promise(function(resolve) {
                setTimeout(function() {
                    resolve();
                }, delay);
            })
                .then(function() {
                    return promise
                        .then(fn)
                        .then(function(condition) {
                            if (!condition) {
                                return true;
                            }
                            return loop(callNext(), fn)
                        })
                });
        }
        return promise
            .then(fn)
            .then(function(condition) {
                if (!condition) {
                    return true;
                }
                return loop(callNext(), fn)
            })
    }

    return loop(callNext(), conditionChecker);
}


function makeRequest(param) {
    return new Promise(function(resolve, reject) {
        var req = https.request(function(res) {
            var data = '';
            res.on('data', function (chunk) {
                data += chunk;
            });
            res.on('end', function () {
                resolve(data);
            });
        });
        req.on('error', function(e) {
            reject(e);
        });
        req.write(param);
        req.end();
    })
}

function getSomething() {
    var param = 0;

    var limit = 10;

    var results = [];

    function paramGetter() {
        return [param];
    }
    function conditionChecker() {
        return param <= limit;
    }
    function callback(result) {
        results.push(result);
        param++;
    }

    return promiseLoop(makeRequest, paramGetter, conditionChecker, callback)
        .then(function() {
            return results;
        });
}

getSomething().then(function(res) {
    console.log('results', res);
}).catch(function(err) {
    console.log('some error along the way', err);
});
Tengiz
  • 1,902
  • 14
  • 12
0

How about this one using BlueBird?

function fetchUserDetails(arr) {
    return Promise.each(arr, function(email) {
        return db.getUser(email).done(function(res) {
            logger.log(res);
        });
    });
}
wayofthefuture
  • 8,339
  • 7
  • 36
  • 53
0

Here's another method (ES6 w/std Promise). Uses lodash/underscore type exit criteria (return === false). Note that you could easily add an exitIf() method in options to run in doOne().

const whilePromise = (fnReturningPromise,options = {}) => { 
    // loop until fnReturningPromise() === false
    // options.delay - setTimeout ms (set to 0 for 1 tick to make non-blocking)
    return new Promise((resolve,reject) => {
        const doOne = () => {
            fnReturningPromise()
            .then((...args) => {
                if (args.length && args[0] === false) {
                    resolve(...args);
                } else {
                    iterate();
                }
            })
        };
        const iterate = () => {
            if (options.delay !== undefined) {
                setTimeout(doOne,options.delay);
            } else {
                doOne();
            }
        }
        Promise.resolve()
        .then(iterate)
        .catch(reject)
    })
};
GrumpyGary
  • 201
  • 2
  • 6
0

Using the standard promise object, and having the promise return the results.

function promiseMap (data, f) {
  const reducer = (promise, x) =>
    promise.then(acc => f(x).then(y => acc.push(y) && acc))
  return data.reduce(reducer, Promise.resolve([]))
}

var emails = []

function getUser(email) {
  return db.getUser(email)
}

promiseMap(emails, getUser).then(emails => {
  console.log(emails)
})