Promise.all does not await for then

Question

First of all I am a newbie in nodejs, please be indulgent :) I have a problem with some async functions. In my case, a first API call is made and then a sub call is made on every item returned. this is my code after the first call is made :

const countryStatesCalls: Promise<any>[] = [];
countries.forEach((country, index) => {
  countries[index] = convertToCountry(country);
  const countryStatesCall = (countryId: number) => {
    return new Promise((resolve, reject) => {
      const countryStatesList = getStateCountries(countryId);
      if (countryStatesList) {
        if (Object.prototype.toString.call(countryStatesList) !== '[object Array]') {
          resolve([]);
        } else {
          resolve(countryStatesList);
        }
      } else {
        reject([]);
      }
    });
  };
  countryStatesCalls.push(
    countryStatesCall(countries[index].id).then(countryStates => {
      countries[index].states = countryStates;
    }).catch(countryStatesType => {
      countries[index].states = countryStatesType;
    })
  );
});

await Promise.all(countryStatesCalls).then(_ => {
  res.json(countries)
});

the following code

res.json(countries)

is supposed to be executed after all promises execution has been finished.

EDIT : the getStateCountries async function :

const getStateCountries = async (countryId: number, lang?: string) => {
const listState = await odoo.searchRead(
  'res.country.state',
  {
    filters: [
      {
        field: 'country_id',
        operator: '=',
        value: countryId,
      },
    ],
    fields: fieldState,
  },
  { lang }
);

if (!listState.length) {
  return [];
} else {
  listState.forEach((state, index) => {
      listState[index] = convertToState(state);
  });
  return listState;
}

};

the main function is an express controller

I think my mistake may be obvious but can you help me please, I read many topics and I cannot see what am I doing wrong.

Thank you in advance

[SOLUTION]

As pointed out by comments, I was doing it the wrong way around, my call on promise called another promise: the solution was to write :

        const countryStatesCalls: Promise<any>[] = [];
        for (let index = 0; index < countries.length; index++) {
          countries[index] = convertToCountry(countries[index]);
          if (withSates) {
            countryStatesCalls.push(
              getStateCountries(countries[index].id)
            );
          } else {
            countries[index].states = [];
          }
        }

        if (withSates) {
          await Promise.all(countryStatesCalls).then((countriesStates) => {
            countries.forEach((country, index) => {
              country.states = [];
              for (const countryStates of countriesStates) {
                if (countryStates.length
                  && countryStates[0].country_id.code === country.id) {
                  countries[index].states = countryStates;
                }
              }
            });
            res.json(countries);
          });
        } else {
          res.json(countries);
        }

thank you everyone

`Promise.all()` has [fail-fast behavior](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all#promise.all_fail-fast_behavior) by default. — Sean, Aug 06 '21 at 15:39
you are using 'countries' incorrectly in your `Promise.all`, see here - https://stackoverflow.com/questions/23667086/why-is-my-variable-unaltered-after-i-modify-it-inside-of-a-function-asynchron Generally speaking you want to get your `countries` from the `then` callback of the `Promise.all` at the minute you are ignoring the value (denoted by `_`) — andy mccullough, Aug 06 '21 at 15:40
I see no `res` variable in this code, is this inside an express middleware? — MinusFour, Aug 06 '21 at 15:44
@MinusFour This is presumably the body of a controller function where `res` is its parameter. — Barmar, Aug 06 '21 at 15:46
Why are you using Promises in the first place? There are no asynchronous operations in the promise. If `getStateCountries` is asynchronous, you need to use `.then()` there. — Barmar, Aug 06 '21 at 15:48
@Barmar seems like it might be both (returns either an array or a promise). It's probably better to pass it to `Promise.resolve` to normalize it but it's not exactly the same logic as here. — MinusFour, Aug 06 '21 at 15:52
@MinusFour The `if` test after that seems to expect it to be an array, not a promise. — Barmar, Aug 06 '21 at 15:53
BTW, a better way to test if something is an array is with `if (Array.isArray(countryStatesList))` — Barmar, Aug 06 '21 at 15:53
thank you for your help, I will refactor with all your advice, I will edit the question with the ```getStateCountries``` function — Xav GM, Aug 06 '21 at 16:01
You are missing `await` on the `getStateCountries` call. That function doesn't seem to return anything other than a promise that resolves to an array so there's no need for the promise wrapping here. — MinusFour, Aug 06 '21 at 16:08
in fact @MinusFour, I figured it out this week-end, thank you again to all for your help — Xav GM, Aug 09 '21 at 08:53

score 1 · Accepted Answer · answered Aug 06 '21 at 16:54

I can't exactly tell why it doesn't work, but this looks a bit like the Promise constructor antipattern. The getStateCountries function is async and already creates a promise, so you need to await it or chain .then() to it. The new Promise is not necessary.

Also I'd recommend to avoid forEach+push, rather just use map, and get rid of the countryStatesCall helper:

const countryStatesCalls: Promise<any>[] = countries.map((country, index) => {
  countries[index] = convertToCountry(country);
  return getStateCountries(countries[index].id).then(countryStatesList => {
    if (countryStatesList) {
      if (Object.prototype.toString.call(countryStatesList) !== '[object Array]') {
        return [];
      } else {
        return countryStatesList;
      }
    } else {
      throw new Error("getStateCountries() resolved to nothing");
    }
  }).then(countryStates => {
    countries[index].states = countryStates;
  });
});

await Promise.all(countryStatesCalls).then(_ => {
  res.json(countries)
});

Promise.all does not await for then

1 Answers1