4

I have an asynchronous problem here. The forEach is still running when res.json({ errors }); returns so all the errors aren't picked up. How do I deal with this?

router.post('/new/user', async function (req, res, next) {
    const validateEmail = async (email) => {
      var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
      return re.test(email);
    };
    
    const reqBody = { email, password };
    
    let errors = {};

    Object.keys(reqBody).forEach(async (field) => {
      if (field === 'email' && validateEmail(await reqBody[field])) {
        errors = { ...errors, [field]: 'Not a valid Email' };
      }
      console.log(3);
    
      if (field === 'password' && password !== '' && password < 4) {
        errors = { ...errors, [field]: 'Password too short' };
      }
    });
    
    if (Object.keys(errors).length > 0) {
      res.json({ errors });
    }
}
Unmitigated
  • 76,500
  • 11
  • 62
  • 80
bp123
  • 3,217
  • 8
  • 35
  • 74
  • 1
    You can't make a `.forEach()` loop wait for an asynchronous operation inside of it. It just isn't designed to work that way. Instead, you can use a plain `for` loop with `await` inside the loop. IMO, `.forEach()` should be ditched permanently. Regular `for` loops are so much more powerful these days and easier for the interpreter to optimize too. – jfriend00 Jul 10 '20 at 05:03

4 Answers4

3

Use for...of to loop over the array elements instead and you can use await.

for(const field of Object.keys(reqBody)){
   //Code using await
}
Unmitigated
  • 76,500
  • 11
  • 62
  • 80
1

using map instead of forEach to get promises Object.keys(reqBody).map(async (field) => {...} and then using Promise. allSettled to wait for all promise resolved or rejected and you can catch that error to log

router.post('/new/user', async function (req, res, next) {
    const validateEmail = async (email) => {
      var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
      return re.test(email);
    };
    
    const reqBody = { email, password };
    
    try{
      const promises =  Object.keys(reqBody).map(async (field) => {
      if (field === 'email' && validateEmail(await reqBody[field])) {
        Promise.reject({[field]: 'Not a valid Email'})
      }
      console.log(3);
    
      if (field === 'password' && password !== '' && password < 4) {
        Promise.reject({[field]: 'Password too short'})
      }
     });
     // List of resolved and rejected promises
     const results = await Promise. allSettled(promises)
     const errors = results.filter(result => result.status === "rejected")
     if (Object.keys(results).length > 0) {
        res.json({ errors });
     }
    }catch(error){
      res.json({ error });
    }
}
Tony Nguyen
  • 3,298
  • 11
  • 19
1

I don't like the syntax of awaiting within an async loop function. But if I were to keep the existing map structure, I would probably add an array outside it to hold blank promises. Push a promise into that array each time you loop, and then resolve it after your await and your pushing of the error. Then put await Promise.all on that array in between your object key loop and setting the response.

joshstrike
  • 1,753
  • 11
  • 15
0

It's far more easy to just use a plain for loop on Object.keys(reqBody) when dealing with async stuff.

let keys =  Object.keys(reqBody)
for(let i in keys){
   ...
}
Brain Foo Long
  • 2,057
  • 23
  • 28
  • 1
    You [shouldn't ever use a `for..in` loop with arrays](https://stackoverflow.com/q/500504/8376184)! Consider a `for..of` or, if index is needed, a regular `for` instead. – FZs Jul 10 '20 at 11:23