Node.js For-Loop Inner Asynchronous Callback

Question

I have to following function that assesses if a password is in the user's password history. Here is the code:

isPasswordInPwdHistory : function(redisClient, userId, encPassword, cb) {

        var key = 'user:' + userId + ':pwdhistory';
        redisClient.llen(key, function(err, reply) {
            if (err) {
                status.results = [ ];
                xutils.addStatusResultsItem(status.results, err, null, null, null);
                cb(status);
            }
            else {
                var numPwds = reply;

                var match = false;
                var funcError;

                for (i=0; i <= numPwds - 1; i++) {
                    var error;
                    var pwdValue;
                    redisClient.lindex(key, i, function(err, reply) {
                        if (err) {
                            console.log('lindex err = ' + err);
                            error = err;
                        }
                        else {
                            console.log('lindex reply = ' + reply);
                            console.log('lindex encPassword = ' + encPassword);
                            console.log('lindex (encPassword === reply) = ' + (encPassword === reply));
                            pwdValue = reply;
                        }
                    });

                    console.log('for-loop error = ' + error);
                    console.log('for-loop pwdValue = ' + pwdValue);
                    console.log('for-loop (encPassword === pwdValue) = ' + (encPassword === pwdValue));

                    if (error) {
                        funcError = error;
                        break;
                    }
                    else if (encPassword === pwdValue) {
                        console.log('passwords match');
                        match = true;
                        break;
                    }
                }

                console.log('funcError = ' + funcError);
                console.log('match = ' + match);

                if (funcError) {
                    status.results = [ ];
                    xutils.addStatusResultsItem(status.results, err, null, null, null);
                    cb(status);
                }
                else
                    cb(match);
            }
        });
    }

Here is the console output:

for-loop error = undefined
for-loop pwdValue = undefined
for-loop (encPassword === pwdValue) = false
funcError = undefined
match = false
isPasswordInPwdHistory = false
lindex reply = 5f4f68ed57af9cb064217e7c28124d9b
lindex encPassword = 5f4f68ed57af9cb064217e7c28124d9b
lindex (encPassword === reply) = true

Once I leave the scope of the redisClient.lindex() call I lose the values. How can I pass these values for evaluation in the for-loop?

UPDATE

I refactored the code a bit to handle the matching of the new password (encPassword) vs. the existing password at index i when the redisClient.lindex() callback is issued.

isPasswordInPwdHistory : function(redisClient, userId, encPassword, cb) {

        var status = new Object();
        var key = 'user:' + userId + ':pwdhistory';

        redisClient.llen(key, function(err, reply) {
            if (err) {
                status.results = [ ];
                xutils.addStatusResultsItem(status.results, err, null, null, null);
                cb(status);
            }
            else {
                var numPwds = reply;
                var loopCt = 0;

                for (i=0; i <= numPwds - 1; i++) {
                    loopCt++;
                    redisClient.lindex(key, i, function(err, reply) {
                        if (err) {
                            status.results = [ ];
                            xutils.addStatusResultsItem(status.results, err, null, null, null);
                            cb(status);
                        }
                        else if (encPassword === reply) {
                            status.results = [ ];
                            xutils.addStatusResultsItem(status.results, null, 0, null, true);
                            cb(status);
                        }
                        else if (loopCt === numPwds && encPassword !== reply) {
                            status.results = [ ];
                            xutils.addStatusResultsItem(status.results, null, 0, null, false);
                            cb(status);
                        }
                    });
                }
            }
        });
    }

Unfortunately, caller sees status === undefined even though encPassword === reply is true and I issue the cb(status). How can I exit the for-loop for good after issuing cb(status)?

Answer 1

It looks like you have a mix of synchronous and asynchronous thinking in your logic. Your for loop is going to fire off requests to Redis as fast as it can, without waiting for Redis to respond (this is the nature of asynchronous IO). So, by the time your code runs console.log('funcError = ' + funcError); and console.log('match = ' + match);, Redis likely hasn't even responded for the very first value of i in the loop (which matches what you're finding in your output).

I would probably look into a library like async to help with this task; in particular, whilst looks like it might be a good fit. Perhaps something like:

var numPwds = reply;
...
var match = false;
var i = 0;

async.whilst(
  // keep looping until we have a match or we're done iterating the list
  function () { return match == false && i < numPwds; },

  // this is our function to actually check Redis
  function (callback) {
    redisClient.lindex(key, i, function(err, reply) {
      if (err) {
        console.log('lindex err = ' + err);
        error = err;
      }
      else {
        if (encPassword === reply) { match = true; } // this will cause `whilst` to stop.
      }
      i++;
      callback();
    });
  },

  function (err) {
    // this is called when our first function that calls
    // return match == false && i < numPwds
    // returns false, which will happen when we're out of list elements to check
    // or we have a match. At this point, match being true or false
    // let us know if we found the password.
  }
);

As a final note, unless you need to support the same password showing up multiple times in a the "used passwords" list, you can save yourself a lot of heartache by using a set or a sorted set.