fix(defaultFetcher): Avoid known hang in node-fetch

americanexpress · Jul 8, 2021 · d33399c · d33399c
1 parent 0548dc6
commit d33399c
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 54 deletions.
diff --git a/packages/fetchye-core/__tests__/defaultFetcher.spec.js b/packages/fetchye-core/__tests__/defaultFetcher.spec.js
@@ -26,24 +26,19 @@ describe('defaultFetcher', () => {
     const fetchClient = jest.fn(async () => ({
       ok: true,
       status: 200,
-      json: async () => true,
+      text: async () => JSON.stringify({ jsonBodyMock: 'jsonBodyMock' }),
       headers: new global.Headers({
         'Content-Type': 'application/json',
       }),
-      clone: () => ({
-        ok: true,
-        status: 200,
-        headers: new global.Headers({
-          'Content-Type': 'application/json',
-        }),
-      }),
     }));
     const data = await defaultFetcher(fetchClient, 'http://example.com');
     expect(data).toMatchInlineSnapshot(`
       Object {
         "error": undefined,
         "payload": Object {
-          "body": true,
+          "body": Object {
+            "jsonBodyMock": "jsonBodyMock",
+          },
           "headers": Object {
             "content-type": "application/json",
           },
@@ -53,24 +48,41 @@ describe('defaultFetcher', () => {
       }
     `);
   });
+  it('should return payload with the raw text body and undefined error when the body cannot be parsed', async () => {
+    const fetchClient = jest.fn(async () => ({
+      ok: true,
+      status: 200,
+      text: async () => 'unparsableTextBodyMock',
+      headers: new global.Headers({}),
+    }));
+    const data = await defaultFetcher(fetchClient, 'http://example.com');
+    expect(data).toMatchInlineSnapshot(`
+      Object {
+        "error": undefined,
+        "payload": Object {
+          "body": "unparsableTextBodyMock",
+          "headers": Object {},
+          "ok": true,
+          "status": 200,
+        },
+      }
+    `);
+  });
   it('should return empty object if header entries does not exist', async () => {
     const fetchClient = jest.fn(async () => ({
       ok: true,
       status: 200,
-      json: async () => true,
+      text: async () => JSON.stringify({ jsonBodyMock: 'jsonBodyMock' }),
       headers: undefined,
-      clone: () => ({
-        ok: true,
-        status: 200,
-        headers: undefined,
-      }),
     }));
     const data = await defaultFetcher(fetchClient, 'http://example.com');
     expect(data).toMatchInlineSnapshot(`
       Object {
         "error": undefined,
         "payload": Object {
-          "body": true,
+          "body": Object {
+            "jsonBodyMock": "jsonBodyMock",
+          },
           "headers": Object {},
           "ok": true,
           "status": 200,
@@ -95,14 +107,8 @@ describe('defaultFetcher', () => {
     const fetchClient = jest.fn(async () => ({
       ok: true,
       status: 204,
-      json: async () => { throw new SyntaxError('Unexpected end of JSON input'); },
+      text: async () => '',
       headers: new global.Headers({}),
-      clone: () => ({
-        ok: true,
-        status: 204,
-        headers: new global.Headers({}),
-        text: async () => '',
-      }),
     }));
     const data = await defaultFetcher(fetchClient, 'http://example.com');
     expect(data).toMatchInlineSnapshot(`

diff --git a/packages/fetchye-core/src/defaultFetcher.js b/packages/fetchye-core/src/defaultFetcher.js
@@ -27,8 +27,13 @@ export const defaultFetcher = async (fetchClient, key, options) => {
   let error;
   try {
     res = await fetchClient(key, options);
-    const responseClone = res.clone();
-    const body = await res.json().catch(() => responseClone.text());
+    let body = await res.text();
+    try {
+      body = JSON.parse(body);
+      // eslint-disable-next-line no-empty
+    } catch (e) {
+      // body will still be the text from the response, so no action needed here
+    }
     payload = {
       body,
       ok: res.ok,

diff --git a/packages/fetchye/__tests__/makeServerFetchye.spec.js b/packages/fetchye/__tests__/makeServerFetchye.spec.js
@@ -29,16 +29,9 @@ const defaultPayload = {
   }),
   ok: true,
   status: 200,
-  json: async () => ({
+  text: async () => JSON.stringify({
     fakeData: true,
   }),
-  clone: () => ({
-    headers: new global.Headers({
-      'Content-Type': 'application/json',
-    }),
-    ok: true,
-    status: 200,
-  }),
 };
 
 describe('makeServerFetchye', () => {

diff --git a/packages/fetchye/__tests__/useFetchye.spec.jsx b/packages/fetchye/__tests__/useFetchye.spec.jsx
@@ -31,16 +31,9 @@ const defaultPayload = {
   }),
   ok: true,
   status: 200,
-  json: async () => ({
+  text: async () => JSON.stringify({
     fakeData: true,
   }),
-  clone: () => ({
-    headers: new global.Headers({
-      'Content-Type': 'application/json',
-    }),
-    ok: true,
-    status: 200,
-  }),
 };
 
 const ReduxSetup = (props) => {
@@ -125,13 +118,7 @@ describe('useFetchye', () => {
         global.fetch = jest.fn(async () => ({
           ...defaultPayload,
           status: 204,
-          json: async () => { throw new SyntaxError('Unexpected end of JSON input'); },
-          clone: () => ({
-            ok: true,
-            status: 204,
-            headers: new global.Headers({}),
-            text: async () => '',
-          }),
+          text: async () => '',
         }));
         render(
           <AFetchyeProvider cache={cache}>
@@ -165,14 +152,14 @@ describe('useFetchye', () => {
           if (url === 'http://example.com/one') {
             return {
               ...defaultPayload,
-              json: async () => ({
+              text: async () => JSON.stringify({
                 id: 'abc123',
               }),
             };
           }
           return {
             ...defaultPayload,
-            json: async () => ({
+            text: async () => JSON.stringify({
               resourceUrl: url,
             }),
           };
@@ -271,7 +258,7 @@ describe('useFetchye', () => {
             payload: {
               ok: res.ok,
               status: res.status,
-              body: await res.json(),
+              body: JSON.parse(await res.text()),
             },
             error: undefined,
           };
@@ -340,9 +327,7 @@ describe('useFetchye', () => {
     ].forEach(([name, AFetchyeProvider]) => {
       describe(name, () => {
         it('ensures fetch is called once per key', async () => {
-          const fakeFetchClient = jest.fn({
-            clone: () => {},
-          });
+          const fakeFetchClient = jest.fn();
           global.fetch = fakeFetchClient;
           render(
             <AFetchyeProvider cache={cache}>