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

Observable.ajax response is string for IE11 #1381

Closed
gbabiars opened this issue Feb 23, 2016 · 10 comments
Closed

Observable.ajax response is string for IE11 #1381

gbabiars opened this issue Feb 23, 2016 · 10 comments

Comments

@gbabiars
Copy link

I'm seeing an issue on 5.0.0-beta.2 where the ajax observable returns plain text when responseType is either set to json or left blank.

Example:

Observable.ajax({
  method: 'GET',
  url: `api/blah`,
  crossDomain: true,
  resultSelector(res) {
    return res.response
  }
});

For a payload that is an Array, res.response is of type Array in Chrome, however, on IE11 it is a string. Digging into it, it appears the responseType property on the xhr at https://github.com/ReactiveX/RxJS/blob/master/src/observable/dom/AjaxObservable.ts#L353 appears to always be empty, regardless of what request.responseType was at https://github.com/ReactiveX/RxJS/blob/master/src/observable/dom/AjaxObservable.ts#L215.

This is easy enough to work around in the resultSelector:

function jsonResultSelector(res) {
  if(typeof res.response === 'string') {
    return JSON.parse(res.response);
  }
  return res.response;
}
@benlesh benlesh added bug Confirmed bug help wanted Issues we wouldn't mind assistance with. labels Feb 24, 2016
@kwonoj
Copy link
Member

kwonoj commented Mar 11, 2016

I got interested in this and try to reproduce it, but for some reason I wasn't able to do so. I assume I've done something incorrectly.. @gbabiars , would you mind to share some details how I can reproduce this one?

@benlesh
Copy link
Member

benlesh commented Mar 14, 2016

I can't reproduce this either. So I'm removing the bug label for now.

@benlesh benlesh removed help wanted Issues we wouldn't mind assistance with. bug Confirmed bug labels Mar 14, 2016
@benlesh
Copy link
Member

benlesh commented Mar 14, 2016

@gbabiars can you show the full request and response headers that produced this issue?

@wardbell
Copy link
Contributor

A plunker or codepen or something like that would help.

@gbabiars
Copy link
Author

@Blesh @kwonoj sorry for the slow response. I will try to get a repro today that I can share.

@gbabiars
Copy link
Author

I've created a repo at https://github.com/gbabiars/rxjs-ajax-repro since ajax is not included in the npmcdn build. The request I'm doing is a CORS request over http (the original was https, so that does not appear to matter). The repro demo appends the actual type of the response property and the value of the responseType.

On Chrome, I get an object for response and responseType is json.
screen shot 2016-03-17 at 11 10 25 am

On IE11, I am getting string for response and responseType is empty.
screen shot 2016-03-17 at 11 10 16 am

I will try to do some more investigating into this, but hopefully this helps for repro steps.

@kwonoj
Copy link
Member

kwonoj commented Mar 17, 2016

Thanks! trying now...

@kwonoj
Copy link
Member

kwonoj commented Mar 17, 2016

OK, could reproduce issue. Able to confirm XHR responseType is empty instead of being set to json, and after digging into bit more it seems like IE doesn't support json as responseType (https://msdn.microsoft.com/en-us/library/hh872882(v=vs.85).aspx) while other browser (Chrome > 31, Firefox > 10, https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType / http://caniuse.com/#search=XMLHttpRequest) implemented it.

Should ajax implementation takes care of parsing json on IE? Seems there are some of IE specific handling (onreadystatechange) so it may makes sense.. /cc @trxcllnt also for asking opinion.

kwonoj added a commit to kwonoj/rxjs that referenced this issue Apr 7, 2016
- IE does not support json responseType, internally parse it into JSON object

closes ReactiveX#1381
staltz pushed a commit to staltz/RxJSNext that referenced this issue Apr 13, 2016
- IE does not support json responseType, internally parse it into JSON object

closes ReactiveX#1381
jayphelps pushed a commit that referenced this issue Aug 31, 2016
* Return null value from JSON.Parse

JSON.parse('') will throw a syntax error (in any browser). Fix is to return a null value. I saw this when my server returned HTTP 204 No Content.

* added ajax tests for 204 No Content

* mock the behavior of IE
unit test for IE

* pascal case class name.
added missing semicolon.

* Use 'null' instead of '' for JSON.parse

* remove empty constructor

* fix(AjaxObservable): return null value from JSON.Parse

JSON.parse('') will throw a syntax error (in any browser). Fix is to return a null value.
This happens during an HTTP 204 'No Content' in IE.
Added a mock for InternetExplorer. Added unit tests for HTTP 204.

closes #1381
@trzewiczek
Copy link

Hi,

I run into similar problem with 400 response.

Response Headers:
    HTTP/1.1 400 Bad Request
    Content-Type: application/json; charset=utf-8
"xhr": {
    "response": "{\"message\":\"Username can not be changed\"}",
    "responseType": "",
    "status": 400,
    "statusText": "Bad Request"
}

Non 2xx status responses fall under AjaxError constructor which does non of the IE magic that AjaxResponse constructor does, see: AjaxObservable.ts#L451.

Is that on purpose or should be fixed? (Happy to help here)

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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 a pull request may close this issue.

5 participants