feat(fetchOEmbedData): Handle non-ok responses (#104)

* fix(fetchOEmbedData): throw on non-ok response

* adjust tests to use actual Response object instead of object approximation

Author: pieh

src/__tests__/transformers/GIPHY.js

@@ -11,12 +11,13 @@ import {
 
 import { cache, getMarkdownASTForFile, parseASTToMarkdown } from '../helpers';
 
+const { Response } = jest.requireActual('node-fetch');
 jest.mock('node-fetch', () => jest.fn());
 
 const mockFetch = ({ height, width }) =>
-  fetchMock.mockResolvedValue({
-    json: () => Promise.resolve({ height, width }),
-  });
+  fetchMock.mockImplementation(() =>
+    Promise.resolve(new Response(JSON.stringify({ height, width })))
+  );
 
 beforeEach(() => {
   fetchMock.mockClear();

src/__tests__/transformers/Instagram.js

@@ -6,10 +6,13 @@ import { getHTML, shouldTransform } from '../../transformers/Instagram';
 
 import { cache, getMarkdownASTForFile, parseASTToMarkdown } from '../helpers';
 
+const { Response } = jest.requireActual('node-fetch');
 jest.mock('node-fetch', () => jest.fn());
 
 const mockFetch = html =>
-  fetchMock.mockResolvedValue({ json: () => Promise.resolve({ html }) });
+  fetchMock.mockImplementation(() =>
+    Promise.resolve(new Response(JSON.stringify({ html })))
+  );
 
 beforeEach(() => {
   fetchMock.mockClear();

src/__tests__/transformers/Streamable.js

@@ -10,10 +10,13 @@ import {
 
 import { cache, getMarkdownASTForFile, parseASTToMarkdown } from '../helpers';
 
+const { Response } = jest.requireActual('node-fetch');
 jest.mock('node-fetch', () => jest.fn());
 
 const mockFetch = html =>
-  fetchMock.mockResolvedValue({ json: () => Promise.resolve({ html }) });
+  fetchMock.mockImplementation(() =>
+    Promise.resolve(new Response(JSON.stringify({ html })))
+  );
 
 beforeEach(() => {
   fetchMock.mockClear();

src/__tests__/transformers/Twitter.js

@@ -6,16 +6,17 @@ import { getHTML, shouldTransform } from '../../transformers/Twitter';
 
 import { cache, getMarkdownASTForFile, parseASTToMarkdown } from '../helpers';
 
+const { Response } = jest.requireActual('node-fetch');
 jest.mock('node-fetch', () => jest.fn());
 
 const mockFetch = (status, moment) =>
   fetchMock
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: status }) })
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: status }) })
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: moment }) })
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: moment }) })
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: moment }) })
-    .mockResolvedValueOnce({ json: () => Promise.resolve({ html: moment }) });
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: status })))
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: status })))
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: moment })))
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: moment })))
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: moment })))
+    .mockResolvedValueOnce(new Response(JSON.stringify({ html: moment })));
 
 beforeEach(() => {
   fetchMock.mockReset();

src/__tests__/transformers/utils/fetchOEmbedData.js

@@ -34,6 +34,25 @@ describe(`fetchOEmbedData`, () => {
     });
   });
 
+  test(`throws on non-ok status`, () => {
+    // make sure we make our result assertion
+    expect.assertions(1);
+
+    fetch.mockResolvedValue(
+      new Response(JSON.stringify(MockedResponseResult), {
+        // non-ok status is one outside of 200-299 range
+        // ( https://developer.mozilla.org/en-US/docs/Web/API/Response/ok )
+        status: 403,
+      })
+    );
+
+    return fetchOEmbedData(URL).catch(err => {
+      expect(err).toMatchInlineSnapshot(
+        `[Error: Request to https://google.com returned non-OK status (403)]`
+      );
+    });
+  });
+
   describe(`network resilience`, () => {
     test(`retries requests if there was network error`, () => {
       // make sure we make our result assertion

src/transformers/utils/index.js

@@ -6,7 +6,17 @@ export const fetchOEmbedData = url =>
   fetchWithRetries(url, {
     retries: 3,
     retryDelay: attempt => 2 ** attempt * 1000,
-  }).then(data => data.json());
+  })
+    .then(response => {
+      if (!response.ok) {
+        throw new Error(
+          `Request to ${url} returned non-OK status (${response.status})`
+        );
+      }
+
+      return response;
+    })
+    .then(data => data.json());
 
 export const getTrimmedPathName = pathname =>
   // Trim leading and trailing slashes