Angular - Async method doesn't return a rejected promise

Question

I'm using an async method in component for file upload. Like this:

//component
uploadPhotos = async (event: Event) => {
    const upload = await this.uploadService.uploadPhotos(event, this.files, this.urls);
}

UploadService returns a promise with the updated files and path files after upload is called. The service is working as expected for a promise that reaches resolve() without any setbacks. However, if reject() is called, the code will keep going until it reaches the resolve() inside reader.onload().

// service
uploadPhotos(event: Event, oldFiles: File[], oldUrls: string[]): Promise<{files: File[], urls: string[]}> {
     return new Promise((resolve, reject) => {
          const files = (event.target as HTMLInputElement).files;

          if ((files.length + oldFiles.length) > 5) {
               this.alertService.error('Número máximo de fotos permitidos é 5.');
               reject();
               // there is an error, so it reaches here first
          }
      
          for (let i = 0; i < files.length; i++) {
              const exists = oldFiles.findIndex(file => file.name === files[i].name);
              if (exists === -1) {
                   if (files[i].type === 'image/png' || files[i].type === 'image/jpeg') {
                      oldFiles.push(files[i]);

                      const reader = new FileReader();
                      reader.onerror = (error: any) => {
                          this.alertService.error(`Erro ao carregar a imagem: ${error}`);
                          reject();
                      };
                      reader.readAsDataURL(files[i]);
                      reader.onload = () => {
                      // it reaches here after reject()
                          oldUrls.push(reader.result);
                          if (i === files.length - 1) { resolve({ files: oldFiles, urls: oldUrls }); }
                 };
            } else {
                this.alertService.error('Formato inválido. Somente imagens do formato Png, Jpeg e Jpg são permitidos.');
                reject();
            }
       }
  }
});
}

Is there a way to avoid that reader.onload() block if reject() is reached before resolve()?

Hrm, so the load handler is called even if there's an error, is that it? (sounds a bit odd) If so, there's a solution, but just for clarity's sake, can you confirm that? — CertainPerformance
– CertainPerformance, Commented Oct 23, 2018 at 5:16
The load listener's if (i === files.length - 1) { test is weird (it will only ever trigger on files[files.length - 1] regardless of the order in which they actually get processed, I'm doubtful that the Promise always resolves with all urls? — CertainPerformance
– CertainPerformance, Commented Oct 23, 2018 at 5:33
It seems to work so far... im trying to return resolve after last element is added. However if something goes wrong between the process I would like to abort mission. — James
– James, Commented Oct 23, 2018 at 5:38
so, when you reject you don't want any further code to run? reject isn't like return it doesn't end function execution ... try return reject(...) — Jaromanda X
– Jaromanda X, Commented Oct 23, 2018 at 5:38

Bergi · Accepted Answer · 2018-10-23 16:31:51Z

1

Yes, reject() and resolve() are plain (callback) function calls. They have nothing to do with control flow, they don't stop your function. Use return or if/else for that.

Also, you have a loop inside your new Promise. If an error in one file reader happens, it has no effects on the other file readers. I recommend promisification on the innermost level instead. Then write

readFile(file: File): Promise<string> {
    return new Promise((resolve, reject) => {
        const reader = new FileReader();
        reader.onerror = (error: any) => reject(error);
        reader.onload = () => resolve(reader.result);
        reader.readAsDataURL(file);
    });
}
// service
async uploadPhotos(event: Event, oldFiles: File[], oldUrls: string[]): Promise<{files: File[], urls: string[]}> {
    try {
        const files = (event.target as HTMLInputElement).files;
        if ((files.length + oldFiles.length) > 5) {
            throw new Error('Número máximo de fotos permitidos é 5.');
        }
        const newFiles = Array.from(files).filter(newFile =>
            !oldFiles.some(oldFile => oldFile.name === newFile.name)
        );
        if (!newFiles.every(file => file.type === 'image/png' || file.type === 'image/jpeg' || file.type === 'image/jpg')) {
            throw new Error('Formato inválido. Somente imagens do formato Png, Jpeg e Jpg são permitidos.');
        }
        const newUrls = await Promise.all(newFiles.map(file =>
            this.readFile(file).catch(error => {
                throw new Error(`Erro ao carregar a imagem: ${error}`);
            })
        ));
        return {
            files: oldFiles.concat(newFiles),
            urls: oldUrls.concat(newUrls)
        };
    } catch(err) {
        this.alertService.error(err.message);
        throw new Error("something went wrong during file upload");
    }
}

edited Oct 23, 2018 at 16:31

answered Oct 23, 2018 at 7:15

Bergi

671k162 gold badges1k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

James Over a year ago

for removing files it should work like this?

deleteImage(file: File, url: string) { this.files.splice(this.files.indexOf(file)); this.urls.splice(this.urls.indexOf(url)); }

. Let's say my array has 5 elements... when splicing just this.urls[2]..... this.urls[3] and this.urls[4] got removed as well... can you help me?

Bergi Over a year ago

@James splice takes the index and the number of elements to be deleted - so ….splice(….indexOf(…), 1) should do it. But make sure indexOf does actually find the element.

CertainPerformance · Accepted Answer · 2018-10-23 05:25:42Z

0

You can add the load listener, then remove it in the error handler to ensure that the load listener doesn't trigger:

reader.onerror = (error: any) => {
  this.alertService.error(`Erro ao carregar a imagem: ${error}`);
  reader.removeEventListener('load', loadHandler);
  reject();

};
reader.readAsDataURL(files[i]);
const loadHandler = () => {
  oldUrls.push(reader.result);
  if (i === files.length - 1) {
    resolve({ files: oldFiles, urls: oldUrls });
  }
};
reader.addEventListener('load', loadHandler);

answered Oct 23, 2018 at 5:25

CertainPerformance

373k55 gold badges354 silver badges359 bronze badges

Collectives™ on Stack Overflow

Angular - Async method doesn't return a rejected promise

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related