joostvunderink / angular-jsonrpc-client

A JSON-RPC 2.0 client for AngularJS
MIT License
26 stars 20 forks source link

TypeError if server returns 500 (using wrong 'Access-Control-Allow-Origin' header) #16

Closed Harper04 closed 8 years ago

Harper04 commented 8 years ago

image

Instead of crashing it should check for data === null and reject the promise so we can handle the error afterwards.

joostvunderink commented 8 years ago

Thank you for your bug report. The screenshot is very useful. I'll have a look to see if it's easy to reproduce this, to make sure the fix actually fixes the bug.

joostvunderink commented 8 years ago

When I try to reproduce this bug, by removing the Access-Control-Allow-Origin header from the server's response, the code ends up in promise.error on line 198. In your browser, the code ends up in promise.success, even though there was a 500 HTTP status code. I'm not sure what's causing the difference.

The variable data is null in your case. Could you tell me what the values are of status, headers, and config (in the callback of promise.success) so I can make a meaningful error message in case data is null?

Harper04 commented 8 years ago

I try to reproduce it later on my side (fixed my api usage 10 days ago )

I think the point was that there was no real request made. Just the OPTIONS preflight which ended up in a 500 and spitting the No Acces-Control-Allow-Origin message.

status headers could therefore also be null. I try to check that later

Harper04 commented 8 years ago

image

ben-8409 commented 8 years ago

I am affected by the same issues, seems to be caused by a OPTIONS request which returns net::ERR_CONNECTION_REFUSED (i deliberately blocked to connection for testing).

Then, I get the same Error: TypeError: Cannot read property 'result' of null

With the same values: status: -1 headers: function (d) data: null

Could this be an error in angulars $http? Because @joostvunderink is right, that actually the promise.error callback should be called instead of promise.success. Maybe this is related to interceptors (see http://stackoverflow.com/questions/31229019/err-connection-refused-calls-my-http-sucess-function for reference)?

Also note:

A response status code between 200 and 299 is considered a success status and will result in the success callback being called. Any response status code outside of that range is considered an error status and will result in the error callback being called. Also, status codes less than -1 are normalized to zero. -1 usually means the request was aborted, e.g. using a config.timeout

from https://docs.angularjs.org/api/ng/service/$http

ben-8409 commented 8 years ago

It seems we actually caused the error ourselves. We used a $http interceptor for authentification (as described for example in [1]) but made the same small but fatal error as described in this stackoverflow question [2]: in some cases, on responseError we did return rejection instead of return $q.reject(rejection).

With this fix applied in our code, everything works as expected. 1: http://arthur.gonigberg.com/2013/06/29/angularjs-role-based-auth/ 2: http://stackoverflow.com/questions/31229019/err-connection-refused-calls-my-http-sucess-function

joostvunderink commented 8 years ago

I've just released version 0.5.2 with a workaround for this situation. If other people encounter this problem, they will get a rejected promise instead of an exception. The message of the rejection mentions this issue. Thank you @Harper04 for reporting this and @ben-8409 for the additional analysis!