async.forEach: final callback getting called before execution of each task

Question

I have a user schema in mongodb database (which i am handling via mongoose). I want to update the details of a user via the values provided by the client through a webpage. The details are as follows:

We have a mongoose schema User which has an attribute called authorities[] i.e. each user can hold multiple of object types called authorities. The userSchema can be seen below:

`

var UserSchema = Schema({
      name: {type: String, required: true, max: 50},
      username:{type: String, required: true, unique: true, max: 30},
      password:{type: String, required: true, max: 30},
      authorities:[{type: Schema.ObjectId, ref: 'Authority'}],
      mobileNo:{type: Number},
      superadmin:{type: Boolean, default: false}
    });

`

Each Authority has an id and a name as can be seen below:

`

var AuthoritySchema = Schema({
    name: {type: String, required: true, max: 100}
  });

`

The admin sends the updated values in a POST request from the client webpage. We want to find the user and then update its values in the database with the client provided values.

The relevant portion of the webpage code is as below:

`

var url = "/db/users/edit?token=" + token; // constructing the url for post operation
 var params = {"token": token, "userId": user_id, "userName": user_name, "authorities": auth_list.toString(), "superadmin": admin_status}; // constructing the params for post operation
 $.post(url, params, function(data, status){
   alert(data);
 });

`

The router function which handles this call is as below:

`

/* POST for editing a user */
router.post('/users/edit', function (req, res, next){
  console.log('Inside db router edit user function with decoded request = ' + req.decoded.sub + ' and request user_id as ' + req.body.userId+ ' and request userName as '+req.body.userName+ ' and authorities as ' + req.body.authorities + ' and admin status as ' + req.body.superadmin);
  if (req.decoded.su){
    next();
  } else{
    res.render('index', {title: 'Home Page', errors: [{msg: 'You don\'t have the privileges to access the page.'}]});
  }
}, user_controller.user_update_post);

`

The relevant function in the controller is as below:

`

// Handle User update on POST
exports.user_update_post = function(req, res) {
    console.log("inside user controller.update_post");
    var auth_name_array = req.body.authorities.split(","); // We get the authority names in a string in req.body.authorities and split it on commas here
    var auth_id_array = []; // We want to find the corresponding authority ids because the userSchema takes an array of ids in the authority field and not authority names. We will use this array to store them in. We want to store all authority ids in this array and only then pass the full array on to the findByIdAndUpdate function that is why using async.forEach
    async.forEach(auth_name_array, function(auth_name, callback){
      Authority.find({'name': auth_name}, '_id', function(err, authId){ // We find the corresponding authority in the database and return its id
          if (err) res.send(err);
          console.log("authId: " + authId); // The id is fetching properly and output is good
          auth_id_array.push(authId); // We store the id in our array
          console.log("auth_id_array.length is now " + auth_id_array.length); // The id is added properly and array length is incrementing by 1 with each push
          });      
      callback();
    }, function(err){
      if (err) res.send(err);
      console.log("here to update the record with auth_id_array.length as " + auth_id_array.length); // However, here array length is shown as 0 and the authorities supplied by client are not getting updated in database. This part of the code is seeing an empty array
      User.findByIdAndUpdate(req.body.userId, {$set: {name: req.body.userName, superadmin: req.body.superadmin, authorities: auth_id_array}}, function(err, result){
        if (err) res.send(err);
        res.send(result);
      });
    });
};

` The problem is that even though authority ids are being fetched properly and pushed onto the auth_id_array, it is happening after the auth_id_array is being passed onto for findByIdAndUpdate database operation. This is counter-intuitive since this piece of code is in final callback and should run only after each of the async.forEach tasks have run. The console output to back this up is as follows:

Inside db router edit user function with decoded request = admin and request user_id as 59cf64989cbff357fc3b85aa and request userName as Mummy10 and authorities as SDO Bali,Revenue,Miscllaneous and admin status as true
inside user controller.update_post
here to update the record with auth_id_array.length as 0
authId: { _id: 59c4ea3efaebb61c19af9432 }
auth_id_array.length is now 1
authId: { _id: 59c4ea8933294b1c2f1962ee }
auth_id_array.length is now 2
authId: { _id: 59c4eaa165ccc01c3c7bc07e }
auth_id_array.length is now 3

See the third line of the console output. It should have been printed after all authority ids have been fetched and pushed onto the array.

Please help! I am stuck on this since past 48 hours!! This is my final hope.

Answer 1

You're calling callback before the callback to Authority.find() gets called. Instead, move the call to callback inside it:

Authority.find({'name': auth_name}, '_id', function(err, authId){ // We find the corresponding authority in the database and return its id
    if (err) return callback(err);
    console.log("authId: " + authId); // The id is fetching properly and output is good
    auth_id_array.push(authId); // We store the id in our array
    console.log("auth_id_array.length is now " + auth_id_array.length); // The id is added properly and array length is incrementing by 1 with each push
    callback();
});      

Notice how it also calls callback(err) when an error happens.

Some other issues in your final callback:

if (err) res.send(err);

Once you sent a response, make sure that you actually stop running the following code. You can do that by returning:

if (err) return res.send(err);

Same issue here:

  User.findByIdAndUpdate(req.body.userId, {$set: {name: req.body.userName, superadmin: req.body.superadmin, authorities: auth_id_array}}, function(err, result){
    if (err) res.send(err);
    res.send(result);
  });

That should be:

if (err) res.send(err)
else res.send(result);

Or:

if (err) return res.send(err);
res.send(result);

Finally: because you're building an array inside the async.forEach "loop", you could also use async.map().