CODEWITHSUNDEEP
javascriptnode.jspromise
user6002037
user6002037

Reputation:

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);
    });
  }
})

Upvotes: 2

Views: 8449

Answers (1)

nem035
nem035

Reputation: 35501

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).

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);
  }
}

Upvotes: 2

Related Questions