Chained promises broken when testing due to $exceptionHandler interference

[Thorn1089]

The Promises specification has the following to say about chaining promises:

then must return a promise

promise2 = promise1.then(onFulfilled, onRejected);

If either onFulfilled or onRejected returns a value that is not a promise, promise2 must be fulfilled with that value.

If either onFulfilled or onRejected throws an exception, promise2 must be rejected with the thrown exception as the reason.

However, when using Karma to test a module, I noticed that the following inside the method under test failed:

return $http.post(endpoint, data).then(function(response) {
  //return some data from the response
}, function(error) {
  var throwable;
  //grab some data from the response to create a meaningful rejection reason
  throw throwable;
});

When invoking the method as follows, with a mocked HTTP backend to force a failure:

service.do(args).then(function(unused) {
  expect(false).toBe(true);//In the absence of a fail() method, use a contradiction
}, function(error) {
  expect(error.code).toBe(FAILURE_VAL);
});

Instead of the expectation being met in the onRejected function, the exception propagates up the stack and fails the test. This is because apparently during tests, the default policy of $exceptionHandler is rethrow.

I don't understand what business $exceptionHandler has at all being invoked by $q, especially since this behavior violates the Promises/A specification. Exceptions being thrown is a normal and expected part of promise execution.

[FrigoEU]

I'm wondering if this has been looked at in the meantime? I'm having the same problem where my code is correctly throwing an error to reject a promise and then handle it in my .catch() handler, but angular is intercepting the error and immediately propagating it upward making karma think there was an uncaught error.

[rasmusvhansen]

+1

I also find this behaviour odd. I guess a workaround is to configure the exceptionhandler to log instead of rethrow exception.

I use this in the relevant tests:

beforeEach(module(function ($exceptionHandlerProvider) {
  $exceptionHandlerProvider.mode('log'); // See https://github.com/angular/angular.js/issues/3174
}));

[Narretz]

$q is not promises/A compliant, IIRC. But it's definitely something to keep in mind for a future rewrite @caitp

[caitp]

It is actually aplus compliant, at least a few months ago it was, not sure how much aplus has changed since then =) Work is being done on refactoring the logging of exceptions (ITF: rejections in general), however @rasmusvhansen's solution is really the best you can do right now, short of monkey-patching ngMock

[n-rook]

Hey @alexeagle, how are you doing? You still like testing right? That means you should fix this bug ;)

Here's a jsfiddle demonstrating it: http://jsfiddle.net/tz8f7tvs/2/

[alexeagle]

Hi Nate, I do like testing. But I don't know anything about Angular 1. Do you want to spend an "escalation token" on this one? :)

[n-rook]

Oh, I didn't realize the teams were split, too! I saw Angular 2 just launched publicly, so congratulations on that.

As a Xoogler, I'm not sure I get escalation tokens 😳 If I do, though, I'd love to see this fixed, since I think it makes it really hard to write good tests in some cases.

[juliemr]

Escalation token noted but placed on side board, with reason: "Travel and Conferences."

I can take a look at this one in the "next month" kinda timeframe.

[n-rook]

Cool, thanks for taking a look!

[gkalpak]

This is not a bug, it is a feature :stuck_out_tongue:

The exception is indeed turned into a rejection, but at the same time passed to the exceptionHandler. So, $q is still A+ compliant. By default, the exceptionHandler will just log the error, but in tests it will rethrow it (as mentioned above).

@rasmusvhansen's solution is the way to go.

The reason why thrown exceptions are passed to the exceptionHandler (in addition to being converted to rejections), was that they might indicate a programming error, which would otherwise get possibly swallowed in an unhandled rejection.

Since this turned out to be inconvenient/confusing for people, we are planning to remove this "feature" in 1.6.0. (The timing is good, because starting with 1.6.0 "Possibly Unhandled Rejections" will be reported as errors.)

[n-rook]

Oh, okay! Thanks for getting back to me. I think this behavior is pretty crazy, but then you're already going to remove it, so no need for me to convince you.

Please document this behavior in the $q documentation! I could also do this and send a PR, but I don't have deep knowledge of Angular so I might get something wrong.

Is there a doc somewhere saying that $q is supposed to be A+ compliant? I was hunting for one but couldn't find any source one way or another. This might also be good to document.

And again, thanks for getting back :)

[gkalpak]

Is there a doc somewhere saying that $q is supposed to be A+ compliant?

@n-rook, hey, we run $q against the Promises/A+ Compliance Test Suite on every build. We are compliant (as long as $exceptionHandler doesn't do weird things, suhc as re-throwing errors 😛). I am concerned that this is a bit too low-level to add to the docs, but I have added a note that $q is Promises/A+ compliant in #15213 (which also removes the extra $exceptionHandler call).

[n-rook]

Alright, thanks @gkalpak! To be honest, I just wanted a canonical source to resolve my stackoverflow question ;) You could try to get Angular 1 listed on this page, but I have no clue how that page was set up or if anyone but me would ever have found it.

I think I recognize your username... if you see the TAP team anytime soon, say hi to them for me.

[gkalpak]

_{No idea what the TAP team is, but if I happen to see them I will say hi :smiley:}