search

danialfarid / ng-file-upload

Lightweight Angular directive to upload files with optional FileAPI shim for cross browser support
MIT License
7.87k stars 1.6k forks source link

"Possibly unhandled rejection" if using Angular 1.6+ #2019

Open reesemartin opened 6 years ago

reesemartin commented 6 years ago

package version 12.2.13 Angular version 1.6.4

This package is affected by the "Possibly unhandled rejection" error notification added in Angular 1.6 used to prompt developers to catch promise rejections. The rejections in this package are not being caught.

The area that brought this to my attention was the upload.dataUrl function when it rejects the deferred promise after URL.createObjectURL(file); fails. This is on line 998 of the dist file seen here: https://github.com/danialfarid/ng-file-upload/blob/master/dist/ng-file-upload.js#L998

The immediate fix for my app using this package was to comment the rejection our since it wasnt being handled anyway. Obviously this is not a recommended "fix" but it stops my console from filling up.

The fix is to add a catch to all promises or just do catch(angular.noop) if you dont care about what happens when it errors.

I notice 31 rejections in this file. Many of them are likely the same promise so catching where they are used should resolve them.

More information on the Angular 1.6 change pertaining to promises can be found here: http://www.codelord.net/2017/08/20/angular-1-dot-6-s-possibly-unhandled-rejection-errors/

reesemartin commented 6 years ago

1853 is referring to the same issue but in a specific spot. This would be an issue anywhere that is using a promise without a catch.

reesemartin commented 6 years ago

I identified exactly what was using the promise but not handling it. It is these two lines: https://github.com/danialfarid/ng-file-upload/blob/master/dist/ng-file-upload.js#L1047 https://github.com/danialfarid/ng-file-upload/blob/master/dist/ng-file-upload.js#L1049

The the "correct" fix is to change this section to:

      if (disallowObjectUrl) {
        p = file.$$ngfDataUrlPromise = deferred.promise.catch(angular.noop);
      } else {
        p = file.$$ngfBlobUrlPromise = deferred.promise.catch(angular.noop);
      }

or actually do something in the catch to let them know it failed even if you just console.error that it failed.

I will submit a PR of what I think fixes it. I think I see where you update this in the src