Async parallel HTTP request

Question

I'm having a control flow problem with an application loading a large array of URLs. I'm using Caolan Async and the NPM request module.

My problem is that the HTTP response starts as soon as the function is added to the queue. Ideally I want to build my queue and only start making the HTTP requests when the queue starts. Otherwise the callbacks start firing before the queue starts - causing the queue to finish prematurely.

var request = require('request') // https://www.npmjs.com/package/request
    , async = require('async'); // https://www.npmjs.com/package/async

var myLoaderQueue = []; // passed to async.parallel
var myUrls = ['http://...', 'http://...', 'http://...'] // 1000+ urls here

for(var i = 0; i < myUrls.length; i++){
    myLoaderQueue.push(function(callback){

        // Async http request
        request(myUrls[i], function(error, response, html) {

            // Some processing is happening here before the callback is invoked
            callback(error, html);
        });
    });
}

// The loader queue has been made, now start to process the queue
async.parallel(queue, function(err, results){
    // Done
});

Is there a better way of attacking this?

robertklep · Accepted Answer · 2015-08-01 12:38:40Z

34

Using for loops combined with asynchronous calls is problematic (with ES5) and may yield unexpected results (in your case, the wrong URL being retrieved).

Instead, consider using async.map():

async.map(myUrls, function(url, callback) {
  request(url, function(error, response, html) {
    // Some processing is happening here before the callback is invoked
    callback(error, html);
  });
}, function(err, results) {
  ...
});

Given that you have 1000+ url's to retrieve, async.mapLimit() may also be worth considering.

edited Aug 1, 2015 at 12:38

answered Aug 1, 2015 at 12:32

robertklep

205k38 gold badges421 silver badges413 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

ChrisRich Over a year ago

Works perfectly. Thanks.

xShirase Over a year ago

How would that be less problematic in ES6?

robertklep Over a year ago

@xShirase ES6 has block scoping using let, which would scope i to the for block; in the existing code, with var, you'd run into scoping issues when referencing myUrls[i]

xShirase Over a year ago

damn, you're right, and that's awesome! thanks for taking the time to answer

robertklep Over a year ago

@Jonny depends on your exact requirements, but I guess any Promise-based HTTP library (got, http.min, axios) combined with Promise.allSettled() (or a Promise-based rate limiter/queue)

|

Thomas Decaux · Accepted Answer · 2016-04-27 10:15:02Z

9

If you're willing to start using Bluebird and Babel to utilize promises and ES7 async / await you can do the following:

let Promise = require('bluebird');
let request = Promise.promisify(require('request'));

let myUrls = ['http://...', 'http://...', 'http://...'] // 1000+ urls here

async function load() {
  try {
    // map myUrls array into array of request promises
    // wait until all request promises in the array resolve
    let results = await Promise.all(myUrls.map(request));
    // don't know if Babel await supports syntax below
    // let results = await* myUrls.map(request));
    // print array of results or use forEach 
    // to process / collect them in any other way
    console.log(results)
  } catch (e) {
    console.log(e);
  }
}

edited Apr 27, 2016 at 10:15

Thomas Decaux

22.8k3 gold badges129 silver badges140 bronze badges

answered Aug 1, 2015 at 12:27

krl

5,3164 gold badges40 silver badges55 bronze badges

4 Comments

ChrisRich Over a year ago

Can't use this answer since my project is ES5 based and don't want to rewrite the entire program

ChrisRich Over a year ago

...but, cudos for posting Node.js ES7!

Michael Over a year ago

You can also do await* myUrls.map(request), instead of Promise.all.

SimpleJ Over a year ago

@MichaelVayvala That syntax might still work with Babel, but it is no longer part of the spec.

trex005 · Accepted Answer · 2015-08-01 12:40:33Z

0

I'm pretty confident you experiencing the results of a different error. By the time your queued functions are evaluating, i has been redefined, which might result in it appearing like you missed the first URLs. Try a little closure when you are queing the functions.

var request = require('request') // https://www.npmjs.com/package/request
    , async = require('async'); // https://www.npmjs.com/package/async

var myLoaderQueue = []; // passed to async.parallel
var myUrls = ['http://...', 'http://...', 'http://...'] // 1000+ urls here

for(var i = 0; i < myUrls.length; i++){
    (function(URLIndex){
       myLoaderQueue.push(function(callback){

           // Async http request
           request(myUrls[URLIndex], function(error, response, html) {

               // Some processing is happening here before the callback is invoked
               callback(error, html);
           });
       });
    })(i);
}

// The loader queue has been made, now start to process the queue
async.parallel(queue, function(err, results){
    // Done
});

edited Aug 1, 2015 at 12:40

answered Aug 1, 2015 at 12:34

trex005

5,1354 gold badges32 silver badges42 bronze badges

2 Comments

trex005 Over a year ago

My answer may be correct, but robertklep's is better. Wish he had posted 5 minutes earlier or I was 5 minutes slower.

robertklep Over a year ago

Yeah familiar problem :-) BTW, myUrls[i] should be myUrls[URLIndex].

Collectives™ on Stack Overflow

Async parallel HTTP request

3 Answers 3

7 Comments

4 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

7 Comments

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related