phantom phantom - 11 days ago 5
Javascript Question

node wait for response from one GET request before triggering the next one

I am making two successive calls to an API using

node
.

Firstly, I make a
GET
request, following which I use data from the response to make the second
GET
request. I want to ensure the first
API
call has finished before triggering the second call.

I have tried using both node packages
request
/
request-promise
to make a server side
GET
request to the Spotify API.

Initially, I was using
request
but just installed
request-promise
. Perhaps this is not necessary. Please see my code below. Can anyone spot any obvious mistake I am making? I expected it to work after following this post.

var request = require('request-promise');

request(url, function(error, response, body) {
var body = JSON.parse(body);
for (var i = 0; i < body.length; i++) {
test.push(body[i].url)
}

}
res.send(req.body);
}).then(function() {
for (var i = 0; i < test.length; i++) {
request(url2, function(error, response, body) {
test2.push(body);
});
}
})

Answer

Using request-promise for your issue is a good idea because promises are better equiped to handle async code than callbacks, especially when dealing with multiple requests and error handling.

However, your usage of the request-promise is incorrect. The callback-style you have here is the way request works but not promises.

The success callback in promises is passed into the then hook, not into the initial promise construction.

Additionally, the error isn't passed to the success (first) callback but is caught in the rejection (second) callback (or in the catch hook).

Read more about promises

For the second request, you are actually trying to make multiple request, one for every url in the provided array. To ensure all those request are finished before proceeding, you should use Promise.all.

Here's how you'd structure your promises using request-promise.

request({ uri: url, json: true }) // this is how the docs for request-promise say you can request a json
  .then(function (json) { // accumulate array of urls and pass them along the promise chain
    var urls = [];
    for (var i = 0; i < json.length; i++) {
      urls.push(json[i].url)
    }
    return urls;
  })
  .then(function (urls) {
    var promises = [];
    for (var i = 0; i < urls.length; i++) {
      promises.push(request(urls[i])); // create a promise for each url and fire off the request
    }
    return Promise.all(promises); // continue down the chain once all promises (request) are resolved
  })
  .then(function (arrayOfResultsFromEachPreviousRequest) {
    // ...
  });

Using ES6 arrow functions and Array.prototype.map, you can reduce the code even further:

request({ url: url, json: true })
  .then(json => json.map(item => item.url))
  .then(urls => Promise.all(urls.map(url => request(url))))
  .then(arrayOfResultsFromEachPreviousRequest => {
    // ...
  });

If you want to use the callback-styled request, things would be much more painful, especially the second part that waits on multiple requests.

For comparison purposes, here's an approach using callback (hell) without error handling:

request.get({ url:url, json:true }, function(err, res, json) {
  var urls = [];
  for (var i = 0; i < json.length; i++) {
    urls.push(json[i].url)
  }
  makeRemainingRequests(urls, function(arrayOfResultsFromEachPreviousRequest) {
    // ...
  });
});

function makeRemainingRequests(urls, cb) {
  var results = new Array(urls.length).fill(false); // array holding the results of all requests
  for (var i = 0; i < urls.length; i++) {
    // wrap each request in an IIFE so we have a correct value of i
    (function(idx) {
      request(urls[idx], function(err, res, body) {
        results[idx] = body;
        // if all results obtained, call the callback
        if (results.every(function (res) { return !!res; })) {
          cb(results);
        }
      });
    })(i);
  }
}
Comments