Merge pull request #44 from sergiop/bugfix/api-error-handling

Fix a bug that prevents to handle API errors.
sergiop · Dec 23, 2020 · c4c1847 · c4c1847
2 parents 7eec11c + f82331e
commit c4c1847
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 44 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "react-giphy-searchbox",
-  "version": "1.5.0",
+  "version": "1.5.1",
   "description": "React Giphy Searchbox",
   "main": "lib/index.js",
   "module": "es/index.js",

diff --git a/src/hooks/useApi.js b/src/hooks/useApi.js
@@ -17,7 +17,15 @@ const useApi = () => {
     }
 
     fetch(url)
-      .then(response => response.json())
+      .then(response => {
+        if (!response.ok) {
+          return response.json().then(json => {
+            throw json
+          })
+        }
+
+        return response.json()
+      })
       .then(response => {
         if (isMore) {
           dispatch({

diff --git a/src/hooks/useApi.spec.js b/src/hooks/useApi.spec.js
@@ -1,76 +1,82 @@
-import fetchMock from 'fetch-mock-jest'
 import { renderHook, act } from '@testing-library/react-hooks'
+import giphyTrendingGet404Error from '../../tests/fixtures/giphyTrendingGet404Error.json'
+import giphyTrendingGetSuccess from '../../tests/fixtures/giphyTrendingGetSuccess.json'
 import useApi from './useApi'
 
 describe('useApi', () => {
-  const fetchingValues = {
+  const fetchingInitValues = {
     loading: true,
     error: false,
     data: [],
     lastPage: false,
   }
 
-  fetchMock.mock('/foo', {
-    status: 200,
-    data: [{ foo: 'foo' }],
-    pagination: {
-      total_count: 200,
-      count: 25,
-      offset: 0,
-    },
-  })
-
-  fetchMock.mock('/foo2', { status: 500 })
-
   test('perform a get request and receive some data', async () => {
     const { result, waitForNextUpdate } = renderHook(() => useApi())
-
     const [, fetchImages] = result.current
 
+    window.fetch = jest.fn().mockResolvedValueOnce({
+      ok: true,
+      status: 200,
+      json: async () => giphyTrendingGetSuccess,
+    })
+
     act(() => {
-      fetchImages('/foo')
+      fetchImages()
     })
 
-    expect(result.current[0]).toEqual(fetchingValues)
+    expect(result.current[0]).toEqual(fetchingInitValues)
     await waitForNextUpdate()
     expect(result.current[0]).toEqual({
       loading: false,
       error: false,
-      data: [{ foo: 'foo' }],
+      data: giphyTrendingGetSuccess.data,
       lastPage: false,
     })
   })
 
   test('perform a get request with `isMore` option and receive some data', async () => {
     const { result, waitForNextUpdate } = renderHook(() => useApi())
-
     const [, fetchImages] = result.current
 
+    window.fetch = jest.fn().mockResolvedValueOnce({
+      ok: true,
+      status: 200,
+      json: async () => giphyTrendingGetSuccess,
+    })
+
     act(() => {
-      fetchImages('/foo', true)
+      fetchImages('', true)
     })
 
-    expect(result.current[0]).toEqual(fetchingValues)
+    expect(result.current[0]).toEqual(fetchingInitValues)
     await waitForNextUpdate()
     expect(result.current[0]).toEqual({
       loading: false,
       error: false,
-      data: [{ foo: 'foo' }],
+      data: giphyTrendingGetSuccess.data,
       lastPage: false,
     })
   })
 
   test('perform a get request and receive an error', async () => {
     const { result, waitForNextUpdate } = renderHook(() => useApi())
+    const [, fetchImages] = result.current
 
-    const [, fetch] = result.current
+    window.fetch = jest.fn().mockResolvedValueOnce({
+      ok: false,
+      status: 404,
+      json: async () => giphyTrendingGet404Error,
+    })
 
     act(() => {
-      fetch('/foo2')
+      fetchImages()
     })
 
-    expect(result.current[0]).toEqual(fetchingValues)
+    expect(result.current[0]).toEqual(fetchingInitValues)
+
     await waitForNextUpdate()
+
     expect(result.current[0]).toEqual({
       loading: false,
       error: true,

diff --git a/tests/index.spec.js b/tests/index.spec.js
@@ -10,12 +10,6 @@ import assetsPoweredByGiphy from '../src/assets/poweredByGiphy.png'
 
 // TO-DO: Test the loading more (infinite scrolling)
 
-const fetchMock = data => {
-  return Promise.resolve({
-    json: () => Promise.resolve(data),
-  })
-}
-
 describe('ReactGiphySearchbox', () => {
   const onSelect = jest.fn()
   const onSearch = jest.fn()
@@ -51,12 +45,13 @@ describe('ReactGiphySearchbox', () => {
   })
 
   test('fetches Giphy Api and displays trending gifs', async () => {
-    window.fetch = jest
-      .fn()
-      .mockImplementationOnce(() => fetchMock(giphyTrendingGetSuccess))
+    window.fetch = jest.fn().mockResolvedValueOnce({
+      ok: true,
+      status: 200,
+      json: async () => giphyTrendingGetSuccess,
+    })
 
     const { getByTestId } = buildSubject()
-
     // Loading message displayed
     const Loader = await waitForElement(() => getByTestId('SpinnerText'))
     expect(Loader).toHaveTextContent(defaults.messageLoading)
@@ -71,9 +66,11 @@ describe('ReactGiphySearchbox', () => {
   })
 
   test('fetches Giphy Api and returns an error', async () => {
-    window.fetch = jest
-      .fn()
-      .mockRejectedValueOnce(() => fetchMock(giphyTrendingGet404Error))
+    window.fetch = jest.fn().mockResolvedValueOnce({
+      ok: false,
+      status: 404,
+      json: async () => giphyTrendingGet404Error,
+    })
     const { getByTestId } = buildSubject()
 
     expect(getByTestId('SpinnerText')).toHaveTextContent(
@@ -91,8 +88,16 @@ describe('ReactGiphySearchbox', () => {
     let Loader
     window.fetch = jest
       .fn()
-      .mockImplementationOnce(() => fetchMock(giphyTrendingGetSuccess))
-      .mockImplementationOnce(() => fetchMock(giphySearchGetSuccess))
+      .mockResolvedValueOnce({
+        ok: true,
+        status: 200,
+        json: async () => giphyTrendingGetSuccess,
+      })
+      .mockResolvedValueOnce({
+        ok: true,
+        status: 200,
+        json: async () => giphySearchGetSuccess,
+      })
 
     const { getByTestId } = buildSubject()
 
@@ -132,8 +137,16 @@ describe('ReactGiphySearchbox', () => {
     let Loader
     window.fetch = jest
       .fn()
-      .mockImplementationOnce(() => fetchMock(giphyTrendingGetSuccess))
-      .mockImplementationOnce(() => fetchMock(giphySearchGetSuccessEmpty))
+      .mockResolvedValueOnce({
+        ok: true,
+        status: 200,
+        json: async () => giphyTrendingGetSuccess,
+      })
+      .mockResolvedValueOnce({
+        ok: true,
+        status: 200,
+        json: async () => giphySearchGetSuccessEmpty,
+      })
 
     const { getByTestId } = buildSubject()