JavaScript: Simple Async Issue

Question

So I'm somewhat new to asynchronous JavaScript, and I can't figure out why the '.2's are logging before the '.1's.

The only asynchronous method here is makePokemon()

My goal is that all the '.1's log before the '.2's. thanks

                sender.room().forEach(async (client) => {
                    const pokemon = await makePokemon(client.getPokemon());
                    client.setPokemon(pokemon);
                    console.log('.1');
                });
                sender.room().forEach(client => {
                    console.log('.2');
                    client.emit('redirect', {
                        yourPokemon: client.getPokemon(),
                        theirPokemon: client.getOpponent().getPokemon()
                    });
                });

Your first callback function is equivalent to (client) => { makePokemon(client.getPokemon()).then((pokemon) => { client.setPokemon(pokemon); console.log('1.'); }); } — Patrick Roberts
– Patrick Roberts, Commented Apr 26, 2020 at 7:08
I think this will do exactly what you want: const AsyncAF = require('async-af'); AsyncAF(sender.room()).forEachAF(async (client) => { const pokemon = await makePokemon(client.getPokemon()); client.setPokemon(pokemon); console.log('.1'); console.log('.2'); client.emit('redirect', { yourPokemon: client.getPokemon(), theirPokemon: client.getOpponent().getPokemon() }); }); — Scott Rudiger
– Scott Rudiger, Commented Apr 26, 2020 at 9:14

Dacre Denny · Accepted Answer · 2020-04-26 07:23:02Z

My understanding is that when using forEach() in the browser, the expectation is that the callback will execute synchronously.

You could however revise your code as follows (provided the parent function is declared async):

/* 
The following changes require the calling function to be async

async function foo() { */

/* Express iteration with for..of to achieve desired behavior */
for(const client of sender.room()) {
  
  const pokemon = await makePokemon(client.getPokemon());
  client.setPokemon(pokemon);
  console.log('.1');

}

for(const client of sender.room()) {
  console.log('.2');
  client.emit('redirect', {
    yourPokemon: client.getPokemon(),
    theirPokemon: client.getOpponent().getPokemon()
  });
}

/* } */

Alternatively, as Patrick Roberts points out, you could express this logic in part with Promise.all(). One advantage with this approach is it allows multiple async tasks (ie makePokemon) to be dispatched at once rather than in a sequential fashion (as is the case above):

/* Map each client to a promise and execute all with Promise.all() */
Promise.all(sender.room().map(async(client) => {
  const pokemon = await makePokemon(client.getPokemon());
  client.setPokemon(pokemon);
  console.log('.1');
}))
/* If all prior promises are resolved, continue with next iteration */
.then(() => {

    for (const client of sender.room()) {
      console.log('.2');
      client.emit('redirect', {
        yourPokemon: client.getPokemon(),
        theirPokemon: client.getOpponent().getPokemon()
      });
    }
})

You probably don't want to use for...of. Rather, you should await Promise.all(sender.room().map(async (client) => ...)) to avoid making the loop in the question sequential instead of parallel. Also, code snippets are for complete examples. It's not very helpful to click "Run code snippet" and immediately get some sort of error.
I would advice against using Promise.all in cases where you don't know how many promises you are going to create for example... Also, it's easier to debug when you are doing a for loop when having issues. As much as Node's async handling should be good I prefer to control the flow myself... maybe it's just a me thing
@Odinn if makePokemon() took even 1/10th of a second to execute, this answer would take 10 seconds to run for 100 clients, while using Promise.all() would still allow it to complete in close to 1/10th of a second.
@Odinn your knowledge is flawed. async functions are non-blocking, and awaiting them suspends execution of the control flow, it doesn't block the thread. It works exactly as I described.
@Odinn it's definitely in the ballpark, even when taking into account "initiating socket connections". Maybe you want to reconsider your claim.

sroes · Accepted Answer · 2020-04-26 07:14:36Z

Try waiting on all the promises:

            const promises = sender.room().map(async (client) => {
                const pokemon = await makePokemon(client.getPokemon());
                client.setPokemon(pokemon);
                console.log('.1');
            });
            await Promise.all(promises);
            sender.room().forEach(client => {
                console.log('.2');
                client.emit('redirect', {
                    yourPokemon: client.getPokemon(),
                    theirPokemon: client.getOpponent().getPokemon()
                });
            });

Parker Tailor · Accepted Answer · 2020-04-26 07:19:13Z

forEach won't wait for each loop to finish, it simply fires off a bunch of promises. If you want to explicitly wait for all those promises to finish before the second loop starts you can use map followed by Promise.all. The map will return an array of promises. Promise.all will explicitly pause until all promises in the given array have resolved. Does something like this work?

// use `map` and capture each promise in a new array
const pokemonPromises = sender.room().map(async (client) => {
  const pokemon = await makePokemon(client.getPokemon());
  client.setPokemon(pokemon);
  console.log('.1');
});

// explicitly wait for all promises to finish before continuing.
await Promise.all(pokemonPromises);

// Continue with non async stuff
sender.room().forEach(client => {
  console.log('.2');
  client.emit('redirect', {
    yourPokemon: client.getPokemon(),
    theirPokemon: client.getOpponent().getPokemon()
  });
});

is it ever better to use forEach instead of map, given that map can return these promises?

Collectives™ on Stack Overflow

JavaScript: Simple Async Issue

3 Answers 3

12 Comments

Comments

2 Comments

Linked

Hot Network Questions