-1

Background Information

I have a node / express application that connects to a Redis database to query a list of values. And then for each value in the return data, I need to do a further query.

Problem

The initial query (redis "scan" command) works fine, but the HGETALL query I attempt for each item in the initial result set is not behaving as expected.

To demonstrate this problem, I added a console.log command on line 34 to print out the value of the current scan record... and then I print it out again inside the callback function of the HGETALL (line 36) but the values are different. In my mind, they should be the same... but I'm sure it's something basic about the way node.js works, that I'm missing.

Code

 27 router.get('/', function(req, res, next) {
 28         redis.send_command("SCAN", [0,"MATCH", "emergency:*"], function(err, reply) {
 29                 if (reply) {
 30                         var retdata = [];
 31                         console.log('length is: ' + reply[1].length);
 32                         for (var i = 0; i < reply[1].length; i++) {
 33                                 var emergIP = reply[1][i];
 34                                 console.log('emergIP outside the call: ' + emergIP);
 35                                 redis.hgetall(emergIP, function (err, data) {
 36                                         console.log('emergIP inside the call: ' + emergIP);
 37                                         if (data) {             
 38                                                 var temp = emergIP.split(":");
 39                                                 var key = temp[1];
 40                                                 console.log('the key is: ' + key);
 41                                                 //retdata.push({key:data.callerid});
 42                                         }
 43                                 });
 44                         }//end for loop
 45                         res.send(JSON.stringify(retdata));
 46                 } //end if
 47                 else {  
 48                         //no emergency data defined yet
 49                         var retval = {"res":false, "msg":"no emergency data defined"};
 50                         res.send(JSON.stringify(retval));
 51                 
 52                 }             
 53         }); //end send_command
 54 });
 55 

Output from the code

length is: 8
emergIP outside the call: emergency:10.1
emergIP outside the call: emergency:10.2
emergIP outside the call: emergency:10.13
emergIP outside the call: emergency:10.14
emergIP outside the call: emergency:10.18.90
emergIP outside the call: emergency:10.19
emergIP outside the call: emergency:10.20
emergIP outside the call: emergency:10.244
GET /emergency/ 200 220.368 ms - 2
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244
emergIP inside the call: emergency:10.244
the key is: 10.244

Question

I think the problem is that I'm expecting the callbacks on the send_command function to happen sequentially. In other words, maybe the problem is that I'm not supposed to have a function call within another function call? This is my first node application... and I'm learning as I go.

Any suggestions would be appreciated.

2
  • Your post is duplicate of stackoverflow.com/questions/9644197/… Commented Sep 28, 2016 at 21:07
  • @amitmah, yes i think you're right. but I guess I didn't know the right terminology to search for to find that post. I'm wondering if I should leave my question for others who might phrase their question the way I have...? Commented Sep 29, 2016 at 12:28

1 Answer 1

0

Instead of using the async.forEach you can take advantage of closures and Immediately-Invoked Functions:

// get all elements from redis
function getAll(emergIP, callback) {
  redis.hgetall(emergIP, function (err, data) {
    if (err) {
      return callback(err);
    }

    console.log('emergIP inside the call: ' + emergIP);
    if (data) {
      var temp = emergIP.split(":");
      var key = temp[1];
      console.log('the key is: ' + key);
    }

    return callback(null, data);
  });
}

// Iterate all over the results
function getResults (reply, callback) {
  var retdata = [];
  console.log('length is: ' + reply.length);
  var length = reply.length;

  if (!length) {
    return callback(null, retdata);
  }

  for (var i = 0; i < length; i++) {
    var emergIP = [i];
    console.log('emergIP outside the call: ' + emergIP);

    (function (emergIP, i) {
      getAll(emergIP, function (err, data) {
        if (err) {
          // @todo: how would you like to handle the error?
        }

        retdata.push({key:data.callerid});

        if (i === length - 1) {
          return callback(null, retdata);
        }
      });
    }(emergIP, i));
  } //end for loop
}

router.get('/', function (req, res, next) {
  redis.send_command("SCAN", [0, "MATCH", "emergency:*"], function (err, reply) {
    if (reply) {
      getResults(reply[1], function (err, data) {
        if (err) {
          // @todo: how would you like to handle this in case of error?
        }

        res.send(JSON.stringify(data));
      });
    } //end if
    else {
      //no emergency data defined yet
      var retval = { "res": false, "msg": "no emergency data defined" };
      res.send(JSON.stringify(retval));

    }
  }); //end send_command
});

EDIT

I've modified a bit the code to return the json when the iteration will finish. It is not the perfect code, because in this case if there are no results to iterate, it will not return response to the client at all. Consider using async.each: http://caolan.github.io/async/docs.html#.each

EDIT2

I've tried to decouple the code and split it into functions. You can move these functions to another file and then require them into the router. Sorry if there is any typo that does not allow the code to run properly, i didn't actually try to run it.

Sign up to request clarification or add additional context in comments.

9 Comments

This works perfectly. I will research node closures and immediately-invoked functions for future reference. Thanks so much
The only other related question is how / where should i return this data via the response object for display on the web page? I tried uncommenting the retdata.push() method but it's not working
Of course, it will not work because remember that the within the for iteration you are executing async code :) So, your code will send back to the client the response res.send(JSON.stringify(retdata)); without waiting for the redis.hgetall to finish. I will post a solution as another answer in a bit.
@Happydevdays Just modified the answer above, you can have a look.
Thanks. So it sounds like you are recommending that the best solution is to use async.each instead of the solution you have posted?
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.