Promise.all() not waiting for async process

Question

In node.js I am trying to loop through some items, complete an async process for each one and then wait for each to be complete before the next one starts. I must be doing something wrong as the Promise.all() is not waiting for any of the async processes to complete! My code is below:

getChildLessons() {

 return new Promise((resolve, reject) => {

  Promise.all(

    //nested for loop is needed to get all information in object / arrays
   this.lessons.levels.map((item, i) => {

      item.childlevels.map((childItem, iChild) => {

        return ((i, iChild) => {

          //return async process with Promise.resolve();
          return this.horseman
          .open(childItem.url)
          .html()
          .then((html) => {
            //adding some information to an array
          })
          .then(() => {
              return Promise.resolve();
          }).catch((err) => {
            reject(err);
          });

        })(i, iChild);

      });
  })

  // Promise.all().then()
).then(() => {
  resolve(this.lesson);
})
.catch((err) => {
  console.log(err);
});
});
}

I am fairly new to async with node.js so please could you provide a clear example if possible.

Hi Mike, I have tried wrapping it in a Promise.resolve() but i get the same result. — Atomicts, Aug 29 '17 at 16:31
First of all, I recommend you put the inside of the .all part into a function to see what happens easily and make sure it returns an iterable i.e array, object etc. In current way , it is very difficult to understand what happens — Mumin Korcan, Aug 29 '17 at 16:44
Why don't you create an array variable (var myArray = []) and then move your map(s) code out of the Promise.all? Then instead of returning each promise like you are doing now push it into the array (myArray.push(promise)) instead. Then pass THAT array to Promise.all. This way you can verify that you have an iterable of promises which is what Promise.all expects. — Mike Cheel, Aug 29 '17 at 16:49

trincot · Answer 1 · 2021-10-19T11:05:08.280

Two issues that need to be fixed to make it work:

The callback provided to the outer map call does not have a return statement, so by consequence that map creates an array in which all elements are undefined. You need to return the result of child.items.map, i.e. an array of promises.
Once the above is fixed, the outer map will return an array of arrays. That 2D array of promises needs to be flattened to a simple array of promises. You can do this with [].concat and the spread syntax.

So the first lines of your code should become:

Promise.all(
    [].concat(...this.lessons.levels.map((item, i) => {
        return item.childlevels.map((childItem, iChild) => {

Add a closing parenthesis -- to close the argument list of concat( -- at the appropriate spot.

Some other remarks:

The following code is useless:

              .then(() => {
                  return Promise.resolve();
              })

The promise on which .then is called is by definition resolved at the moment the callback is called. To return a resolved promise at that very moment does not add anything useful. To return the promise on which .then is called is just fine. You can just remove the above .then call from the chain.

Near the end you call resolve(this.lesson). This is an example of the promise constructor anti pattern. You should not create a new Promise, but instead return the result of the Promise.all call, and inside its .then call return this.lesson so that it becomes the promised value.

Chaining all the promises

To chain all the promises instead of using Promise.all, it is easiest if you use the async/await syntax. Something like this:

async getChildLessons() {
    for (let item of this.lessons.levels) {
        for (let childItem of item.childlevels) {
            let html = await this.horseman
                                 .open(childItem.url)
                                 .html();
            //adding some information to an array
            // ...
        }
    }
    return this.lesson;
}

Hi, trincot, thank you for your answer, it's really clear and helpful! However do you know of a way to make each of the promises in the array go one after the other instead of all at the same time? — Atomicts, Aug 29 '17 at 21:35

score 0 · Answer 2 · answered Aug 29 '17 at 16:40

maybe try doing something like this ?

let firstPromise = new Promise((resolve,reject) => {
    // your logic/code
})

//add more promises you want into array if u want


 Promise.all([firstPromise])
.then((response) => {
    //do stuff with response
})
.catch((error) => {
    //do stuff with error
})

score 0 · Answer 3 · answered Aug 29 '17 at 17:29

The line of code below fails to return anything. You are attempting to pass an array of undefined to Promise.all

this.lessons.levels.map((item, i) => {...})

There are several more problems with your code, though. The block below is completely unnecessary. It literally does nothing except add an extra block to your code

return ((i, iChild) => {...})(i, iChild);

You don't need to return a Promise from the main function. The result of Promise.all() IS a Promise.

Taking the above into account, here is a code snippet.

// an array of Promises
var levelPromises = this.lessons.levels.map((item, i) => {

    var childPromises = item.childlevels.map((childItem, iChild) => {
        return this.horseman.open(childItem.url)
        //...
    })

    return Promise.all(childPromises)
})

return Promise.all(levelPromises)

Promise.all() not waiting for async process

3 Answers3

Chaining all the promises