Merge pull request #6 from PilotConway/feature/requestError

Refactor Errors

Author: John Conway

package.json

@@ -16,6 +16,7 @@
   },
   "devDependencies": {
     "codecov": "^3.5.0",
+    "nock": "^10.0.6",
     "typescript": "3.3.3"
   },
   "scripts": {

src/client/__mocks__/getRequest.js

@@ -36,6 +36,10 @@ const client = {
   /**
    * Runs a get operation on the provided path and returns the data.
    *
+   * @param {boolean} ok False if the response contains an error.
+   * @param {number} status The http status code of the response.
+   * @param {string} statusText A message if provided to describe the status response. Null if none
+   *                 was supplied.
    * @param {object|array} data The data from the server.
    * @param {AjaxResponse} rawResponse The RxJS AJAX Response object from the server.
    * @param {object} links The links object containing any link header URLS. All properties are
@@ -51,7 +55,10 @@ const client = {
     const links = processLinks(response);
 
     return {
+      ok: response.ok,
       data: response.response,
+      status: response.status,
+      statusText: (response.response || {}).message || response.message || null,
       rawResponse: response,
       links,
     };

src/client/client.js

@@ -1,15 +1,15 @@
 import client from './client';
-// Require get request import for mocks to work
-// eslint-disable-next-line no-unused-vars
-import getRequest from './getRequest';
+import nock from 'nock';
 
-// SEE __mocks__/getRequest for which endpoint paths result in what types of
-// responses from the getRequest().toPromise() method.
-jest.mock('./getRequest');
+// make sure no requests are actually sent
+nock.disableNetConnect();
 
 describe('raw client', () => {
   it('calls getRequest and returns data', async () => {
-    const response = await client.get('foo');
+    nock('http://localhost')
+      .get('/foo')
+      .reply(200, { bar: 'baz' });
+    const response = await client.get('http://localhost/foo');
 
     expect(response.data).toEqual({ bar: 'baz' });
     expect(response.links).toEqual({
@@ -20,30 +20,64 @@ describe('raw client', () => {
     });
   });
 
-  it('rejects when when an error occurs', async () => {
-    expect(client.get('/404')).rejects.toThrow();
+  it('responds with an error response when when an error from the server occurs', async () => {
+    nock('http://localhost')
+      .get('/404')
+      .reply(404, { message: 'Not Found' });
+    const response = await client.get('http://localhost/404');
+    expect(response.ok).toBe(false);
+    expect(response.data).toEqual({ message: 'Not Found' });
+    expect(response.status).toEqual(404);
+    expect(response.statusText).toEqual('Not Found');
+  });
+
+  it('Has statusText set to ajax error messsage when no message is in the response', async () => {
+    nock('http://localhost')
+      .get('/500')
+      .reply(500);
+    const response = await client.get('http://localhost/500');
+    expect(response.ok).toBe(false);
+    expect(response.data).toEqual(null);
+    expect(response.status).toEqual(500);
+    expect(response.statusText).toEqual('ajax error 500');
   });
 
   describe('links', () => {
     it('returns next link', async () => {
+      nock('http://localhost')
+        .get('/link/next')
+        .reply(200, {}, { Link: '<https://example.com/foo?page=7>; rel="next"' });
+
       const response = await client.get('/link/next');
 
       expect(response.links.next).toEqual('https://example.com/foo?page=7');
     });
 
     it('returns previous link', async () => {
+      nock('http://localhost')
+        .get('/link/prev')
+        .reply(200, {}, { Link: '<https://example.com/foo?page=5>; rel="prev"' });
+
       const response = await client.get('/link/prev');
 
       expect(response.links.prev).toEqual('https://example.com/foo?page=5');
     });
 
     it('returns first link', async () => {
+      nock('http://localhost')
+        .get('/link/first')
+        .reply(200, {}, { Link: '<https://example.com/foo?page=1>; rel="first"' });
+
       const response = await client.get('/link/first');
 
       expect(response.links.first).toEqual('https://example.com/foo?page=1');
     });
 
     it('returns last link', async () => {
+      nock('http://localhost')
+        .get('/link/last')
+        .reply(200, {}, { Link: '<https://example.com/foo?page=10>; rel="last"' });
+
       const response = await client.get('/link/last');
 
       expect(response.links.last).toEqual('https://example.com/foo?page=10');

src/client/client.spec.js

@@ -1,11 +1,7 @@
 import createClient from './createClient';
-// Require get request import for mocks to work
-// eslint-disable-next-line no-unused-vars
-import getRequest from './getRequest';
+import nock from 'nock';
 
-// SEE __mocks__/getRequest for which endpoint paths result in what types of
-// responses from the getRequest().toPromise() method.
-jest.mock('./getRequest');
+nock.disableNetConnect();
 
 describe('createClient', () => {
   it('intializes', () => {
@@ -30,29 +26,63 @@ describe('createClient', () => {
 
   describe('get()', () => {
     it('createdClient getRequest creates full url from relative endpoint', async () => {
+      nock('https://example.com:443')
+        .defaultReplyHeaders({ 'access-control-allow-origin': '*' })
+        .get('/api/v2/foo')
+        .reply(200);
+
       const client = createClient('https://example.com/api/v2');
       const response = await client.get('foo');
 
-      expect(response.data).toEqual({ url: 'https://example.com/api/v2/foo' });
+      expect(response.rawResponse.xhr.responseURL).toEqual('https://example.com/api/v2/foo');
     });
 
     it('does not double leading /', async () => {
+      nock('https://example.com:443')
+        .defaultReplyHeaders({ 'access-control-allow-origin': '*' })
+        .get('/api/v2/foo')
+        .reply(200, { url: 'https://example.com/api/v2/foo' });
+
       const client = createClient('https://example.com/api/v2');
       const response = await client.get('/foo');
 
-      expect(response.data).toEqual({ url: 'https://example.com/api/v2/foo' });
+      expect(response.rawResponse.xhr.responseURL).toEqual('https://example.com/api/v2/foo');
     });
 
     it('does not modify full urls', async () => {
+      const gitHubScope = nock('https://github.com')
+        .defaultReplyHeaders({ 'access-control-allow-origin': '*' })
+        .get('/users')
+        .reply(200, { url: 'https://github.com/users' });
+
+      const exampleScope = nock('https://example.com')
+        .defaultReplyHeaders({ 'access-control-allow-origin': '*' })
+        .get('/api/v2/users')
+        .reply(404, { url: 'https://exmaple.com/api/v2/users' });
+
       const client = createClient('https://example.com/api/v2');
       const response = await client.get('https://github.com/users');
 
+      expect(gitHubScope.isDone()).toBe(true);
+      expect(exampleScope.isDone()).toBe(false);
+      expect(response.ok).toBe(true);
+      expect(response.status).toEqual(200);
       expect(response.data).toEqual({ url: 'https://github.com/users' });
     });
 
     it('rejects when when an error occurs', async () => {
+      nock('https://example.com')
+        .defaultReplyHeaders({ 'access-control-allow-origin': '*' })
+        .get('/api/v2/404')
+        .reply(404, { message: 'Not Found' });
+
       const client = createClient('https://example.com/api/v2');
-      expect(client.get('/404')).rejects.toThrow();
+      const response = await client.get('/404');
+
+      expect(response.ok).toBe(false);
+      expect(response.data).toEqual({ message: 'Not Found' });
+      expect(response.status).toEqual(404);
+      expect(response.statusText).toEqual('Not Found');
     });
   });
 });

src/client/createClient.spec.js

@@ -1,16 +1,15 @@
 import { ajax } from 'rxjs/ajax';
 import { catchError, map } from 'rxjs/operators';
-import { throwError, of } from 'rxjs';
+import { of } from 'rxjs';
 
 const getRequest = path =>
   ajax(path).pipe(
     map(response => {
-      if (response === null) {
-        return throwError('API Timed out', response);
-      }
+      response.ok = true;
       return response;
     }),
     catchError(error => {
+      error.ok = false;
       return of(error);
     }),
   );

src/client/getRequest.js

@@ -0,0 +1,29 @@
+import nock from 'nock';
+
+import getRequest from './getRequest';
+
+describe('getRequest', () => {
+  it('returns response', async () => {
+    const data = { foo: 'bar' };
+    nock('http://localhost')
+      .get('/foo')
+      .reply(200, data);
+
+    const response = await getRequest('http://localhost/foo').toPromise();
+    expect(response.ok).toBe(true);
+    expect(response.response).toEqual(data);
+    expect(response.status).toBe(200);
+  });
+
+  it('returns error', async () => {
+    const data = { message: 'Error' };
+    nock('http://localhost')
+      .get('/foo')
+      .reply(500, data);
+
+    const response = await getRequest('http://localhost/foo').toPromise();
+    expect(response.ok).toBe(false);
+    expect(response.response).toEqual(data);
+    expect(response.status).toBe(500);
+  });
+});

src/client/getRequest.spec.js

@@ -18,13 +18,9 @@ export default function Dashboard() {
   const [users, loading, error] = useEndpointData(
     `https://api.github.com/usersdsafasdfa?per_page=5`,
   );
+
   return (
-    <Box
-      display="flex"
-      flexDirection="row"
-      justifyContent="space-between"
-      flexWrap="wrap"
-    >
+    <Box display="flex" flexDirection="row" justifyContent="space-between" flexWrap="wrap">
       <Box width={{ xs: '100%', md: 350 }} m={2}>
         <UsersCard />
       </Box>

src/components/Dashboard.jsx

@@ -13,7 +13,8 @@ export default function ReposList({ repos, isLoading, error }) {
   }
 
   if (error || !Array.isArray(repos)) {
-    return <Banner variant="error" message="An error occured loading repos." />;
+    const message = (error || {}).statusText || 'An unknown error occured loading users. ';
+    return <Banner variant="error" message={message} />;
   }
 
   return (

src/components/ReposCard/ReposList.jsx

@@ -15,7 +15,8 @@ export default function UsersList({ users, isLoading, error }) {
   }
 
   if (error || !Array.isArray(users)) {
-    return <Banner variant="error" message="An error occured loading users." />;
+    const message = (error || {}).statusText || 'An unknown error occured loading users. ';
+    return <Banner variant="error" message={message} />;
   }
 
   return (

src/components/UsersCard/UsersList.jsx

@@ -75,7 +75,12 @@ export default function useEndpointData(path, options) {
       setLoading(true);
       try {
         const response = await client.get(currentEndpoint);
-        setValue(response.data);
+
+        if (response.ok) {
+          setValue(response.data);
+        } else {
+          setError(response);
+        }
 
         // If we aren't a paged response, then these will be null
         setNextLink(response.links.next);
@@ -90,11 +95,5 @@ export default function useEndpointData(path, options) {
     };
     load();
   }, [path, currentEndpoint, client]);
-  return [
-    value,
-    isLoading,
-    error,
-    { getNext, getPrevious, getFirst, getLast },
-    client,
-  ];
+  return [value, isLoading, error, { getNext, getPrevious, getFirst, getLast }, client];
 }