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

HttpClient 'Http failure during parsing' error for valid json #18396

Closed
jamiemac87 opened this Issue Jul 28, 2017 · 19 comments

Comments

Projects
None yet
@jamiemac87

jamiemac87 commented Jul 28, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Request throws an error when trying to parse a response of "ok" which is considered valid json according to https://jsonlint.com/


request:

    const requestOptions = Object.assign(
      {}, 
      { responseType: 'json' }, 
      { observe: 'body' as HttpObserve }
    );

    this.httpClient.request('GET', './mockjson.json', requestOptions)
    ...

error: "Unexpected token o in JSON at position 0"

Expected behavior

No error should be thrown for a response of "ok"

Minimal reproduction of the problem with instructions

https://embed.plnkr.co/klPNUS/

What is the motivation / use case for changing the behavior?

Environment


Angular version: 4.3.2


Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@trotyl

This comment has been minimized.

Show comment
Hide comment
@trotyl

trotyl Jul 28, 2017

Contributor

This is definitely a bug, in HttpClient, Angular already sets the responseType to json, so that browser would automatically parse the body and provide the parsed JavaScript value as response. Then again, HttpClient is going to parse the already parsed result if it's a string value.

Contributor

trotyl commented Jul 28, 2017

This is definitely a bug, in HttpClient, Angular already sets the responseType to json, so that browser would automatically parse the body and provide the parsed JavaScript value as response. Then again, HttpClient is going to parse the already parsed result if it's a string value.

@alxhub

This comment has been minimized.

Show comment
Hide comment
@alxhub

alxhub Jul 28, 2017

Contributor

Agreed, this is a bug. Nice catch.

As a workaround, you can add an interceptor which turns the json request into text and does the JSON.parse itself.

Contributor

alxhub commented Jul 28, 2017

Agreed, this is a bug. Nice catch.

As a workaround, you can add an interceptor which turns the json request into text and does the JSON.parse itself.

@jamiemac87

This comment has been minimized.

Show comment
Hide comment
@jamiemac87

jamiemac87 Jul 28, 2017

Thanks @alxhub I'll try your workaround

jamiemac87 commented Jul 28, 2017

Thanks @alxhub I'll try your workaround

@Toxicable Toxicable referenced this issue Jul 28, 2017

Closed

fix(http): handle string json bodies #18397

3 of 3 tasks complete
@Toxicable

This comment has been minimized.

Show comment
Hide comment
@Toxicable

Toxicable Jul 28, 2017

Contributor

https://github.com/angular/angular/blob/master/packages/common/http/test/xhr_spec.ts#L95-L100
From what I'm reading here this specifically tests for a situation where the body is unparsed json but responseText is set to 'json' which according to this that's incorrect
https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/response
since it says that if it's set to 'json' that means it is parsed
So unless there is some case where this is untrue the test needs to be changed

Contributor

Toxicable commented Jul 28, 2017

https://github.com/angular/angular/blob/master/packages/common/http/test/xhr_spec.ts#L95-L100
From what I'm reading here this specifically tests for a situation where the body is unparsed json but responseText is set to 'json' which according to this that's incorrect
https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/response
since it says that if it's set to 'json' that means it is parsed
So unless there is some case where this is untrue the test needs to be changed

@jamiemac87

This comment has been minimized.

Show comment
Hide comment
@jamiemac87

jamiemac87 Aug 1, 2017

how long before this is fixed and merged?

jamiemac87 commented Aug 1, 2017

how long before this is fixed and merged?

alxhub added a commit to alxhub/angular that referenced this issue Aug 1, 2017

fix(common): fix XSSI prefix stripping by using JSON.parse always
Currently HttpClient sends requests for JSON data with the
XMLHttpRequest.responseType set to 'json'. With this flag, the browser
will attempt to parse the response as JSON, but will return 'null' on
any errors. If the JSON response contains an XSSI-prevention prefix,
this will cause the browser's parsing to fail, which is unrecoverable.

The only compelling reason to use the responseType 'json' is for
performance (especially if the browser offloads JSON parsing to a
separate thread). I'm not aware of any browser which does this currently,
nor of any plans to do so. JSON.parse and responseType 'json' both
end up using the same V8 code path in Chrome to implement the parse.

Thus, this change switches all JSON parsing in HttpClient to use
JSON.parse directly.

Fixes #18396, #18453.

alxhub added a commit to alxhub/angular that referenced this issue Aug 7, 2017

fix(common): fix XSSI prefix stripping by using JSON.parse always
Currently HttpClient sends requests for JSON data with the
XMLHttpRequest.responseType set to 'json'. With this flag, the browser
will attempt to parse the response as JSON, but will return 'null' on
any errors. If the JSON response contains an XSSI-prevention prefix,
this will cause the browser's parsing to fail, which is unrecoverable.

The only compelling reason to use the responseType 'json' is for
performance (especially if the browser offloads JSON parsing to a
separate thread). I'm not aware of any browser which does this currently,
nor of any plans to do so. JSON.parse and responseType 'json' both
end up using the same V8 code path in Chrome to implement the parse.

Thus, this change switches all JSON parsing in HttpClient to use
JSON.parse directly.

Fixes #18396, #18453.
@mrahhal

This comment has been minimized.

Show comment
Hide comment
@mrahhal

mrahhal Aug 17, 2017

Anyone has workaround code for this?

mrahhal commented Aug 17, 2017

Anyone has workaround code for this?

@mrahhal

This comment has been minimized.

Show comment
Hide comment
@mrahhal

mrahhal Aug 17, 2017

I tried my hand at it, came up with this:

import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { Observable } from 'rxjs/Observable';

// Remove when https://github.com/angular/angular/pull/18466 gets merged.

@Injectable()
export class WA18396Interceptor implements HttpInterceptor {
	constructor() { }

	intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
		if (req.responseType == 'json') {
			req = req.clone({ responseType: 'text' });

			return next.handle(req).map(response => {
				if (response instanceof HttpResponse) {
					response = response.clone<any>({ body: JSON.parse(response.body) });
				}

				return response;
			});
		}

		return next.handle(req);
	}
}

Works in my initial tests.

mrahhal commented Aug 17, 2017

I tried my hand at it, came up with this:

import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { Observable } from 'rxjs/Observable';

// Remove when https://github.com/angular/angular/pull/18466 gets merged.

@Injectable()
export class WA18396Interceptor implements HttpInterceptor {
	constructor() { }

	intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
		if (req.responseType == 'json') {
			req = req.clone({ responseType: 'text' });

			return next.handle(req).map(response => {
				if (response instanceof HttpResponse) {
					response = response.clone<any>({ body: JSON.parse(response.body) });
				}

				return response;
			});
		}

		return next.handle(req);
	}
}

Works in my initial tests.

@jamiemac87

This comment has been minimized.

Show comment
Hide comment
@jamiemac87

jamiemac87 Aug 18, 2017

This works for me. Pretty much the same as @mrahhal answer only I also found I had to catch and parse the error as well.

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const clonedRequest = req.clone({
      responseType: 'text'
    });

    return next.handle(clonedRequest)
      .map((event: HttpEvent<any>) => {
        if (event instanceof HttpResponse) {
          return event.clone({
            body: JSON.parse(event.body),
          });
        }
      })
      .catch((error: HttpErrorResponse) => {
          const parsedError = Object.assign({}, error, { error: JSON.parse(error.error) });
          return Observable.throw(new HttpErrorResponse(parsedError));
      });
  }

jamiemac87 commented Aug 18, 2017

This works for me. Pretty much the same as @mrahhal answer only I also found I had to catch and parse the error as well.

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const clonedRequest = req.clone({
      responseType: 'text'
    });

    return next.handle(clonedRequest)
      .map((event: HttpEvent<any>) => {
        if (event instanceof HttpResponse) {
          return event.clone({
            body: JSON.parse(event.body),
          });
        }
      })
      .catch((error: HttpErrorResponse) => {
          const parsedError = Object.assign({}, error, { error: JSON.parse(error.error) });
          return Observable.throw(new HttpErrorResponse(parsedError));
      });
  }
@codemonkey00

This comment has been minimized.

Show comment
Hide comment
@codemonkey00

codemonkey00 Aug 25, 2017

I updated our Http to HttpClient and all strings being returned were giving me 'Http failure during parsing'.
Thanks to mrahhal and jamiemac87, I combined your fixes and Json responses as strings are now working.

codemonkey00 commented Aug 25, 2017

I updated our Http to HttpClient and all strings being returned were giving me 'Http failure during parsing'.
Thanks to mrahhal and jamiemac87, I combined your fixes and Json responses as strings are now working.

jasonaden added a commit that referenced this issue Aug 30, 2017

fix(common): fix XSSI prefix stripping by using JSON.parse always (#1…
…8466)

Currently HttpClient sends requests for JSON data with the
XMLHttpRequest.responseType set to 'json'. With this flag, the browser
will attempt to parse the response as JSON, but will return 'null' on
any errors. If the JSON response contains an XSSI-prevention prefix,
this will cause the browser's parsing to fail, which is unrecoverable.

The only compelling reason to use the responseType 'json' is for
performance (especially if the browser offloads JSON parsing to a
separate thread). I'm not aware of any browser which does this currently,
nor of any plans to do so. JSON.parse and responseType 'json' both
end up using the same V8 code path in Chrome to implement the parse.

Thus, this change switches all JSON parsing in HttpClient to use
JSON.parse directly.

Fixes #18396, #18453.

PR Close #18466

@jasonaden jasonaden closed this in 452a7ae Aug 30, 2017

@dyh333

This comment has been minimized.

Show comment
Hide comment
@dyh333

dyh333 Oct 17, 2017

this.http.get(url, {responseType: 'text'})

it works as well.

dyh333 commented Oct 17, 2017

this.http.get(url, {responseType: 'text'})

it works as well.

@michahell

This comment has been minimized.

Show comment
Hide comment
@michahell

michahell Nov 3, 2017

For those who can not change to a text content type (in my case a POST file upload with content type application/octet-stream, the following seems like a solution:

return this.httpClient.request<File>(req)
			//TODO remove when Angular 4.4 Json parse bug is fixed
			.catch((error: HttpErrorResponse) => this.handleAngularJsonBug(error))
private handleAngularJsonBug (error: HttpErrorResponse) {
		const JsonParseError = 'Http failure during parsing for';
		const matches = error.message.match(new RegExp(JsonParseError, 'ig'));

		if (error.status === 200 && matches.length === 1) {
			// return obs that completes;
			return Observable.empty();
		} else {
			this.log.debug('re-throwing ');
			return Observable.throw(error);		// re-throw
		}
	}

michahell commented Nov 3, 2017

For those who can not change to a text content type (in my case a POST file upload with content type application/octet-stream, the following seems like a solution:

return this.httpClient.request<File>(req)
			//TODO remove when Angular 4.4 Json parse bug is fixed
			.catch((error: HttpErrorResponse) => this.handleAngularJsonBug(error))
private handleAngularJsonBug (error: HttpErrorResponse) {
		const JsonParseError = 'Http failure during parsing for';
		const matches = error.message.match(new RegExp(JsonParseError, 'ig'));

		if (error.status === 200 && matches.length === 1) {
			// return obs that completes;
			return Observable.empty();
		} else {
			this.log.debug('re-throwing ');
			return Observable.throw(error);		// re-throw
		}
	}

jhenrich added a commit to jhenrich/HomeHub that referenced this issue Nov 16, 2017

@mmanavaz

This comment has been minimized.

Show comment
Hide comment
@mmanavaz

mmanavaz Dec 5, 2017

+1 I had the same issue with interceptors.

I have a question on @jamiemac87 and @mrahhal 's answers...I'm getting that any is not a valid data-type for HttpRequest. Is there an easy way to get intelliJ IDE to recognize this datatype for HttpRequest?

mmanavaz commented Dec 5, 2017

+1 I had the same issue with interceptors.

I have a question on @jamiemac87 and @mrahhal 's answers...I'm getting that any is not a valid data-type for HttpRequest. Is there an easy way to get intelliJ IDE to recognize this datatype for HttpRequest?

@mrahhal

This comment has been minimized.

Show comment
Hide comment
@mrahhal

mrahhal Dec 5, 2017

@mmanavaz make sure you're importing from the correct module ('@angular/common/http'), and that you're on latest typescript version. Not sure why it would tell you that otherwise.

mrahhal commented Dec 5, 2017

@mmanavaz make sure you're importing from the correct module ('@angular/common/http'), and that you're on latest typescript version. Not sure why it would tell you that otherwise.

@mmanavaz

This comment has been minimized.

Show comment
Hide comment
@mmanavaz

mmanavaz Dec 5, 2017

@mrahhal still getting the same issue...I checked for the correct import and latest Typescript version...both are correct.

mmanavaz commented Dec 5, 2017

@mrahhal still getting the same issue...I checked for the correct import and latest Typescript version...both are correct.

@mmanavaz

This comment has been minimized.

Show comment
Hide comment
@mmanavaz

mmanavaz Dec 7, 2017

I was able to figure it out.

mmanavaz commented Dec 7, 2017

I was able to figure it out.

@trotyl

This comment has been minimized.

Show comment
Hide comment
@trotyl

trotyl Feb 6, 2018

Contributor

@subinsebastian That's completely irrelevant issue. Plain text is not JSON, what you're looking for is #19413. And all of these issues have already been fixed.

Contributor

trotyl commented Feb 6, 2018

@subinsebastian That's completely irrelevant issue. Plain text is not JSON, what you're looking for is #19413. And all of these issues have already been fixed.

@leojkwan

This comment has been minimized.

Show comment
Hide comment
@leojkwan

leojkwan Mar 18, 2018

Ran into same problem making a put request to my endpoint. I realized I was sending 200 without a response body. Sending status code 204 made angular http 5.2.8 stop complaining. This problem should not be happening even with a 200, but 204 is the correct code for empty responses.

leojkwan commented Mar 18, 2018

Ran into same problem making a put request to my endpoint. I realized I was sending 200 without a response body. Sending status code 204 made angular http 5.2.8 stop complaining. This problem should not be happening even with a 200, but 204 is the correct code for empty responses.

@ryanbuening

This comment has been minimized.

Show comment
Hide comment
@ryanbuening

ryanbuening Mar 28, 2018

@leojkwan thanks for your comment. I ran into the same issue with Angular 5.2.9.

ryanbuening commented Mar 28, 2018

@leojkwan thanks for your comment. I ran into the same issue with Angular 5.2.9.

@dreamid27

This comment has been minimized.

Show comment
Hide comment
@dreamid27

dreamid27 May 2, 2018

@dyh333
this.http.get(url, {responseType: 'text'})

it works as well.

Some people may be confusing about this and I'm included,
this is the reference for he's mean : https://angular.io/guide/http#requesting-non-json-data

thank for you @dyh333 and sorry my english too bad.

dreamid27 commented May 2, 2018

@dyh333
this.http.get(url, {responseType: 'text'})

it works as well.

Some people may be confusing about this and I'm included,
this is the reference for he's mean : https://angular.io/guide/http#requesting-non-json-data

thank for you @dyh333 and sorry my english too bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment