Using promises to request order-dependent information in a for loop

Question

I have a relatively simple task; existing information is out of date, so I have to request information from an API, modify the existing info file and push it back to the server, updated. The information is in the form of a json file; simple enough. That file contains an object with an array of objects that have several properties that must be updated. This is where the problems occur; the array of objects generates an array of API requests, whose responses must match the original object that spawned the request (since the response contains info that must be updated in the object).

This is the gist of what I've managed to do so far with promises:

function main() {
    // First get existing data.
 getExistingData().then(function(result) {
   console.log(result); // It worked, return it for next 'then' to use.
   return result;
 }, function(err) {
   console.log(err); // This usually never happens.
 }).then(function(result) { // Use the existing data to generate the requests for new data.
  requestNewData(result).then(function(moddedJson) {
   console.log(moddedJson); // This happens BEFORE I get responses back from the request, which is wrong.
  });
 });
}

function getExistingData() {
 return new Promise(function(resolve, reject) {
  fetch('dataURLHere')
  .then(function(res) {
         resolve( res.json()); // Turn result into JSON, and return it.
     })
 })
}

function requestNewData(rawJson) {
 return new Promise(function(resolve) {
        // Loop over the number of objects in the original data.
  for (var i = 0; i < rawJson.length; i++) { 
            // Loop over the array of objects within each object.
      for (var multiId = 0; multiId < rawJson.hits.length; multiId++) {
       var requestUrl = "someURLConstructedFromJsonData";
                var hit = rawJson.hits[multiId];
    new Promise(function(resolve) {
     request(requestUrl, function(error, response, body) {
        if (!error && response.statusCode == 200) {
                            // Need to parse the XML response into a js object.
         parseString(body, function (err, result) {
                                hit.propertyToChange = result.propertyToChange;
                                hit.propertyToChange2 = result.propertyToChange2;
       });
        }
        else {
         console.log("No data for this item.");
        }
      resolve(hit);
     });
    })
   }
  }
  resolve(rawJson);
 })
}

Basically, the things I want to happen are: 1) Get original data. This is easy and accomplished by my code already. 2) Use original data to generate requests for each document in the data, and for each set of properties within each document. This is also not a problem. 3) Ensure the returning data from requests gets matched to existing data. THIS is the problem part that I can't wrap my head around.

Answer 1

The problem is that you're resolving too early.

The red flag is when you create a promise but never do anything with it:

new Promise(function(resolve) {
    request(requestUrl, function(error, response, body) {
    ...

That promise does get resolved correctly, but no one is waiting on it. The simple solution is Promise.all:

function requestNewData(rawJson) {
    return new Promise(function(resolve, reject) {
        var promises = [];

        for (var i = 0; i < rawJson.length; i++) {
            ...

            promises.push(new Promise(function(resolve) {
                ...
            }));
        }

        resolve(Promise.all(promises));
    });
}

Now, Promise.all(promises) will resolve with an array of results. This might not be ideal, but you can but a then on it if you just want to wait using it:

return Promise.all(promises).then(function() {
    resolve(updatedJson);
}, reject);

In this way, you could have each of the individual promises modify the response data. The promise returned by requestNewData won't resolve until all of them are done, so at that point updatedJson would be updated.

Be warned: Promise.all has fast-fail behavior. In your case, I think this is exactly what you want. But if you need to know which ones failed, or if you need to wait until all requests complete (fail or otherwise), Promise.all may not be the right thing.

PS: You should maybe reject if the request() function provides an error. Otherwise you could have holes in your data if there's a network error, without actually getting a reject.