forEach with JS Promise issues

Question

I have a database of users and I want to set longitude and latitude for them. However, after 6+ calls im getting error 400, bad request. I figured it's because I make too many calls to google maps API, so decided to create a setTimeout function, so I would get coordinates every 1 second.

However then I discovered my forEach acts weird. Here is the code and then I'm going to explain what's wrong. (part of the code I think is relevant)

let noOfSuccess = 0;
        let noCoords = 0;
        let forEachPromise = new Promise((resolve, reject) => {
            arr.forEach(function (user) {
                console.log('street', user.street)
                let coordPromise = new Promise((resolve, reject) => {

                    let street = user.street;
                    let city = user.city;

                    let address = street.concat(', ').concat(city);

                    const url = `https://maps.googleapis.com/maps/api/geocode/json?address=${address}&key=APIKEY`;

                    setTimeout(function () {
                        let coords = axios.get(url).then((response) => {
                            return response;
                        })
                        resolve(coords);
                    }, 1000);

                })

                coordPromise.then(response => {
                    if (response.data.results[0].types == "street_address") {
                        console.log('adres', response.data.results[0].formatted_address)
                        arrSucc.push(response.data.results[0].formatted_address);
                        noOfSuccess++;
                    } else {
                        arrFail.push(response.data.results[0].formatted_address);
                        noCoords++;
                    }
                    console.log('coordResp', 'succ', noOfSuccess, 'fail', noCoords)
                })
            });

How I want this to work: I take user from the database, I console.log the street name for test. Then I create a promise. In the promise I wait 1 second to call google API. After I get the response, i resolve this promise. Then I take the response, do some checks and console.log what's happening, either if there was a success or fail. Then I go to the next user. Prefered output then: User street -> google API calls -> log success or fail Repeat for all users.

However what's happening is: It logs ALL streets of the users, then goes to the promise, after 1 second it makes all call to the API at once without waiting 1 second each and then logs for each user if there was a succes or fail. How it looks:

    now listening for requests
 street Kwiatowa 40
street Kwiatowa 40
street Kwiatowa 43
street Kwiatowa 36
street Kwiatowa 27
street Kwiatowa 42
street Kwiatowa 29
street Kwiatowa 45
(node:5800) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): Error: Request failed with status code 400
(node:5800) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
adres Kwiatowa 36, 02-579 Warszawa, Poland
coordResp succ 1 fail 0
adres Kwiatowa 43, 02-579 Warszawa, Poland
coordResp succ 2 fail 0
adres Kwiatowa 40, 02-579 Warszawa, Poland
coordResp succ 3 fail 0
adres Kwiatowa 27, 02-579 Warszawa, Poland
coordResp succ 4 fail 0
adres Kwiatowa 29, Radom, Poland
coordResp succ 5 fail 0
adres Kwiatowa 42, 02-579 Warszawa, Poland
coordResp succ 6 fail 0
adres Kwiatowa 40, 02-579 Warszawa, Poland
coordResp succ 7 fail 0

What am I doing wrong? Is there an issue with me understanding promises or forEach loops?

Answer 1

I would avoid using Array.prototype.forEach with Promises, since in it's implementation, it doesn't wait for the callback function to be completed before iterating to the next item in the array (which explains the behaviour you mentioned). My suggestion for this case would be to use a for loop with async/await:

async function getDataForUsers () {
    for (let user of arr) {
        console.log('street', user.street)
        const response = await myPromiseFunction(user) // inside this function you can make your api calls with axios and setTimeout
        // do some stuff with response
    }
}

I would also take a look at bluebird module, which may have some interesting methods for different implementations for your use case.

Answer 2

There are couple of ways to deal with this issue. You can create a Promise based solution, use async/await or use RxJS.

For this context, I would recommend to refactor the code a bit and take a look to Promise's features, you can also check bluebird library to have a more functionalities over Promises.

The core problem is asynchrony: you are calling all Promises secuentially, even if they try to wait one second, they are in the event queue at once (let's say). What you have to do is wait one second after processing each user.

• Let's say you have an array of users: users
• Let's also say that you have a function that given one user, it fetchs from google maps and returns the response (in fact, a Promise that resolves to that response): fetchUserLocation

Using solo Promises you could try a recursive strategy

// The only responsible to know **what** to do with each user
function processUser(user) {
  console.log(user.street, user.city);

  // then method returns a new Promise that is resolved when the passed function is called, also it resolves to the value returned
  return fetchUserLocation.then(function(location) {
    console.log(location);
    // do someStuff after you have fetched the data
  });
}

// The only responsible to know **when** to do stuff with an user
function controlFetchAllUsers(users, index, delayMills) {
  var currentUser = users[index];
  var hasNextUser = index < users.length - 1;

  processUser(currentUser).then(function() {
    // Once we processed the user, we can now wait one second
    // If there is another user
    if (hasNextUser) {
      setTimeout(
        function() {
          controlFetchAllUsers(users, index + 1, delayMills)
        },
        delayMills
      );
    }
  });
}

controlFetchAllUsers(users, 0, 1000);

Some considerations, you may want to know when all the process finished, for this you could use a Promise that wraps the whole process and maybe use the deferred pattern.

On the other hand, for this kind of problems I sincerely recommend RxJS (yet, any reactive programming library may be really useful)

Have luck!