Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(angular.js): wrong $httpBackend status code #4514

Closed
wants to merge 1 commit into from
Closed

fix(angular.js): wrong $httpBackend status code #4514

wants to merge 1 commit into from

Conversation

c0rrupt
Copy link
Contributor

@c0rrupt c0rrupt commented Oct 18, 2013

If I run html document as a file in browser (e.g. file:///index.html), then all ajax responses have a protocol "file" (and therefore response status code can be 200 or 404)

(protocol == 'file') ? (response ? 200 : 404)

locationProtocol has higher priority in completeRequest function.

It's critical in mobile hybrid applications which use angularjs (index.html run from local filesystem)

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@rtpm
Copy link

rtpm commented Oct 30, 2013

Can you please move this issue into the 1.2 milestone ? Angularjs is being used by more phonegap/cordova users every day and this issue makes angular working with remote data impossible.

@lord2800
Copy link

I'm happily using Angular 1.2rc3 on phonegap right now with no issues--I also don't use file:// urls, I use relative urls; that may make all the difference.

@rtpm
Copy link

rtpm commented Oct 31, 2013

@lord2800 I also use relative paths and the problem exists. Have you tried with $http.post() with backend returning HTTP_400 for example ?

@lord2800
Copy link

Ah, I haven't tried POSTing to myself. I can give that a try.

jerep6 added a commit to jerep6/ogi that referenced this pull request Nov 1, 2013
@damrbaby
Copy link
Contributor

damrbaby commented Nov 3, 2013

+1, thank you @c0rrupt for this fix, this is a major bug, and should be in 1.2 release.

Currently if you are using phonegap/cordova (or anything that serves your app over a file:// protocol) and $http with a backend that responds with a non-200 status code (e.g. 400-500) with a response it will be considered a 200 status code, and the success function will be called.

@ADmad
Copy link

ADmad commented Nov 9, 2013

@damrbaby Yup I am having the exact same problem you describe. In my phonegap app all responses from my backend API with non 200 status code get converted to 200.

Wondering why this patch hasn't been merged. @c0rrupt Have you signed the CLA as requested? 😄

@c0rrupt
Copy link
Contributor Author

c0rrupt commented Nov 9, 2013

@ADmad yep, CLA has been signed as requested (all checkboxes are checked in @mary-poppins bot's comment). Core devs mark this PR for 1.2.1 milestone.

@ADmad
Copy link

ADmad commented Nov 9, 2013

@c0rrupt Okay cool, let's hope it gets merged soon or at least some feedback from the devs.

@philholden
Copy link

Spent today trying to find the same problem used the following as a temporary fix:

 status = (protocol == 'file' && status === 0) ? (response ? 200 : 404) : status;

@gauthamses
Copy link

+1..

Need fix soon

@IgorMinar
Copy link
Contributor

@c0rrupt unfortunately I'm unable to verify your CLA signature. Can you please sign it again and make sure that you use the same email address as the one associated with your commit in this PR.

@@ -1,6 +1,7 @@
'use strict';

var PATH_MATCH = /^([^\?#]*)(\?([^#]*))?(#(.*))?$/,
var SERVER_MATCH = /^([^:]+):\/\/(\w+:{0,1}\w*@)?(\{?[\w\.-]*\}?)(:([0-9]+))?(\/[^\?#]*)?(\?([^#]*))?(#(.*))?$/,
PATH_MATCH = /^([^\?#]*)(\?([^#]*))?(#(.*))?$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this hunk is obsolete

@c0rrupt
Copy link
Contributor Author

c0rrupt commented Nov 26, 2013

@IgorMinar Has been updated PR, re-signed CLA

@IgorMinar
Copy link
Contributor

@c0rrupt that looks much better! Thanks

@IgorMinar IgorMinar closed this in 68ceb17 Nov 26, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…file:// apps

Previously if an app was running from file:// origin we would always return either
http 200 or 404 depending on whether the response was present.

This changes the behavior so that we do this only if the protocol of the request
(not the origin) is file:// and only if the status code is 0.

Closes angular#4436
Closes angular#4587
Closes angular#4514
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…file:// apps

Previously if an app was running from file:// origin we would always return either
http 200 or 404 depending on whether the response was present.

This changes the behavior so that we do this only if the protocol of the request
(not the origin) is file:// and only if the status code is 0.

Closes angular#4436
Closes angular#4587
Closes angular#4514
@adityamenon
Copy link

I'm using 1.2.21, but even now for HTTP 400 requests the .success() callback is being called (I'm using the $http service). Any ideas why?

@caitp
Copy link
Contributor

caitp commented Aug 3, 2014

more than likely it's one of your interceptors returning a value which is not a rejected promise, @adityamenon.

@adityamenon
Copy link

Ah, yep, I just began returning $q.reject(response) in my responseError interceptor like in this answer and it works as expected @caitp, thanks! Should I be doing the same thing in my response and request and interceptors as well?

@adityamenon
Copy link

Okay, sorry just read the relevant part of the docs that says I can directly modify the objects in case of the request and response properties. I do think it would be nice to mention that returning a promise is compulsory in case of the *Error interceptors. Do you agree?

@caitp
Copy link
Contributor

caitp commented Aug 4, 2014

It's not that it's compulsory, but it's just how promises work --- subsequent chained promises are fulfilled with the result of the promise handler --- if that value is not a rejected promise, then you fulfill the next promise with a value and the subsequent success handler is called.

@adityamenon
Copy link

Ah, I get it now. I just wanted to make sure future me or someone else doesn't repeat this mistake, but there is no non-redundant way of warning in that area of the docs... thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.