$http suppressing syntax errors

[SamanthaAdrichem]

I'm submitting a ...

• bug report
• feature request
• other (Please do not submit support requests here (see above))

Current behavior:

If a javascript syntax error occurs in a promise.then chain, the promise is now rejected where the error is send as response to the next chain's reject function. This causes our main javascript errorHandler to not trigger, nor to have the error logged in the console. Whilst the code does stop working.

Expected / new behavior:

It used to work in angular 1.5.5, it no longer does in 1.6.2 -> 1.6.4. Syntax error's / programming errors should not just be caught in a promise and then reject the promise. They should always be fired as the syntax error they are.

Minimal reproduction of the problem with instructions:

angular 1.5.5 working:
https://plnkr.co/edit/fi32lCTkH7U8zgYQQWLN

angular 1.6.3 no longer working (1.6.4 isn't CDN available yet)
https://plnkr.co/edit/zoYtfbCfElr2MKmMP7RG

Setup angular version 1.6.4, start a $http call to an API with a JSON response, use the following successFunction

function processResponse( response ) {
    response.nonExistingVar.subVariable = 'test'; 
}

It should result in : Uncaught TypeError: Cannot set property 'subVariable' of undefined, instead your code is silent.

Add a second .then with the following reject function (or a catch);

function errorFunction( response ) {
    console.log( response );
}

Angular version: 1.6.4

Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

Alteast in Chrome 57.0.2987.98 (64-bit), but i think in all browsers

Anything else:

It seems to relate to this issue: #15855

[chriscodeweb]

We experience the same as described. It's now almost impossible to find your bug, because you don't get the default javascript error in your browser.
Please fix this!

[frederikprijck]

I guess this is related to this commit: e13eeab which landed as of 1.6.0.RC0.

This looks like an intended breaking change, it's also mentioned on the changelog as a breaking change: https://github.com/angular/angular.js/blob/master/CHANGELOG.md

Previously, throwing an error from a promise's onFulfilled or onRejection handlers, would result in passing the error to the $exceptionHandler() (in addition to rejecting the promise with the error as reason).

Now, a thrown error is treated exactly the same as a regular rejection. This applies to all services/controllers/filters etc that rely on $q (including built-in services, such as $http and $route). For example, $http's transformRequest/Response functions or a route's redirectTo function as well as functions specified in a route's resolve object, will no longer result in a call to $exceptionHandler() if they throw an error. Other than that, everything will continue to behave in the same way; i.e. the promises will be rejected, route transition will be cancelled, $routeChangeError events will be broadcasted etc..

This looks alot like how ES6 Promises are handled: https://jsfiddle.net/jv711rwt/1/
Looking at the above fiddle shows that no errors are logged in the console, instead the error is treated as a regular rejection.

[chriscodeweb]

Thanks Frederickprijck! Do you know how to turn this off temporary for debug purpuse?
As described in the note comment it says:
Unless explicitly turned off, possibly unhandled rejections will still be caught and passed to the
$exceptionHandler()

But how do you know how to turn it off?

[frederikprijck]

The commit states:

Unless explicitly turned off, possibly unhandled rejections will still be caught and passed to the
$exceptionHandler(), so errors thrown due to programming errors and not otherwise handled (with a
subsequent onRejected handler) will not go unnoticed.

In the plunkr provided for this issue, there is a handler to handle promise rejections (so it has a handler, hence it won't be logged). Removing this secondReject handler results in the error in the console, as expected:

https://plnkr.co/edit/KSRSPexlWjsVpO0x8JUp?p=preview

Basicly all I did was change

function click() {
    console.log('clicked');
    $http.get('response.json')
      .then( successWithSyntaxError, firstReject )
      .then( secondSuccess, secondReject );
  }

into

function click() {
    console.log('clicked');
    $http.get('response.json')
      .then( successWithSyntaxError, firstReject )
      .then( secondSuccess );
  }

[SamanthaAdrichem]

@frederikprijck wow you're right.

In our case we had response data that always contained a valid category ID. Then after 1.5 years there was one entry that did not and it broke the code.. took us 30+ mins to figure out what the reason was.

I'll look into why we don't get the errors since we do not have the second then in our code, i only added it to the plunkr, but can see removing it there indeed results into the syntax error.

[frederikprijck]

Imho a syntax error should always remain a syntax error and then be caught by an errorHandler. This new solution forces you to add a catch to every single promise you'll ever do :(

@SamanthaAdrichem I guess we don't get to decide how Promises in ES6 handle errors. But aligning $q with them doesn't sound like a bad idea.

I don't think you will need to have a try/catch in every promise. Whenever a promise throws an exception, which you do not need to handle locally (so no try/catch) results in a rejection of the promise with the reason of rejection (the error) passed as a callback. The error is not lost, it's just not logged to the console if you have a error handler (which you'll probably disable for production anyway).

So if you want them to log to the console, don't handle the promise rejection (or rethrow after handling the rejection could work I guess)

[SamanthaAdrichem]

Wow figured it out. Because we use angular-ui modals, which always have a 'close' promise, eventough we don't use those.. All the modals started giving errors "Possibly unhandled rejection". Since it wasn't a requirement, since there never was a rejection, only in the live environment (to prevent a ton of not-really errors) we turned errorOnUnhandledRejections off. Then you no longer get the original syntax errors eather.. lol

see this: https://plnkr.co/edit/UKz8M7CjNEx7Edg2zPfQ

[frederikprijck]

This looks like it works as designed because you have a promise, which throws an exception which is not handled by any handler. You're also telling angularjs to not throw an error when any promise rejection was unhandled. So yep, you will not be notified on the error in your last plunkr, but you explicitly told angularjs so that looks as expected.

[SamanthaAdrichem]

Yes you are right, it does work as designed. :)

I added the comment if other people run into this due to the same 'fix' (which you should not implement, just add a .then to all the modals ;) ;))