Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Juggling Async unexpected output #1270

Closed
Tachii opened this issue Jul 1, 2015 · 5 comments
Closed

Juggling Async unexpected output #1270

Tachii opened this issue Jul 1, 2015 · 5 comments

Comments

@Tachii
Copy link

Tachii commented Jul 1, 2015

So here is my code:

var http = require('http');
var dataStream = [];
var dataArr = [];
var count = 0;
/*
 Function to print results
 @dataArr - array
*/
function printResults(dataArr) {
  for (var i = 0; i < process.argv.length - 2; i++)
    console.log(dataArr[i]);
}

/*
  Function to get data from http
  @i - int
  Getting command line arguments as parametrs.
*/
function httpGet(i) {
  http.get(process.argv[2 + i], function(res) {

    res.setEncoding('utf8');

    res.on('data', function(data) {
      dataStream.push(data);
    });

    res.on('end', function() {
      dataArr[i] = (dataStream.join(""));
      dataStream = [];
      count++;
      if (count == process.argv.length - 2) {
        printResults(dataArr);
      }
    });

    res.on('error', function(e) {
      console.log("Got error: " + e.message);
    });
  });
}

for (var i = 0; i < process.argv.length - 2; i++) {
  httpGet(i);
}

And for some strange reason sometimes if i run:
learnyounode verify program.js
It show that everything is correct, sometimes it shows that there is an error.

Why can it be happening?

@francesmcmullin
Copy link

Your code makes the assumption that the 3 http responses will not overlap - that a data event for one response will never occur before the end event of the previous response, this is not always the case. I recommend you move the definition of your dataStream variable inside your httpGet function, this way each request/response will have its own variable and they cannot interfere with each other, regardless of timing.

@Tachii
Copy link
Author

Tachii commented Jul 2, 2015

@davecocoa Thanks for the answer!

I kinda switched my solution a bit, now I really get why it wasn't working.

var http = require('http');
var dataArr = [];
var count = 0;
/*
 Function to print results
 @dataArr - array
*/
function printResults(dataArr) {
  for (var i = 0; i < process.argv.length - 2; i++) {
    console.log(dataArr[i].replace('undefined', ''));
  }
}

/*
  Function to get data from http
  @i - int
  Getting command line arguments as parametrs.
*/
function httpGet(i) {
  http.get(process.argv[2 + i], function(res) {

    res.setEncoding('utf8');

    res.on('data', function(data) {
        dataArr[i] += data;
    });

    res.on('end', function() {
      count++;
      if (count == process.argv.length - 2) {
        printResults(dataArr);
      }
    });

    res.on('error', function(e) {
      console.log("Got error: " + e.message);
    });
  });
}

for (var i = 0; i < process.argv.length - 2; i++) {
  httpGet(i);
}

But I had a problem.

While adding data to array element, I always had undefined in the begining of my string.
More info here:
http://stackoverflow.com/questions/31181385/how-to-prevent-undefined-data-string-while-reading-data-from-stream

@francesmcmullin
Copy link

Why not put dataArr[i] = ""; at the start of httpGet? This way you'll have an empty string ready to add the data to, instead of attempting to add data to undefined.

@Tachii
Copy link
Author

Tachii commented Jul 2, 2015

@davecocoa Indeed, I should try this. Thanks again for the help!

@SomeoneWeird
Copy link
Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants