Javascript: How to wait for multiple promises to resolve within a loop before moving forward

Question

I saw similarly asked questions but I am having a hard time applying it to my code.

I have the following piece of code:

  createSubmission(sub: Submission, files: Files[]): Promise {

    //first add files to storage and create the url array
    let urlArray: string[];
    return files.forEach(file => {
      var storageRef = this.storage.ref(`files/${file.name}`);
      return storageRef.put(file).then(()=> {storageRef.getDownloadURL().subscribe(url => {
        urlArray.push(url);
        if(urlArray.length === files.length){ //we are at the final element
          sub.filesUrls = urlArray;
          return this.finilize(sub);
        }
      })
    });
    });
  }

  finilize(sub: Submission){ //actually creates the submission
    
      let subRef = this.db.database.ref(`/submissions/${sub.medicUid}`).push(); 
      let subKey = subRef.getKey();
      sub.uid = subKey; 
      return subRef.update(sub);
  }

The caller of the first method (createSubmission) should receive a Promise. My code sequence is finilized when we return from the finilize method.

I want to continue after all the elements in the loop have been iterated, and the urlArray has been filled. It is filled when it has the same number of elements as the passed files array, that's why I placed this condition:

if(urlArray.length === files.length){...}

However, I get an error because the if condition means that Not all code paths return a value. So I believe I should ditch the if condition and wait for all promises to resolve before calling finalize, but I am not very clear on how to do it.

score 0 · Answer 1 · answered Oct 21 '21 at 06:10

First of all foreach loop does not return anything, instead, it modifies the existing array.

Now come to the solution

you cannot use forEach when you want your code to wait for some async event. Instead, use a for of loop, and with that use await

async createSubmission(sub: Submission, files: Files[]): Promise {

    //first add files to storage and create the url array
    let urlArray: string[];
    for (const file of files) {
        const url = await this.getDownloadURL(file)
        urlArray.push(url)
    }
    sub.filesUrls = urlArray
    return this.finilize(sub)    
}

getDownloadURL(file):Promise{
    let storageRef = this.storage.ref(`files/${file.name}`);
    return new Promise((resolve, reject) => {
        storageRef.put(file).then(() => {
            storageRef.getDownloadURL().subscribe(url => {
                resolve(url)
            })
        })
    })
}

finilize(sub: Submission){ //actually creates the submission

    let subRef = this.db.database.ref(`/submissions/${sub.medicUid}`).push();
    let subKey = subRef.getKey();
    sub.uid = subKey;
    return subRef.update(sub);
}

Note: Code snippet may contain syntactical errors

The `new Promise` in your `getDownloadURL` is an anti-pattern due to the Promis chain created within it. And the reject case is not handled correctly. — t.niese, Oct 21 '21 at 06:17
Code snippet is just to give an idea and not the actual code which can be used. Can you tell me more about why it is anti-pattern? Wanted to know more. And thanks for mentioning it. — arpanexe, Oct 21 '21 at 06:21
The problem is that you can easily forget certain edge cases. Which might result in a broken promise chain, code that does not finish, or other unexpected results. In your code, you don't consider the case that `storageRef.put(file)` or `storageRef.getDownloadURL()` resulting in an error. See [What is the explicit promise construction antipattern and how do I avoid it?](https://stackoverflow.com/questions/23803743) — t.niese, Oct 21 '21 at 07:46

t.niese · Answer 2 · 2021-10-21T07:49:47.383

I don't know what storageRef.getDownloadURL() is from (I assume it is related to Firebase and Angular) so I leave most of your code untouched assuming that it is correct and that you just want to know how to correctly deal with the Promise case.

There are two ways of solving the problem.

Processing the files sequentially using await/async

async createSubmission(sub: Submission, files: Files[]): Promise {

  //first add files to storage and create the url array
  let urlArray: string[];
  
  for( let file of files ) {
    const storageRef = this.storage.ref(`files/${file.name}`);
    await storageRef.put(file)
    const url = await new Promise((resolve, reject) => {
      try {
         // pass resolve to subscribe. The subscribe will then call resolve with the url
         storageRef.getDownloadURL().subscribe(resolve)
         // you need to check if the callback to passed to subscribe
         // is guaranteed to be called, otherwise you need to
         // handle that case otherwise your code flow will 
         // stuck at this situation
      } catch (err) {
        reject(err)
      }
    })
    urlArray.push(url)
  }
  
  sub.filesUrls = urlArray;
  
  return this.finilize(sub);
}

finilize(sub: Submission) { //actually creates the submission

  let subRef = this.db.database.ref(`/submissions/${sub.medicUid}`).push();
  let subKey = subRef.getKey();
  sub.uid = subKey;
  return subRef.update(sub);
}

If storageRef.put(file) does some network stuff and you want to parallelize that (which is not always a good idea), you can use Promise.all and Array.map.

async createSubmission(sub: Submission, files: Files[]): Promise {

  //first add files to storage and create the url array
  let urlArray: string[];

  urlArray = Promise.all(files.map(async(file) => {
    const storageRef = this.storage.ref(`files/${file.name}`);
    await storageRef.put(file)
    const url = await new Promise((resolve, reject) => {
      try {
        // pass resolve to subscribe. The subscribe will then call resolve with the url
        storageRef.getDownloadURL().subscribe(resolve)
        // you need to check if the callback to passed to subscribe
        // is guaranteed to be called, otherwise you need to
        // handle that case otherwise your code flow will 
        // stuck at this situation
      } catch (err) {
        reject(err)
      }
    })
    
    return url
  }))



  sub.filesUrls = urlArray;

  return this.finilize(sub);
}

finilize(sub: Submission) { //actually creates the submission

  let subRef = this.db.database.ref(`/submissions/${sub.medicUid}`).push();
  let subKey = subRef.getKey();
  sub.uid = subKey;
  return subRef.update(sub);
}

As you use TypeScript you should add the corresponding type information to my code.

YOu further should check if storageRef.getDownloadURL() provides a Promise API and if so replace new Promise and the subscribe with that one.

score 0 · Answer 3 · answered Oct 21 '21 at 08:03

I think what you're looking for is "Promise.all()"

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

Promise.all resolves an array of Promises.

In this example urlArray will contain the resolved values of getDownloadUrl


    let urlArrayTasks = [];

    for(let file of files){
      urlArrayTasks.push(this.getDownloadURL(file));
    }

    let urlArray = await Promise.all(urlArrayTasks);

Leo Letto · Answer 4 · 2021-10-21T08:14:53.370

For that, you can simple use Promise.all(), which allows you to await for multiple promises to complete, and have their results in an array.

async createSubmission(sub: Submission, files: Files[]): Promise {
    // Creates an array of promises
    let urlArrayPromises = files.map(f => this.getDownloadURL(file));

    // await for all the promises to be solved
    let urlArray = await Promise.all(urlArrayPromises);

    sub.filesUrls = urlArray;
    return this.finilize(sub);  
}


async getDownloadURL(file) {
    let storageRef = this.storage.ref(`files/${file.name}`);
    // `put` already returns a promise, you can simple await for it
    // there's no need for creating unnecessary callbacks
    await storageRef.put(file);

    // returns a promise that will be resolved with the url
    return storageRef.getDownloadURL();
}

// Creates the submission
finilize(sub: Submission) {
    let subRef = this.db.database.ref(`/submissions/${sub.medicUid}`).push();
    let subKey = subRef.getKey();
    sub.uid = subKey;
    return subRef.update(sub);
}

You should avoid using the Promise only for returning a new promise, if the functions you are going to call inside that promise already returns a promise.

Make sure that you call your method createSubmission inside a try/catch block in order to handle errors correctly.

Assuming that you are using firebase (judging by the code), the official documentation says that getDownloadURL returns a Promise, and you are using subscribe instead of then to get the result of this method, but you are never closing that subscription, be careful with memory leaks on your application.

Javascript: How to wait for multiple promises to resolve within a loop before moving forward

4 Answers4