CODEWITHSUNDEEP
javascriptmongodbpromise
Siyu
Siyu

Reputation: 12079

Collect promises generated within promises in forEach

I want to display the progress of a migration operation to mongodb.

The script looks like:

let promises = [];
mylist.forEach(idx => {
    myCollection.find({id: idx}).toArray().then(msgs => {
        promises.push(myCollection2.insertMany(msgs.map(msg => ({
            msg: msg,
            otherFields: others
        }))))
    })
});

// function to display the progress:
allProgress(promises,
  (p) => {
     console.log(`% Done = ${p.toFixed(2)}`);
});
function allProgress(proms, progress_cb) {
    let d = 0;
    progress_cb(0);
    proms.forEach((p) => {
      p.then(()=> {    
        d ++;
        progress_cb( (d * 100) / proms.length );
      });
    });
    return Promise.all(proms);
}

This won't work because promises is empty when allProgress() is called.

How can I correctly collect all promises before calling allProgress()?

UPDATE

In the process of producing a MCVE, I came up with

let promises = [];
[1,2,3].forEach(idx => {

    test(1000).then(promises.push(test(10000)));
});

console.log(promises.length);
// function to display the progress:
allProgress(promises,
  (p) => {
     console.log(`% Done = ${p.toFixed(2)}`);
});

function test(ms) {
    return new Promise((resolve) => {
        setTimeout(() => {
            console.log(`Waited ${ms}`);
            resolve();
        }, ms);
    });
}

function allProgress(proms, progress_cb) {
    let d = 0;
    progress_cb(0);
    proms.forEach((p) => {
        p.then(() => {
            d++;
            progress_cb((d * 100) / proms.length);
        });
    });
    return Promise.all(proms);
}

This script, to my surprise, works... Why isn't equivalent to my original script?

UPDATE2

[1,2,3].forEach(idx => {
    test(1000).then(_ => {
        promises.push(test(10000))
    });
});

This should be the MCVE, which does not work.

Upvotes: 1

Views: 56

Answers (2)

basic
basic

Reputation: 3408

The .find() function is async so while you are still in the process of finding elements the forEach loop itself moves on. In the end you end up waiting for your .find().

What you could do is inside of the .then() callback, check the index of the current forEach item, if you are at the last item then we know all promises have been returned. So call your allProgress function there.

This should allow enough time for waiting for everything to come together. Additionally, by checking against the index, we know that we will only call your allPromises function at completion. Not multiple times as each forEach loop occurs. 

let promises = [];
mylist.forEach((idx, index) => {
    myCollection.find({id: idx}).toArray().then(msgs => {
        promises.push(myCollection2.insertMany(msgs.map(msg => ({
            msg: msg,
            otherFields: others
        }))));
        if((index + 1) === mylist.length){
            // function to display the progress:
            allProgress(promises, (p) => {
                console.log(`% Done = ${p.toFixed(2)}`);
            });
        }
    })
});

function allProgress(proms, progress_cb) {
    let d = 0;
    progress_cb(0);
    proms.forEach((p) => {
      p.then(()=> {    
        d ++;
        progress_cb( (d * 100) / proms.length );
      });
    });
    return Promise.all(proms);
}

Edit: Your MCVE (latest edit) is failing for the exact same reason. Your requests are async which is allowing the loop to progress without waiting. Once again, check the index and call when all done. 

let promises = [];
let entries = [1, 2, 3]
entries.forEach((idx, index) => {
    test(1000).then(_ => {
        promises.push(test(10000))
        if((index + 1) === entries.length) {
          console.log(promises.length);
          // function to display the progress:
          allProgress(promises,
            (p) => {
               console.log(`% Done = ${p.toFixed(2)}`);
          });
         }
    });
});

function test(ms) {
    return new Promise((resolve) => {
        setTimeout(() => {
            console.log(`Waited ${ms}`);
            resolve();
        }, ms);
    });
}



function allProgress(proms, progress_cb) {
    let d = 0;
    progress_cb(0);
    proms.forEach((p) => {
        p.then(() => {
            d++;
            progress_cb((d * 100) / proms.length);
        });
    });
    return Promise.all(proms);
}

Upvotes: 2

zhenyu wu
zhenyu wu

Reputation: 43

myCollection.find({id: idx}) is async operation. so you can like this:

let promises = [];
mylist.forEach(idx => {
    myCollection.find({id: idx}).toArray().then(msgs => {
       promises.push(myCollection2.insertMany(msgs.map(msg => ({
           msg: msg,
           otherFields: others
       }))))
      allProgress(promises,
        (p) => {
            console.log(`% Done = ${p.toFixed(2)}`);
      });
  })
});
function allProgress(proms, progress_cb) {
   let d = 0;
    progress_cb(0);
   proms.forEach((p) => {
     p.then(()=> {    
       d ++;
       progress_cb( (d * 100) / proms.length );
     });
   });
   return Promise.all(proms);
}

Upvotes: 0

Related Questions