fix(AjaxObservable): notify with error if fails to parse json response (#3139)

* fix(AjaxObservable): notify with error if fails to parse json response
Catch error when parsing json response and notify observer with thrown error.
JSON could be invalid if proxy overtakes the response and in IE responseType is empty.
closes#3138
* chore(lint): remove semicolon

Author: myklt

spec/observables/dom/ajax-spec.ts

@@ -246,6 +246,34 @@ describe('ajax', () => {
 expect(complete).to.be.true;
 });
 
+it('should fail if fails to parse response',()=>{
+leterror: any;
+constobj={
+url: '/flibbertyJibbet',
+responseType: 'json',
+method: ''
+};
+
+ajax(obj)
+.subscribe((x: any)=>{
+throw'should not next';
+},(err: any)=>{
+error=err;
+},()=>{
+throw'should not complete';
+});
+
+MockXMLHttpRequest.mostRecent.respondWith({
+'status': 207,
+'contentType': '',
+'responseType': '',
+'responseText': 'Wee! I am text, but should be valid JSON!'
+});
+
+expect(errorinstanceofSyntaxError).to.be.true;
+expect(error.message).to.equal('Unexpected token W in JSON at position 0');
+});
+
 it('should fail on 404',()=>{
 leterror: any;
 constobj={
@@ -312,6 +340,36 @@ describe('ajax', () => {
 expect(complete).to.be.true;
 });
 
+it('should fail if fails to parse error response',()=>{
+leterror: any;
+constobj={
+url: '/flibbertyJibbet',
+normalizeError: (e: any,xhr: any,type: any)=>{
+returnxhr.response||xhr.responseText;
+},
+responseType: 'json',
+method: ''
+};
+
+ajax(obj).subscribe(x=>{
+throw'should not next';
+},(err: any)=>{
+error=err;
+},()=>{
+throw'should not complete';
+});
+
+MockXMLHttpRequest.mostRecent.respondWith({
+'status': 404,
+'contentType': '',
+'responseType': '',
+'responseText': 'Wee! I am text, but should be valid JSON!'
+});
+
+expect(errorinstanceofSyntaxError).to.be.true;
+expect(error.message).to.equal('Unexpected token W in JSON at position 0');
+});
+
 it('should succeed no settings',()=>{
 constexpected=JSON.stringify({foo: 'bar'});
 
@@ -1104,8 +1162,6 @@ class MockXMLHttpRequest {
 protecteddefaultResponseValue(){
 if(this.async===false){
 this.response=this.responseText;
-}else{
-thrownewError('unhandled type "'+this.responseType+'"');
 }
 }
 
@@ -1121,8 +1177,9 @@ class MockXMLHttpRequest {
 };
 this.status=response.status||200;
 this.responseText=response.responseText;
+constresponseType=response.responseType!==undefined ? response.responseType : this.responseType;
 if(!('response'inresponse)){
-switch(this.responseType){
+switch(responseType){
 case'json':
 this.jsonResponseValue(response);
 break;

src/internal/observable/dom/AjaxObservable.ts

@@ -221,8 +221,11 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
 this.done=true;
 const{ xhr, request, destination }=this;
 constresponse=newAjaxResponse(e,xhr,request);
-
-destination.next(response);
+if(response.response===errorObject){
+destination.error(errorObject.e);
+}else{
+destination.next(response);
+}
 }
 
 privatesend(): XMLHttpRequest{
@@ -320,7 +323,12 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
 if(progressSubscriber){
 progressSubscriber.error(e);
 }
-subscriber.error(newAjaxTimeoutError(this,request));//TODO: Make betterer.
+constajaxTimeoutError=newAjaxTimeoutError(this,request);//TODO: Make betterer.
+if(ajaxTimeoutError.response===errorObject){
+subscriber.error(errorObject.e);
+}else{
+subscriber.error(ajaxTimeoutError);
+}
 }
 xhr.ontimeout=xhrTimeout;
 (<any>xhrTimeout).request=request;
@@ -346,7 +354,12 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
 if(progressSubscriber){
 progressSubscriber.error(e);
 }
-subscriber.error(newAjaxError('ajax error',this,request));
+constajaxError=newAjaxError('ajax error',this,request);
+if(ajaxError.response===errorObject){
+subscriber.error(errorObject.e);
+}else{
+subscriber.error(ajaxError);
+}
 };
 xhr.onerror=xhrError;
 (<any>xhrError).request=request;
@@ -388,7 +401,12 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
 if(progressSubscriber){
 progressSubscriber.error(e);
 }
-subscriber.error(newAjaxError('ajax error '+status,this,request));
+constajaxError=newAjaxError('ajax error '+status,this,request);
+if(ajaxError.response===errorObject){
+subscriber.error(errorObject.e);
+}else{
+subscriber.error(ajaxError);
+}
 }
 }
 }
@@ -474,17 +492,21 @@ export class AjaxError extends Error {
 }
 }
 
+functionparseJson(xhr: XMLHttpRequest){
+// HACK(benlesh): TypeScript shennanigans
+// tslint:disable-next-line:no-any XMLHttpRequest is defined to always have 'response' inferring xhr as never for the else clause.
+if('response'in(xhrasany)){
+//IE does not support json as responseType, parse it internally
+returnxhr.responseType ? xhr.response : JSON.parse(xhr.response||xhr.responseText||'null');
+}else{
+returnJSON.parse((xhrasany).responseText||'null');
+}
+}
+
 functionparseXhrResponse(responseType: string,xhr: XMLHttpRequest){
 switch(responseType){
 case'json':
-// HACK(benlesh): TypeScript shennanigans
-// tslint:disable-next-line:no-any XMLHttpRequest is defined to always have 'response' inferring xhr as never for the else clause.
-if('response'in(xhrasany)){
-//IE does not support json as responseType, parse it internally
-returnxhr.responseType ? xhr.response : JSON.parse(xhr.response||xhr.responseText||'null');
-}else{
-returnJSON.parse((xhrasany).responseText||'null');
-}
+returntryCatch(parseJson)(xhr);
 case'xml':
 returnxhr.responseXML;
 case'text':