Promise.all() never resolving

Question

I have a function which should get some data asynchronously.

  getPageEvents() {
    const promises = this.calendarPage.days.map(day => {
      return this.dataService.getEventsByDay(day)
      .then(events => {
        if(events){
          day.dailyEvents = events;
        }
      });
    });
    
    Promise.all(promises)
    .then((res) =>{
      console.log(res)
    })
    .catch(error =>{
      console.error(error)
    })
  }

The getEventsByDay() function returns data from a firestore database:

  async getEventsByDay(day: Day): Promise<Event[] | undefined>{
    return new Promise<Event[] | undefined>((resolve) =>{
      const date: string = day.dateFormatted
      let dailyEvents: Event[] = []
      this.getEvents()
      .then(events =>{
        if(events){
          events.forEach(event =>{
            if(event.durationDates?.includes(date)){
              dailyEvents.push(event)
            }
          })
          if(dailyEvents.length > 0){
            resolve(dailyEvents)
          }
        }else{
          resolve(undefined)
        }
      })
    })
  }

The problem is that the code inside the then() block of Promise.all() never runs, even though I can see (on the UI) the data beeing fetched. Here is my getEvents() function:

  async getEvents(): Promise<Event[]> {
    return new Promise<Event[]>((resolve, reject) => {
      this.getTeams()
        .then(teams => {
          const teamList: string[] = teams.map(team => team.id);
          if (teamList) {
            const promises: Promise<Event[]>[] = [];
            teamList.forEach(team => {
              promises.push(this.getEventsByTeam(team));
            })
            // Quando ho tutti gli eventi di tutti i teams
            Promise.all(promises)
              .then(teamEvents => {
                const events: Event[] = [];
                teamEvents.forEach(eventsArray => {
                  eventsArray.forEach(event => {
                    const eventList = events.map(e => e.id)
                    if(!eventList.includes(event.id)){
                      events.push(event);
                    }
                  });
                });
                resolve(events);
              })
              .catch(error => {
                reject(error);
              });
          } else {
            reject("No teams");
          }
        })
        .catch(error => {
          reject(error);
        });
    });
  }

Any guesses on what is wrong? By the way I don't know if this nesting tecnique for Promises is good. It's still a quite difficult topic to handle for me so far.

Answer 1

The problem is that you create a promise in getEventsByDay for which there is a possibility that it does not get settled. This happens when the condition in the following statement is false:

if(dailyEvents.length > 0){

There is no else case for this if in your code, so that the promise is left pending.

The quick fix is to think of some value to resolve this promise with when dailyEvents turns out to be empty and call resolve with it in an else block.

Not your question, but your code is applying the promise constructor antipattern. In short this means you should not do new Promise() when you already have a promise to work with.

Another thing to refactor is that you use async functions but without benefiting from the await operator. If you would use it, the nesting of your code could be flattened and become more readable.

Finally, try to avoid push, and instead use the power of map() wherever you can, and also flat().