2
var http = require('http');
var res = ["","",""];
for(i =2;i<5;i++){
   http.get(process.argv[i],function(response){
           response.setEncoding('utf8');
           str = "";
           count =i-2;
           response.on("data", function(data) {
                   str = str.concat(data);
           });
           response.on("end", function() {
                   res[count] = str;
                   console.log(count);
                   console.log(res[count]);
           });
   });
}

while(true) {
    if(res[0]!==""&&res[1]!==""&&res[2]!=="")
    {
           console.log(res[0]);
           console.log(res[1]);
           console.log(res[2]);
           break;
    }
}

I will have three URLs as the first three command-line arguments. My job is to collect the data from each of the URL's as strings and print them to console in the order they appeared in command line. Right now the code does not print anything and it is stuck in an infinite loop. What is wrong?

varimax
  • 111
  • 1
  • 13

3 Answers3

2

There are two problems in the code. First, you have a closure on a loop variable which makes the value different than you expect as guvinder372 explained. See also this answer which discusses the problem and this answer that demonstrates a better way to fix the problem using Function.bind.

The second problem is the way you set up your while loop at the end. That loop will run continuously and never allow the callback function in your http.get to run. Instead, check in the callback if the other responses have come in and once all three come in, print the output.

for(i =2;i<5;i++){
   http.get(process.argv[i],function(response){
           response.setEncoding('utf8');
           str = "";
           count =i-2;
           response.on("data", function(data) {
                   str = str.concat(data);
           });
           response.on("end", function() {
                   //Check here if responses are in
                   if(res[0]!==""&&res[1]!==""&&res[2]!=="") {
                   }
                   res[count] = str;
                   console.log(count);
                   console.log(res[count]);
           });
   });
}
Community
  • 1
  • 1
just.another.programmer
  • 8,579
  • 8
  • 51
  • 90
0

Problem is - By the time callback handler is invoked value of i has already reached 5 and it will stay 5 for all the callback handler executions.

You need to refactor your code to pass value of i to that calling method

var http = require('http');
var res = ["","",""];
for(i =2;i<5;i++)
{
  callBackDefiner(i)
}

function callBackDefiner( i )
{
   http.get(process.argv[i],function(response){
           response.setEncoding('utf8');
           str = "";
           count =i-2;
           response.on("data", function(data) {
                   str = str.concat(data);
           });
           response.on("end", function() {
                   res[count] = str;
                   console.log(count);
                   console.log(res[count]);
           });
   });
}
gurvinder372
  • 66,980
  • 10
  • 72
  • 94
0

You cannot execute multiple http requests in a for...loop cycle without waiting for a response. To write this code in a modern way you need some new construct/pattern, like Promise. You can then wait for each response, collect responses, and exit to the caller. To make an example, look at my solution for a javascript client. This will work with a small effort in Node.js too, you just have the change the way to do the request in the block function ExecutionBlock.

Supposed that we have an array of parameters we want to send to some urls / or and array of different urls, we will run then using a Promise.all construct.

Try it out in the following snippet.

To see how apply this solution to Node.js see my implementation of http get and post here and see later in this post a for more complex execution in node of asynchronous tasks.

var console = {
    log: function(s) {
      document.getElementById("console").innerHTML += s + "<br/>"
    }
  }
  // Simple XMLHttpRequest
  // based on https://davidwalsh.name/xmlhttprequest
SimpleRequest = {
    call: function(what, response) {
      var request;
      if (window.XMLHttpRequest) { // Mozilla, Safari, ...
        request = new XMLHttpRequest();
      } else if (window.ActiveXObject) { // IE
        try {
          request = new ActiveXObject('Msxml2.XMLHTTP');
        } catch (e) {
          try {
            request = new ActiveXObject('Microsoft.XMLHTTP');
          } catch (e) {}
        }
      }
      // state changes
      request.onreadystatechange = function() {
        if (request.readyState === 4) { // done
          if (request.status === 200) { // complete 
            response(request.responseText)
          } else response();
        }
      }
      request.open('GET', what, true);
      request.send(null);
    }
  }
  //PromiseAll
var promiseAll = function(items, block) {
  var self = this;
  var promises = [],
    index = 0;
  items.forEach(function(item) {
    promises.push(function(item, i) {
      return new Promise(function(resolve, reject) {
        if (block) {
          block.apply(this, [item, index, resolve, reject]);
        }
      });
    }(item, ++index))
  });
  return Promise.all(promises)
}; //promiseAll

// LP: deferred execution block
var ExecutionBlock = function(item, index, resolve, reject) {
  SimpleRequest.call('https://icanhazip.com/', function(result) {
    if (result) {
      console.log("Response[" + index + "] " + result);
      resolve(result);
    } else {
      reject(new Error("call error"));
    }
  })
}

arr = [1, 2, 3]
promiseAll(arr, (item, index, resolve, reject) => {
  console.log("Making request [" + index + "]")
  ExecutionBlock(item, index, resolve, reject);
})
.then((results) => { console.log(results) })
.catch((error) => { console.error(error) });
<div id="console" />

For a similar approach through Promise and Promise.all applied to node.js see my question here. that is about spawn the execution of generic processes in node in a async way.

A more complex example here shows how is possible to spawn and chain more than one level of http calls execution in a N * M http.get executors using Promise. So you will star executing N requests, each of them will start M requests, and thanks to Promise.all you will wait each of them, wait all results for the first M and then all results for the N * M array of requests responses.

loretoparisi
  • 15,724
  • 11
  • 102
  • 146