fix(common): attempt to JSON.parse errors for JSON responses (#19773)

Prior behavior for HttpClient was to parse the body as JSON when
responseType was set to 'json', even if the response was
unsuccessful. This changed due to a recent bugfix, and
unsuccessful responses had their bodies treated as strings.

There is no guarantee that if a service returns JSON in the
successful case that it will do so for errors. However, users
indicate that most APIs in the wild do work this way. Therefore,
this commit changes the error case behavior to attempt a JSON
parsing of the response body, and falls back on returning it as
a string if that fails.

PR Close #19773
This commit is contained in:
Alex Rickabaugh 2017-10-17 17:30:56 -07:00 committed by Tobias Bosch
parent 6e6c866de9
commit 269f5acc54
2 changed files with 16 additions and 1 deletions

View File

@ -191,6 +191,14 @@ export class HttpXhrBackend implements HttpBackend {
// The parse error contains the text of the body that failed to parse.
body = { error, text: body } as HttpJsonParseError;
}
} else if (!ok && req.responseType === 'json' && typeof body === 'string') {
try {
// Attempt to parse the body as JSON.
body = JSON.parse(body);
} catch (error) {
// Cannot be certain that the body was meant to be parsed as JSON.
// Leave the body as a string.
}
}
if (ok) {

View File

@ -17,7 +17,7 @@ import {MockXhrFactory} from './xhr_mock';
function trackEvents(obs: Observable<HttpEvent<any>>): HttpEvent<any>[] {
const events: HttpEvent<any>[] = [];
obs.subscribe(event => events.push(event));
obs.subscribe(event => events.push(event), err => events.push(err));
return events;
}
@ -92,6 +92,13 @@ export function main() {
const res = events[1] as HttpResponse<{data: string}>;
expect(res.body !.data).toBe('some data');
});
it('handles a json error response', () => {
const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'})));
factory.mock.mockFlush(500, 'Error', JSON.stringify({data: 'some data'}));
expect(events.length).toBe(2);
const res = events[1] as any as HttpErrorResponse;
expect(res.error !.data).toBe('some data');
});
it('handles a json string response', () => {
const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'})));
expect(factory.mock.responseType).toEqual('text');