Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IE9 fix for cross domain calls #251

Merged
merged 2 commits into from
Mar 6, 2016
Merged

Conversation

ruchigoyal2005
Copy link
Contributor

This pull request includes these fixes for IE9:

  1. Fixed axios promise rejection on request onload for cors api calls.
  2. Fixed cors api calls timing out prematurely.

@nettofarah
Copy link
Contributor

I'm assuming that this works in all browsers (because travis is passing)
but have you manually checked this in other browsers too?

Also, can I suggest that you rewrite your commit message to make it a little more clear?
Here's a great guide on how to write good commit messages: http://chris.beams.io/posts/git-commit/

@ruchigoyal2005 ruchigoyal2005 changed the title For IE9 this check is failing because there no response.responseText field. Request has the responseText field. so updating the condition. Fixed axios promise rejection for cors api calls in IE9 Feb 29, 2016
@ruchigoyal2005
Copy link
Contributor Author

I tested this change manually in Chrome/firefox/IE9/IE10/IE11. I updated the commit message. thanks for the suggestion !!!

@ruchigoyal2005 ruchigoyal2005 changed the title Fixed axios promise rejection for cors api calls in IE9 IE9 fix for cross domain calls Feb 29, 2016
@mzabriskie
Copy link
Member

fixes #240

@mzabriskie
Copy link
Member

How does this work with the reference to request.responseText? This seems really strange to get the response from the request.

@ruchigoyal2005
Copy link
Contributor Author

Request is XDomainRequest and it has responseText property which retrieves the response body as a string.
https://msdn.microsoft.com/en-us/library/cc288060(v=vs.85).aspx

@mzabriskie
Copy link
Member

It's only XDomainRequest if it exists, otherwise it's XMLHttpRequest. Will it still work in both scenarios?

@ruchigoyal2005
Copy link
Contributor Author

@mzabriskie : For XMLHttpRequest we do get the response status. This scenario exists only for XDomainRequest. If you see this condition:

((response.status >= 200 && response.status < 300) ||
(!('status' in request) && request.responseText) ?
resolve :
reject)(response);

In 1st condition response status getting checked. For XMLHttpReuqest this 1st condition gets executed. For XDomainRequest we don't get the status in the response, that's why we need to check the request.responseText.

@ruchigoyal2005
Copy link
Contributor Author

Can you please merge this pull request if it looks ok ?..thanks !

mzabriskie added a commit that referenced this pull request Mar 6, 2016
IE9 fix for cross domain calls
@mzabriskie mzabriskie merged commit 84755d1 into axios:master Mar 6, 2016
@mzabriskie
Copy link
Member

Thank you for your PR!

@nickuraltsev
Copy link
Contributor

@ruchigoyal2005 @mzabriskie I'm still curious why this code rejects the promise in case when request.responseText is empty. For example, it's common to return an empty response body for successful PUT and DELETE requests.

@axios axios locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants