From 24236c7361ecbedf4c28b39f9780e1b8df8ca38d Mon Sep 17 00:00:00 2001 From: Bart Verbruggen Date: Thu, 14 Mar 2019 16:53:05 +0100 Subject: [PATCH 1/4] "HTTP 204 No content" should not parse as json, even if application/json is set as contentType. --- .../src/RESTDataSource.ts | 7 ++++--- .../src/__tests__/RESTDataSource.test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/apollo-datasource-rest/src/RESTDataSource.ts b/packages/apollo-datasource-rest/src/RESTDataSource.ts index fc12f508884..0b0e9292c0d 100644 --- a/packages/apollo-datasource-rest/src/RESTDataSource.ts +++ b/packages/apollo-datasource-rest/src/RESTDataSource.ts @@ -104,9 +104,10 @@ export abstract class RESTDataSource extends DataSource { protected parseBody(response: Response): Promise { const contentType = response.headers.get('Content-Type'); if ( - contentType && - (contentType.startsWith('application/json') || - contentType.startsWith('application/hal+json')) + response.status !== 204 || + (contentType && + (contentType.startsWith('application/json') || + contentType.startsWith('application/hal+json'))) ) { return response.json(); } else { diff --git a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts index 477ad4cc384..83278670740 100644 --- a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts +++ b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts @@ -473,6 +473,24 @@ describe('RESTDataSource', () => { expect(data).toEqual('bar'); }); + + it('returns data as a string when response status code is 204 no content', async () => { + const dataSource = new class extends RESTDataSource { + baseURL = 'https://api.example.com'; + + getFoo() { + return this.get('foo'); + } + }(); + + dataSource.httpCache = httpCache; + + fetch.mockResponseOnce('', { 'Content-Type': 'application/json' }, 204); + + const data = await dataSource.getFoo(); + + expect(data).toEqual(''); + }); }); describe('memoization', () => { From 9aa3342b082bf628115556d86e273cb8c07dcbaf Mon Sep 17 00:00:00 2001 From: Bart Verbruggen Date: Mon, 18 Mar 2019 11:42:33 +0100 Subject: [PATCH 2/4] update test --- .../apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts index 83278670740..d079618c5df 100644 --- a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts +++ b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts @@ -479,7 +479,7 @@ describe('RESTDataSource', () => { baseURL = 'https://api.example.com'; getFoo() { - return this.get('foo'); + return this.get(''); } }(); From 38e600dd4c8e519c0a49a225352f29414e3dc829 Mon Sep 17 00:00:00 2001 From: Bart Verbruggen Date: Thu, 4 Apr 2019 16:13:03 +0200 Subject: [PATCH 3/4] =?UTF-8?q?use=20AND=20instead=20of=20OR=20for=20the?= =?UTF-8?q?=20response.status=20=F0=9F=98=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/apollo-datasource-rest/src/RESTDataSource.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/apollo-datasource-rest/src/RESTDataSource.ts b/packages/apollo-datasource-rest/src/RESTDataSource.ts index 0b0e9292c0d..c14752d0154 100644 --- a/packages/apollo-datasource-rest/src/RESTDataSource.ts +++ b/packages/apollo-datasource-rest/src/RESTDataSource.ts @@ -104,10 +104,10 @@ export abstract class RESTDataSource extends DataSource { protected parseBody(response: Response): Promise { const contentType = response.headers.get('Content-Type'); if ( - response.status !== 204 || - (contentType && - (contentType.startsWith('application/json') || - contentType.startsWith('application/hal+json'))) + response.status !== 204 && + contentType && + (contentType.startsWith('application/json') || + contentType.startsWith('application/hal+json')) ) { return response.json(); } else { From 935f3bdd3c597dd810a8699af8ccd94e220083fb Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 5 Apr 2019 16:57:30 +0300 Subject: [PATCH 4/4] Update RESTDataSource.ts --- packages/apollo-datasource-rest/src/RESTDataSource.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/apollo-datasource-rest/src/RESTDataSource.ts b/packages/apollo-datasource-rest/src/RESTDataSource.ts index c14752d0154..107709a84b4 100644 --- a/packages/apollo-datasource-rest/src/RESTDataSource.ts +++ b/packages/apollo-datasource-rest/src/RESTDataSource.ts @@ -104,6 +104,8 @@ export abstract class RESTDataSource extends DataSource { protected parseBody(response: Response): Promise { const contentType = response.headers.get('Content-Type'); if ( + // As one might expect, a "204 No Content" is empty! This means there + // isn't enough to `JSON.parse`, and trying will result in an error. response.status !== 204 && contentType && (contentType.startsWith('application/json') ||