-1

I made a file called test-for-each-loop.js in NodeJS to attempt to save a record to a mongodb database via the mongoose db wrapper. It looks like this:

const load = async () => {
    const content = [0,0,0,0,0];
    await content.forEach(async item=>{
      const job = new Job({data:item});
      await job.save();
    });
    process.exit();
};

load();

I ran this command from terminal: node test-for-each-loop.js. The result is nothing gets saved to database. I identified the problem to be await job.save() doesn't execute. My co-worker fixed my code by changing it to this:

const load = async () => {
    const content = [0,0,0,0,0];
    await Promise.all(content.map(async item=>{
      const job = new Job({data:item});
      await job.save();
    }));

    process.exit();
};

load();

I thought that because I put an await in front of the content.forEach(), that it would wait for the foreach to complete execution before firing the process.exit(). Can some one explain to me why the await content.forEach doesn't actually wait?


I noticed that if I remove process.exit() from my first script, then the await content.forEach will actually wait. How come?

John
  • 32,403
  • 80
  • 251
  • 422

3 Answers3

0

.forEach() is not async-aware. So, it just runs your entire loop, runs the code after the loop and then all the asynchronous operations started in each iteration of the loop finish sometime later after the loop is done.

If you insert process.exit(), it does not wait for the asynchronous operations to complete before exiting because you explicitly told nodejs to exit right away. If you leave the process.exit() out, nodejs does wait for the asynchronous operations to finish before it's natural time to exit as nodejs won't naturally exit while there are still outstanding timers, network operations, file operations, etc...

To actually sequence your asynchronous operations so the 2nd one doesn't start until the first one finishes and so the code after the loop doesn't execute until they are all done, you can use a plain for loop instead of .forEach().

const load = async () => {
    const content = [0,0,0,0,0];
    for (let item of content) {
      const job = new Job({data:item});
      await job.save();
    });
    process.exit();
};

load();

A plain for loop is async aware and will actually pause the loop until the await is done.

In fact, I'd say that .forEach() should pretty much be obsoleted these days because it has far, far less loop control than a regular for loop (you can't break or continue or return from in its callback) and it's not async-aware so it's really not friendly with promises at all.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

Array.forEach does not return a promise, it would just call your async function provided without an await or .then after it.

While promise.all with Array.map works I rather use a plain for loop like so:

const load = async () => {
    const content = [0,0,0,0,0];
    for(let item of content){
      const job = new Job({data:item});
      await job.save();
    }
    process.exit();
};

load();
Ido
  • 89
  • 11
0

forEach executes immediately, synchronously.

The callbacks you gave forEach do not however. await doesn't work, because in order for that to work, forEach must return a promise, but it's not aware of promises, and doesn't return anything.

The fix your co-worker gave is fine, and will execute everything in parallel. It can be simplified further:

const load = async () => {
  const content = [0,0,0,0,0];
  await Promise.all(
    content.map(data=>(new Job({data})).save())
  );
  process.exit();
};

load();

Generally the recommendation is to rewrite forEach to use simple loops.

const load = async () => {

  const content = [0,0,0,0,0];
  for(const item of content) {
    const job = new Job({data:item});
    await job.save();
  };
  process.exit();
};

load();

I'd recommend this as the 'default' solution to this, because it's more similar what most people intend when they use forEach. The solution with map and Promise.all(). is faster however, if every iteration of .save() is allowed to run in parallel.

Promise.all() will end the process as soon as one of the promises errored however. If you don't want this, use Promise.allSettled() instead.

Evert
  • 93,428
  • 18
  • 118
  • 189