Node array of function using variables from enclosing scope

Question

I am trying to do dns.reverse() on a list of ip using async.parallel().

The code is as follows:

var functions = [];
for (var i = 0; i < data.length; i++) {
    var ip = data[i].ip;
    var x = function(callback) {
        dns.reverse(ip, (err, hostnames) => {
            if (err) {
                log.error("Error resolving hostname for [" + ip + '] ' + err);
                return callback(null, err);
            }

            callback(null, hostnames);
        });
    };

    functions.push(x);
}

async.parallel(functions, (err, results) => {
    for(var i = 0; i < data.length; i++) {
        data[i]['hostnames'] = results[i];
    }

    handler(null, data);
});

What is happening is dns.reverse() is getting called with the same ip (the last one in data array) for all the calls. May be I am doing something wrong. Can somebody explain what is my mistake?

It will be called with the value of `ip` at the time it's called not at the time it's declared. — apokryfos, Jun 27 '18 at 21:45

davibq · Answer 1 · 2018-06-27T22:00:09.433

The first callback is executed after the entire for loop finished, because it's async. The value of ip will be the one in the last iteration of the loop. You could put some console.log to realize what's really happening.

The correct way of doing it might be:

async.parallel(data.map(({ ip }) => callback => {
    dns.reverse(ip, callback)
}), (err, results) => {
    for (var i = 0; i < data.length; i++) {
       data[i]['hostnames'] = results[i];
    }

    handler(null, data);  
})

Create a new array of functions based on each ip. Every function will call it's callback as dns.reverse.

Also, it might be better to return a new data array, not changing data inside the loop:

(err, results) => {
  const result = data.map((data, index) => ({
      ...data,
      hostnames: results[index]
  })

  handler(null, result);  
})

score 0 · Accepted Answer · answered Jun 27 '18 at 22:01

Thanks to @apokryfos I got a hint. To get the code working I just need to use let instead of var while declaring ip.

        var functions = [];
        for (var i = 0; i < data.length; i++) {
            let ip = data[i].ip;
            var x = function(callback) {
                dns.reverse(ip, (err, hostnames) => {
                    if (err) {
                        log.error("Error resolving hostname for [" + ip + '] ' + err);
                        return callback(null, err);
                    }

                    callback(null, hostnames);
                });
            };

            functions.push(x);
        }
        async.parallel(functions, (err, results) => {
            for(var i = 0; i < data.length; i++) {
                data[i]['hostnames'] = results[i];
            }

            handler(null, data);
        });

For anybody interested in understanding following might be helpful: How do JavaScript closures work?

Node array of function using variables from enclosing scope

2 Answers2