From 269f5acc54823652f132f5eef3a57d3be4eabe18 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 17 Oct 2017 17:30:56 -0700 Subject: [PATCH] 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 --- packages/common/http/src/xhr.ts | 8 ++++++++ packages/common/http/test/xhr_spec.ts | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts index c7123ae792..0d14a01bfa 100644 --- a/packages/common/http/src/xhr.ts +++ b/packages/common/http/src/xhr.ts @@ -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) { diff --git a/packages/common/http/test/xhr_spec.ts b/packages/common/http/test/xhr_spec.ts index 6c37619333..4c76dce6ad 100644 --- a/packages/common/http/test/xhr_spec.ts +++ b/packages/common/http/test/xhr_spec.ts @@ -17,7 +17,7 @@ import {MockXhrFactory} from './xhr_mock'; function trackEvents(obs: Observable>): HttpEvent[] { const events: HttpEvent[] = []; - 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');