Node JS for loop and array push

Question

I have 1,211,434 IP addresses that needed to be converted into geolocations. I found an api that answers this question by using GET request. But the thing is, the when using a for loop, I can not send the ip address and receive the description correctly.

Majorly I have two questions:

1. I just can not output the ip_and_info array, and can't find the reason. Can anybody tell me what went wrong?

2. Now, the code I wrote can retrieve all the information that I need, there are around 200 ip addresses in the test_ip.txt. Would there be a potential problem if I try to send all those 1M IP addresses?

Is there anyone can give me some advice?

Much Appreciated.

My code is as below:

fs = require('fs')
async = require("async")
http = require('http')

ip_and_info = []
// getIPInfo("1.171.58.24")


fs.readFile("../test_ips.txt", "utf-8", (err, content, printArr) => {

    content = content.split("\n")

    async.each(content, (ip) => {
        content = getIPInfo(ip)
        // console.log(ip)
    }, (err) => {
        if (err) {
            console.log(err)
        } else {
            console.log(ip_and_info)
        }
    })

    // for (i in content) {
    //     ((ip) => {
    //         getIPInfo(ip)
    //     })(content[i])
    // }


});



function getIPInfo(ipAddress) {

    options = {
        host: 'freegeoip.net',
        path: '/csv/' + ipAddress
    }

    request = http.get(options, function(response) {
        // console.log('STATUS: ' + response.statusCode)
        // console.log('HEADERS: ' + JSON.stringify(response.headers))

        // Buffer the body entirely for processing as a whole.
        bodyChunks = []
        response.on('data', function(chunk) {

            bodyChunks.push(chunk)

        }).on('end', function() {

            body = Buffer.concat(bodyChunks)
            content = body.toString('ascii')
            ip_and_info.push(content)
            console.log(content)
            return content

        })
    })

    request.on('error', function(e) {
        console.log('ERROR: ' + e.message)
    })
}

Much Appreciated!

Answer 1

The problem lies in this line

content = getIPInfo(ip)

getIPInfo should be an async function. One way of doing it would be to send a callback to the function and in the function return the output in the callback.

async.each(content, getIPInfo, (err) => {
    if (err) {
        console.log(err)
    } else {
        console.log(ip_and_info)
    }
})

And in the getIPInfo function

function getIPInfo(ipAddress, callback) {
  .....
  .....
  ip_and_info.push(content)
  callback();
}

Also, instead of using async.each use async.eachSeries or async.eachLimit else it will try to send request for all 1,211,434 ips .

Answer 2

Use Promise.
Use the let and const keywords. Seriously, implicit global aren't fun.
Decide whether to use ' or " and stick with it, it is way more readable.

With Promise, no need for async or your ip_and_info variable.

'use strict';

const fs = require('fs'),
    http = require('http');

fs.readFile('../test_ips.txt', 'utf-8', (err, content) => {
    content = content.split('\n');

    Promise.resolve().then(() => {
        return getAllIPInfo(content);
    }).then((ipsInfos) => {
        console.log('Info:' + ipsInfos);
    }).catch((error) => {
        console.error('Error: ' + error);
    });
});

function getAllIPInfo(ipsAddress) {
    return new Promise((resolve, reject) => {
        let ipsInfo = [];
        ipsAddress.reduce((previous, current, index, ips) => {
            return previous.then(() => {
                return getIPInfo(ips[index]).then((content) => {
                    ipsInfo.push(content);
                    return Promise.resolve();
                });
            });
        }, Promise.resolve()).then(() => {
            resolve(ipsInfo);
        }).catch((error) => {
            reject(error);
        });
    });
}

function getIPInfo(ipAddress) {
    return new Promise((resolve, reject) => {
        let options = {
            host: 'freegeoip.net',
            path: '/csv/' + ipAddress
        };

        http.get(options, function(response) {
            // console.log('STATUS: ' + response.statusCode)
            // console.log('HEADERS: ' + JSON.stringify(response.headers))

            // Buffer the body entirely for processing as a whole.
            let bodyChunks = [];

            response.on('data', function(chunk) {
                bodyChunks.push(chunk);
            }).on('end', function() {
                let body = Buffer.concat(bodyChunks),
                    content = body.toString('ascii');

                resolve(content);
            });
        }).on('error', function(e) {
            console.log('ERROR: ' + e.message);
            reject(e);
        });
    });
}

Answer 3

I think your problem might be that you are re-declaring the 'content' variable each loop you make.

So perhaps change the loop to this so you don't reset the variable each time the loop executes. I hope that fixes your issue:

  IPList = content.split("\n")

    async.each(IPList, (ip) => {
        IPGeoLocation = getIPInfo(ip)
        console.log(IPGeoLocation)
    }, (err) => {

As for doing this with a million IPs, I cant see major problem as long as you have a decent amount of memory on your computer. You might like to add a 'wait' call so you don't hammer the server so consistently. They might block you! I would wait 1 second between each call by adding

sleep(1000);

after getting the IP.