finally() handler on promise generates Possibly unhandled rejection

[Rouche]

What is the current behavior?
The promise have correct success/error handler but if i add a finally() handler, it triggers the error.

Plunk: http://plnkr.co/edit/eT834BkIEooAMvrVcLDe

Edit: Looks like finally recreates a new Promise() in handleCallback -> resolver and reject this promise,
I was surprised that it returns something already taken care of in the then(), why not just returning the original Promise instead of rejecting a new one?
This means every finally will need to be like this? : finally().then(angular.noop, angular.noop);

It looks weird, not sure how to deal with this situation perfectly.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Chrome / Windows 10
jQuery 1.3.1
Angular 1.6.1

Other information (e.g. stacktraces, related issues, suggestions how to fix)

There is another processQueue() called with a pending promise. Im guessing its the one in finally() and this promise's pur value stay undefined. Wich is the property tested in processChecks()

Strange, because the finally's then() do have success/error

[gkalpak]

This is expected behavior. As documented, .finally() (as well as then/catch) is creating and returning a new promise. Quoting Q's docs (which are linked to from $q's docs):

finally returns a promise, which will become resolved with the same fulfillment value or rejection reason as promise. However, if callback returns a promise, the resolution of the returned promise will be delayed until the promise returned from callback is finished. Furthermore, if the returned promise rejects, that rejection will be passed down the chain instead of the previous result.

The way you use it, you end up with two distinct (unchained) promises, so you need to handle them both:

var deferred = $q.defer();
var rootPromise = deferred.promise;
var p1 = rootPromise.then(onFulfilled, onRejected);   // Creates a new promise (!== rootPromise)
var p2 = rootPromise.finally(finallyCallback);        // Creates a new promise (!== rootPromise)

If it where a tree, it would look like this:

rootPromise
    /\
   /  \
 p1    p2

Now, deferred.reject('Error') will start at the root of the tree (rootPromise):

1. rootPromise does not have an onRejected handler, so the rejection will propagate down the tree (to both branches).
2. p1 does have an onRejected handler, so the rejection will be handled and you won't see a "Possibly Unhandled Rejection" for that branch (unless the onRejected handler returns a rejection too).
3. p2 does not have an onRejected handler, so finallyCallback will be executed and then you will get a "Possibly Unhandled Rejection".

The easiest way to avoid this (and not have to add onRejected handler after every .finally()) is to put the .finally() call first and then chain the onRejected handler:

var deferred = $q.defer();
var rootPromise = deferred.promise;
var p1 = rootPromise.finally(finallyCallback);
var p2 = p1.then(onFulfilled, onRejected);

// or simply:
// deferred.promise.
//   finally(finallyCallback).
//   then(onFulfilled, onRejected);

Now the tree would look like this:

rootPromise
    /
  p1
  /
p2

The rejection would propagate through rootPromise and p1 (which don't have an onRejected handler) and reach p2, which does handle it (so no "Possibly Unhandled Rejection" for this branch - which happens to be the only branch).

Closing, since everything works as expected.

[Rouche]

Thanks a lot for the idea.

That solution is perfect!

[sebastian-zarzycki-apzumi]

I got a little bit confused by the order in the last example. Couldn't it be simply done by:

var deferred = $q.defer();
var rootPromise = deferred.promise;
rootPromise.then(onFulfilled, onRejected).finally(finallyCallback);

to achieve the same result? If not, why?

[Rouche]

No, and sadly, i cannot explain it better than @gkalpak

finally creates a new promise, that you need to catch. Use his solution so they are chained in finaly's then()

[sebastian-zarzycki-apzumi]

Ah, I think I get it. The most sensible way in this case would be (I hope?):
rootPromise.then(onFulfilled).finally(finallyCallback).catch(onRejected);

[Rouche]

rootPromise.finally(onFinally).then(onFulfilled, onRejected);

Simple! I updated my plunk, you can fork it and have fun trying every combinations you want!

[sebastian-zarzycki-apzumi]

Unless I'm missing something (not sure what did you wanted to achieve in the first place), both my examples would work (no exception thrown, tested it), as they would create a single "branch" of promises and each branch has a rejection handler defined in them. The placement of finally just decides when exactly do you want to react - either directly after the rootPromise or later.

[gkalpak]

@sebastian-zarzycki-es: There are subtle differences (e.g. in the order/timing of execution or the returned value in case finallyCallback rejects).

[Rouche]

in case finallyCallback rejects

@gkalpak You just helped me solve another problem i had. Good coincidence!

@sebastian-zarzycki-es I understand. And in fact i'm now in a dilemma on whats the perfect solution between yours and the other. I guess its a coin flip.