Skip to content

Instantly share code, notes, and snippets.

@joeytwiddle
Last active September 24, 2024 14:49
Show Gist options
  • Save joeytwiddle/37d2085425c049629b80956d3c618971 to your computer and use it in GitHub Desktop.
Save joeytwiddle/37d2085425c049629b80956d3c618971 to your computer and use it in GitHub Desktop.
Do not use forEach with async-await

Do not use forEach with async-await

TLDR: Use for...of instead of forEach() in asynchronous code.

For legacy browsers, use for...i or [].reduce()

To execute the promises in parallel, use Promise.all([].map(...))

The problem

Array.prototype.forEach is not designed for asynchronous code. (It was not suitable for promises, and it is not suitable for async-await.)

For example, the following forEach loop might not do what it appears to do:

const players = await this.getWinners();

// BAD
await players.forEach(async (player) => {
  await givePrizeToPlayer(player);
});

await sendEmailToAdmin('All prizes awarded');

What's wrong with it?

  • The promises returned by the iterator function are not handled. So if one of them throws an error, the error won't be caught. (In Node 10, if no unhandledrejection listener has been registered, that will cause the process to crash!)
  • Because forEach does not wait for each promise to resolve, all the prizes are awarded in parallel, not serial (one by one).
  • So the loop actually finishes iterating before any of the prizes have finished being awarded! (But after they have all started being awarded).
  • As a result, sendEmailToAdmin() sends the email before any of the prizes have finished being awarded. Maybe none of them will end up being awarded (they might all throw an error)!

So how should we write this?

Process each player in serial

Fortunately if your language has async-await then it will also have the for...of construction, so you can use that.

for (const player of players) {
  await givePrizeToPlayer(player);
}

This loop will wait for one prize to be awarded before proceeding to the next one.

You could also use a traditional for(...;...;...) here but that is more verbose than we need.

Note: The airbnb style guide recommends not using for...of for web apps at the current time (2018), because it requires a large polyfill. If you are working in the browser, use the traditional for mentioned above, or Array.reduce() described below.

Process all the players in parallel

If the order doesn't matter, it may be quicker to process all the players in parallel.

await Promise.all(players.map(async (player) => {
  await givePrizeToPlayer(player);
}));

This will start awarding all the prizes at once, but it will wait for all of them to complete before proceeding to sendEmailToAdmin().

(In the example above you could use return instead of the second await, or indeed use players.map(givePrizeToPlayer). But the pattern shown above can be useful in general situations.)

Process each player in serial, using Array.prototype.reduce

Some people recommend this approach:

await players.reduce(async (a, player) => {
  // Wait for the previous item to finish processing
  await a;
  // Process this item
  await givePrizeToPlayer(player);
}, Promise.resolve());

(We are using the accumulator a not as a total or a summary, but just as a way to pass the promise from the previous item's callback to the next item's callback, so that we can wait for the previous item to finish being processed.)

This has pretty much the same behaviour as the for...of above, but is somewhat harder to understand.

It is recommended by the Airbnb style guide because it can reduce the browser bundle size and increase performance. for...of requires iterators, and some browsers require a polyfill for iterators, and that polyfill is quite large. You can decide on the trade-off between bundle size and readability.

So which array functions can I use?

TLDR: Only map(), reduce(), flatMap() and reduceRight() if used correctly

async-await works naturally with for loops and while loops, because they are written in the original function body.

But when you call out to another function, it can only work with async-await if it returns a promise, and if that promise is handled (awaited or .then()-ed).

That is why we can use .reduce() and .map() above, because in both cases we return a promise (or an array of promises) which we can await. (In the reduce case, each invocation of the callback function waits for the previous promise to resolve, to ensure sequential processing.)

But most array functions will not give us a promise back, or allow a promise to be passed from one call to the next, so they cannot be used asynchronously. So, for example, you can not use asynchronous code inside array.some() or array.filter():

// BAD
const playersWithGoodScores = await players.filter(async (player) => {
  const score = await calculateLatestScore(player);
  return score >= 100;
});

It might look like that should work but it won't, because filter was never designed with promises in mind. When filter calls your callback function, it will get a Promise back, but instead of awaiting that promise, it will just see the promise as "truthy", and immediately accept the player, regardless of what their score will eventually be.

You may be able to find a library of array functions that can work asynchronously, but the standard array functions do not.

@ShivamRawat0l
Copy link

👍

@albydeca
Copy link

albydeca commented Jul 7, 2023

thanks so much! Solved hours of pain

@msmol
Copy link

msmol commented Jul 12, 2023

Shouldn't the reduce example return the Promise from givePrizeToPlayer rather than awaiting it?

i.e.

await players.reduce(async (a, player) => {
  // Wait for the previous item to finish processing
  await a;
  // Process this item
  return givePrizeToPlayer(player);
}, Promise.resolve());

Since on every iteration except 0, a will be equal to whatever our anonymous function returned last time.

In the original example nothing is returned, so a will always be null (except iteration 0 which has Promise.resolve()), resulting in await null on all iterations except 0.

Edit: I just realized our callback is an async function which always returns a Promise, but it will be a Promise which immediately resolves to undefined, so await a will immediately resolve to undefined

@joeytwiddle
Copy link
Author

@msmol Yes your can either await or return the last promise in the function. Almost the same result.

Advantages of awaiting the last promise:

  • We use the same syntax for all promises, which may be easier to read.
  • We could add another await line after that one, without needing to move the return
  • If givePrizeToPlayer throws an error, this function will appear in the stack trace.

Advantages of returning the last promise:

  • More familiar style, since it was mandatory before async-await.
  • Slightly more efficient, because there are fewer promises in play. May be relevant if you are processing 10,000 records.

@jainamsMagneto
Copy link

Thanks for sharing

@ngangavic
Copy link

for of worked for my case. Thank you

@codewithali9
Copy link

Thanks for sharing! wish If I could star this gist twice ❤️

@raspijabi
Copy link

Man thanks oh my god. <3 Infinite love for you

@karolisgrinkevicius-home24

Thank you!!! ❤️

Copy link

ghost commented Oct 12, 2023

Thank you so much.

@kuangbeibei
Copy link

Thanks!

@AliZeynalov
Copy link

AliZeynalov commented Nov 2, 2023

thank you very much! I learned something new today. I assume this is also the case for javascript every function

@ssrkarthikeya
Copy link

I can't tell you how helpful this post is! I faced a prod issue that is exactly as described!

@ijoliet
Copy link

ijoliet commented Mar 7, 2024

thanks, help's me a lot !

@Tenessy
Copy link

Tenessy commented Mar 16, 2024

Thanks it's very useful !

@vanodevium
Copy link

await Promise.all(players.map(async (player) => {
  await givePrizeToPlayer(player);
}));

For this case, internal await is useless. Because it will try to wait each promise in the loop.
So it is better to be used like:

await Promise.all(players.map(player => givePrizeToPlayer(player)));

@ikarthikng
Copy link

Thank you for a great explanation. I ran into this issue yesterday. Initially, my code looked something like below

const entityToSave = new EntityToSave()
entityToSave.propertyOne = someValue

let idOfContentOne: number = null
arrayOfContent.forEach(async (content) => {
  const response = await getTheIdofContent(content);
  idOfContentOne = response.Id
})

entityToSave.idOfContentOne = idOfContentOne

console.log(entityToSave)

The console.log was printing out the following values

entityToSave {
  propertyOne: someValue
  idOfContentOne: null
}

When I switched to the .map method, I was getting the same results. Eventually, I moved to use normal for loop which worked for me. I hope this helps someone avoid wasting an hour like I did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment