Resolve of promise happening before forEach loop finishes

Question

So, I am attempting to run a server testing function, using node and request. My goal is to run through an array of server objects, build an array of successful server pings, and return a single array with the best priority (hard coded into the server objects). Here is my code so far:

//brings in http script availability
var request = require('request');

//set of test servers
var serverSet = [
      {
        "url": "http://doesNotExist.askdex.co",
        "priority": 1
      },
      {
        "url": "http://tricon.co",
        "priority": 7
      },
      {
        "url": "http://offline.bing.co",
        "priority": 2
      },
      {
        "url": "http://google.com",
        "priority": 4
      },
      {
        "url": "http://yahoo.com",
        "priority": 6
      }
];

//inform user of process taking place
console.log("We are about to test some server requests for you, please stand by.");


function serverTest(url, priority) {
    //creates options to allow for request timeout
    var options = {
        url: url,
        timeout: 5000
    };
    return new Promise (
        function(resolve, reject) {
            request(options, function(err, res, body) {
                //if (err) {console.log("There was an error: " + err)};
                //test if server responds with a positive status
                if (res !== undefined) {
                    if (res.statusCode >= 200 && res.statusCode <= 299) {
                        //console.log("response from online server is " + res.statusCode);
                        resolve({"url": url, "priority": priority});
                    } else
                    if (res.statusCode >= 300 && res.statusCode <= 399) {
                        //console.log("response from redirected server is " + res.statusCode);
                        reject("The server is not working");
                    } else
                    if (res.statusCode >= 400 && res.statusCode <= 499) {
                        //console.log("response from not working server is " + res.statusCode);
                        reject("The server is broken");
                    }//==> end of inner if/else statement
                } else {
                    reject("Server is unresponsive");
                }//==> end of outer if/else statement
            });//==> end of get request
        }
    );
};

//call of function to run program
//var test0 = serverTest(serverSet[0].url, serverSet[0].priority).then(function(resolve){console.log("resolution is: "+ JSON.stringify(resolve))}).catch(function(reject){console.log(reject)});
//console.log("Test0: "+ test0);
//var test1 = serverTest(serverSet[1].url, serverSet[1].priority).then(function(resolve){console.log("resolution is: "+ JSON.stringify(resolve))}).catch(function(reject){console.log(reject)});
//console.log("Test1: "+ JSON.stringify(test1));
//var test2 = serverTest(serverSet[2].url, serverSet[2].priority).then(function(resolve){console.log("resolution is: "+ JSON.stringify(resolve))}).catch(function(reject){console.log(reject)});
//console.log("Test2: "+ JSON.stringify(test2));
//var test3 = serverTest(serverSet[3].url, serverSet[3].priority).then(function(resolve){console.log("resolution is: "+ JSON.stringify(resolve))}).catch(function(reject){console.log(reject)});
//console.log("Test3: "+ test3);
//var test4 = serverTest(serverSet[4].url, serverSet[4].priority).then(function(resolve){console.log("resolution is: "+ JSON.stringify(resolve))}).catch(function(reject){console.log(reject)});
//console.log("Test4: "+ JSON.stringify(test4));

function findServer(array) {
    var build = [];
    return new Promise (
        function(resolution, rejection) {
            array.forEach(function(server){
                serverTest(server.url, server.priority)
                    .then(function(resolve){
                        if (typeof resolve === "object") {
                            console.log("findServer resolve = " + JSON.stringify(resolve));
                            build.push(resolve);
                            console.log("build = " + JSON.stringify(build));
                            return build;
                        }
                    })
                    .catch(function(error){
                        //console.log("Error: " + error);
                    });
            });
                    resolution(build);
            // if (onlineServers.length == 0) {
            //  //console.log("No online servers.");
            //  reject('Error: No servers are online.');
            // }//==> end of if statement
        }
    )
};

findServer(serverSet).then(function(result) {
    console.log("The result is " + JSON.stringify(result));
    }).catch(function(error){
        console.log("The error is " + error);
    });
// function findServer(array) {
//  return new Promise (
//      function(resolution, rejection) {
//          var build = [];
//          array.forEach(function(server){
//              serverTest(server.url, server.priority).then(function(resolve){
//                      if (typeof resolve === "object") {
//                          console.log("findServer resolve = " + JSON.stringify(resolve));
//                          build.push(resolve);
//                          console.log("build = " + JSON.stringify(build));
//                          resolution(build);
//                      }
//                  }).catch(function(error){
//                      //console.log("Error: " + error);
//                  });
//          });
//          // if (onlineServers.length == 0) {
//          //  //console.log("No online servers.");
//          //  reject('Error: No servers are online.');
//          // }//==> end of if statement
//      }
//  )
// };




 // findServer(serverSet).then(function(result){
    //      console.log(result);
    //      console.log(onlineServers);
    //      //condition check and modification if more than one server response is successful
    //      if (result.length > 1) {
    //          result.sort(function(a,b){
    //              return a.priority - b.priority;
    //          })
    //          console.log(result[0].url + " has the lowest available priority.");
    //      } else {
    //          console.log(result[0].url + " has the lowest available priority.");
    //      }//==> end of if statement
    //  }).catch(function(error){
    //      console.log(error);
    //  });

The commented-out code contains numerous attempts to test and figure out other ways to make this work.

Possible duplicate of [How do I return the response from an asynchronous call?](http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) — Heretic Monkey, Dec 13 '16 at 18:48
`let findServers = (servers) => Promise.all(servers.map(s => serverTest(s.url, s.priority))` — royhowie, Dec 13 '16 at 18:49
@royhowie I had this as my answer however, i deleted because it does not work for his code. Promise.all will fail and not return the statuses of all servers based on how his current code is structured. He would need to change `serverTest()` to not reject but to actually resolve a status. — peteb, Dec 13 '16 at 18:55
I am willing to rewrite some code, i've been doing it quite a bit. My goal is to filter out the bad server hits. I am open to changes, i'm trying to learn more than anything — Havamere, Dec 13 '16 at 21:16

Julien Zakaib · Answer 1 · 2016-12-19T17:41:27.483

You'll want to use Promise.all() to wait until many Promises have resolved until continuing, but be aware of this caveat (emphasis mine) :

The Promise.all(iterable) method returns a promise that resolves when all of the promises in the iterable argument have resolved, or rejects with the reason of the first passed promise that rejects.

So in your case, you need to make sure that all the promises you pass to .all() will resolve. For this you can use Promise.catch() to handle rejections from your serverTest() function and continue gracefully.

The Promise returned by catch() is rejected if onRejected throws an error or returns a Promise which is itself rejected; otherwise, it is resolved.

var serverSet = [{
  "url": "http://doesNotExist.askdex.co",
  "priority": 1
}, {
  "url": "http://tricon.co",
  "priority": 7
}, {
  "url": "http://offline.bing.co",
  "priority": 2
}, {
  "url": "http://google.com",
  "priority": 4
}, {
  "url": "http://yahoo.com",
  "priority": 6
}]

function serverTest(url, priority) {
  var options = {
    url: url,
    timeout: 5000
  }
  return new Promise((resolve, reject) => {
    // Always resolve for testing purposes
    resolve({
      "url": url,
      "priority": priority
    })
  })
}

function findServer(servers) {
  var build = []
  var promises = servers.map(server => {
    return serverTest(server.url, server.priority)
      .then(resolve => {
        // Do your validation of resolve here
        build.push(resolve)
      })
      .catch(error => {
        // By using catch, you ensure this promise chain will continue to 
        // resolve as long as you don't throw an error here or return another
        // Promise that will reject
        console.log("Server " + server.url + " failed with : " + error)
      })
  })

  return Promise.all(promises).then(values => {
    // At this point you know that all your promises have resolved, and
    // the successful ones have added an element to 'build'
    return build
  });
};

findServer(serverSet).then(result => {
  console.log("result is : " + JSON.stringify(result))
})

Edit : using map() instead of forEach + push() , thanks for suggestions in comments.

Edit 2 : Added code for complete example, but modified serverTest so that it always resolves, in order to verify that findServer populates the build array correctly.

Thank you very much for the suggested fix, but I am still running into the same issue at present which is that in my function call of: `findServer(serverSet).then(result => { console.log("The result is " + JSON.stringify(result)); }).catch(error =>{ console.log("The error is " + error); });` I still get an empty array from my build being returned in the find server function — Havamere, Dec 14 '16 at 22:15
@Havamere I modified the code example to include the `serverTest` function, but I modified it to always resolve, just to check that `findServer` was populating the `build` array correctly. It seems to do just that, which indicates that the reason you are still getting an empty array is that your `serverTest` is rejecting for some reason. You can check your console for the logs, since the errors should be printed there. — Julien Zakaib, Dec 15 '16 at 15:58
@JulienZakaib you should `return serverTest…` inside of `.map` — royhowie, Dec 16 '16 at 04:03

Resolve of promise happening before forEach loop finishes

1 Answers1