Reputation: 13

node.js/socket.io disappearing variable

In my app I have the following:

client.on('test', function(req, fn) {
        var returnArr = [];
        redis.hkeys(req, function (err, replies) {
            replies.forEach(function(reply, i) {
                if (reply.indexOf('list.') > -1) {
                    redis.hgetall(reply.substring(5), function(err, r) {
                        returnArr.push({name:r['name'],index:i});
                        console.log(returnArr);
                    });
                }
            });
            console.log(returnArr);
        });
        console.log(returnArr);
    });

For some reason, the second and third logs contain a blank array even though the array is declared once at the beginnning of the event. Any ideas?

EDIT: Sorry, I changed the variable name when I posted it here without thinking. This happens when it's named anything.

Upvotes: 1

Answers (4)

Pointy

Reputation: 413866

Those redis calls are asynchronous. That's why you provide them with callbacks. The code won't work even if you fix the variable name for that reason.

To elaborate: the code in the callback to "hkeys" will be invoked when the data is available. The call will return immediately, however, so your array will have nothing in it at that point.

You cannot wrap asynchronous calls in a function and expect to return a value. It simply won't work.

Instead, the general pattern is to do exactly what the redis API (and virtually everything else in the node.js world; that's kind-of the whole point in fact): give your own function a callback argument to be invoked when appropriate. In your case, it'll be inside the "hgetall" callback that's the last one to be invoked. It should figure out that your results array has as many values in it as there are keys, and so it's time to call the callback passed in to your function.

(I should note that it's not clear what you're trying to do, given that the overall function appears to be a callback to something.)

Another approach would be to use some sort of "promise" pattern, though that's really just a restructuring of the same idea.

edit — the general pattern for an API with a callback would be something like this:

function yourAPI( param1, param2, callback ) {
  // ...
  some.asynchronousFunction( whatever, function( result ) {
    callback( result );
  }
}

Now in your case you're making multiple asynchronous service requests, and you'd need to figure out when it's time to invoke the callback. I think you'd probably want to iterate through the "replies" from the call to get the keys and extract the list of ones you want to fetch:

    redis.hkeys(req, function (err, replies) {
        var keys = [];
        replies.forEach(function(reply, i) {
            if (reply.indexOf('list.') > -1) {
                keys.push( reply.substring(5) );
            }
        });
        keys.forEach( function( key ) {
                redis.hgetall(key, function(err, r) {
                    returnArr.push({name:r['name'],index:i});

                    if (returnArr.length === keys.length) {
                      // all values ready
                      callback( returnArr );
                    }
                });

        });

Upvotes: 2

user1207456

Reputation:

@Pointy answered this tersely already, but let me explain it a bit more clearly: Those nested functions are not being run in the order you think they are.

Node.js is non-blocking, and uses Javascript's implicit event loop to execute them when ready. Here's your code with line numbers:

/*01*/ client.on('test', function(req, fn) {
/*02*/     var returnArr = [];
/*03*/     redis.hkeys(req, function (err, replies) {
/*04*/         replies.forEach(function(reply, i) {
/*05*/             if (reply.indexOf('list.') > -1) {
/*06*/                 redis.hgetall(reply.substring(5), function(err, r) {
/*07*/                     returnArr.push({name:r['name'],index:i});
/*08*/                     console.log(returnArr);
/*09*/                 });
/*10*/             }
/*11*/         });
/*12*/         console.log(returnArr);
/*13*/     });
/*14*/     console.log(returnArr);
/*15*/ });
/*16*/ //Any other code you have after this.

So, what's the order of execution of this thing?

Line 1: Register the event handler for the 'test' event.

Line 16: Start running any other code to be run during this pass through the event loop

Line 2: A 'test' event has been received at some point by the event loop and is now being handled, so returnArr is initialized

Line 3: A non-blocking IO request is performed, and a callback function is registered to execute when the proper event is queued into the event loop.

Line 14-15: The last console.log is executed and this function is finished running, which should end the current event being processed.

Line 4: The request event returns and the callback is executed. The forEach method is one of the few blocking Node.js methods with a callback, so every callback is executed on every reply.

Line 5: The if statement is executed and either ends (goes to line 10) or enters the block (goes to line 6)

Line 6: A non-blocking IO request is performed, adding a new event to the event loop and a new callback to be run when the event comes back.

Line 9: Finishes the registration of the callback.

Line 10: Finishes the if statement

Line 11: Finishes the `forEach callbacks.

Line 12: Executes the second console.log request, which still has nothing in the returnArr

Line 7: One of the events returns and fires the event handler. The returnArr is given the new data.

Line 8: The first console.log is executed. Depending on which event this is, the length of the array will be different. Also the order of the array elements DOES NOT have to match the order of the replies listed in the replies array.

Essentially, you can look at the more deeply nested functions as executing after the entirety of the less-deeply nested functions (because that's what's happening, essentially), regardless of whether the method contains statements after nested non-blocking callback or not.

If this is confusing to you, you can write your callback code in a Continuation Passing Style so it's obvious that everything in the outer function is executed before the inner function, or you can use this nice async library to make your code look more imperative.

This, I think, answers your real question, rather than the one you've entered.

Upvotes: 1