Calling done on an array of http.get requests in Node.js

Question

I have an array of URLs that I'm using a for loop to call http.get requests. Since this is an async process, I'd like to call done after ALL requests have returned.

Here is my current attempt:

grunt.registerTask('verify', function() {
    var done = this.async();
    var promises = [];
    var urlPrefix = 'http://example.com/';
    for(var i = 0; i < deployableFiles.length; i++) {

        (function(i) {
            var deferred = Q.defer();
            promises.push(deferred);
            var file = deployableFiles[i];
            var path =  file.filetype + '/' + getVersionedFileName(file.basename, file.filetype);
            http.get(urlPrefix + path, function(res) {
                deferred.resolve();
                if(res.statusCode === 200) {
                    grunt.log.oklns(path + ' was found on production server.');
                } else {
                    grunt.log.error('Error! ' + path + ' was not found on production server!');
                }
            }).on('error', function(e) {
                grunt.log.error("Got error: " + e.message);
                done();
            });
        })(i);
    }

    Q.all(promises)
    .done(function() {
        // Everything executed correctly
        return done();
    }, function(reason) {
        // There was an error somewhere
        return done(false);
    });
});

I'm sure it's just me not wrapping my head around the whole async nature of node correctly, but is there anything glaringly obvious to anyone else?

I've searched about using http with the Q library, and it appears it might be required to use Q.nfcall to get this to work. I'm just having trouble seeing WHY I'd have to do that. (I'm not adverse to actually doing that, I'm more curious than anything else)

Thanks!

Could you be more explicit about the problem you're facing? What did you expect it to do, and what is it actuually doing? — Chris Tavares
– Chris Tavares, Commented Jun 8, 2014 at 22:53

nowk · Accepted Answer · 2014-06-08 23:12:21Z

3

If this is not a typo, promises.push(deferred) should be pushed the promise promises.push(deferred.promise).

function foo() {
  ...
  return defer.promise;
}

// => foo().then(function() ...);

Q.all([
  foo(),
  foo(),
  ...
]).done(function() ...);

Q.all expects an array of promises. https://github.com/kriskowal/q#combination

Q.nfcall is just sugar around that if

working with functions that make use of the Node.js callback pattern, where callbacks are in the form of function(err, result)

https://github.com/kriskowal/q#adapting-node

edited Jun 8, 2014 at 23:12

answered Jun 8, 2014 at 23:02

nowk

33.2k2 gold badges37 silver badges41 bronze badges

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

1 Comment

Jim Wharton Over a year ago

Thank you for the quick answer and info on Q.nfcall. You were correct, my typing was WAY too quick and I left of 'promise' when pushing onto my deferred array.

Benjamin Gruenbaum · Accepted Answer · 2014-06-09 00:01:50Z

2

You should always perform promisification at the lowest level possible. That makes reasoning about concurrency a lot easier.

function getPing(url){
    return new Q.Promise(function(resolve,reject){
         http.get(url,function(res){
             // note this will _not_ wait for the whole request
             // but just the headers.
             if(res.statusCode === 200) resolve();
             else reject();
         });
    });
}

This would let you do:

grunt.registerTask('verify', function() {
    var done = this.async();
    var urlPrefix = 'http://example.com/';
    var pings = deployableFiles.map(function(file){
        var path =  file.filetype + '/' + 
                        getVersionedFileName(file.basename, file.filetype);
        return getPing(urlPrefix + path);
    });
    Q.all(pings).then(done).catch(function(reason) {
        // There was an error somewhere
        // this will happen as soon as _one_ promise rejected
        return done(false);
    });
});

This can be further shortened by using a better promise library like Bluebird.

answered Jun 9, 2014 at 0:01

Benjamin Gruenbaum

278k90 gold badges524 silver badges524 bronze badges

1 Comment

Jim Wharton Over a year ago

I very much liked your way of thinking. Since your solution was the one I went with, I marked yours as correct. Although I see the value in the other answer as well.

mscdex · Accepted Answer · 2014-06-08 23:27:46Z

-1

You can also do this with async:

var urlPrefix = 'http://example.com/';
async.each(deployableFiles, function(file, cb) {
  var path =  file.filetype
              + '/'
              + getVersionedFileName(file.basename, file.filetype);
  http.get(urlPrefix + path, function(res) {
    if (res.statusCode === 200)
      grunt.log.oklns(path + ' was found on production server.');
    else
      grunt.log.error('Error! ' + path + ' was not found on production server!');
    cb();
  }).on('error', function(e) {
    grunt.log.error("Got error: " + e.message);
    cb(e);
  });
}, function(err) {
  // all done

  if (err) throw err;

  // all successful
});

answered Jun 8, 2014 at 23:27

mscdex

107k15 gold badges201 silver badges159 bronze badges

Collectives™ on Stack Overflow

Calling done on an array of http.get requests in Node.js

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related