Extend error checking to API response codes

recipe-server/client/control/tests/utils/test_handleError.js

@@ -22,23 +22,64 @@ describe('handleError util', () => {
   });
 
   it('should detect form validation errors', () => {
-    const err = new APIClient.APIError('Server Error.', { field: 'Validation message. ' });
-
-    const { context, message, reason } = handleError('Test.', err);
+    const { context, message, reason } = handleError('Test.', { field: 'Validation message.' });
     expect(context).toBe('Test.');
     expect(message).toBe(`Test. ${ERR_MESSAGES.FORM_VALIDATION}`);
     expect(reason).toBe(ERR_MESSAGES.FORM_VALIDATION);
   });
 
-  it('should fall back to server messages if no form validation errors', () => {
-    const err = new APIClient.APIError('Something from the server.');
+  describe('API Errors', () => {
+    it('should handle 400 errors', () => {
+      const err = new APIClient.APIError('Something from the server.', { status: 400 });
 
-    const { context, message, reason } = handleError('Test.', err);
-    expect(context).toBe('Test.');
-    expect(message).toBe('Test. Something from the server.');
-    expect(reason).toBe('Something from the server.');
+      const { context, message, reason } = handleError('Test.', err);
+      expect(context).toBe('Test.');
+      expect(message).toBe(`Test. ${ERR_MESSAGES.FORM_VALIDATION}`);
+      expect(reason).toBe(ERR_MESSAGES.FORM_VALIDATION);
+    });
+
+    describe('should handle 403 errors', () => {
+      it('should handle a "not logged in" 403 error', () => {
+        const err = new APIClient.APIError('Authentication credentials were not provided',
+          { status: 403 });
+
+        const { context, message, reason } = handleError('Test.', err);
+        expect(context).toBe('Test.');
+        expect(message).toBe(`Test. ${ERR_MESSAGES.NOT_LOGGED_IN}`);
+        expect(reason).toBe(ERR_MESSAGES.NOT_LOGGED_IN);
+      });
+
+      it('should handle a "no permission" 403 error', () => {
+        const err = new APIClient.APIError('User does not have permission to perform that action.',
+          { status: 403 });
+
+        const { context, message, reason } = handleError('Test.', err);
+        expect(context).toBe('Test.');
+        expect(message).toBe(`Test. ${ERR_MESSAGES.NO_PERMISSION}`);
+        expect(reason).toBe(ERR_MESSAGES.NO_PERMISSION);
+      });
+    });
+
+    it('should handle 500 errors', () => {
+      const err = new APIClient.APIError('Something from the server.', { status: 500 });
+
+      const { context, message, reason } = handleError('Test.', err);
+      expect(context).toBe('Test.');
+      expect(message).toBe(`Test. ${ERR_MESSAGES.SERVER_FAILED} (Something from the server.)`);
+      expect(reason).toBe(`${ERR_MESSAGES.SERVER_FAILED} (Something from the server.)`);
+    });
+
+    it('should fall back to server messages if the response status is unrecognized', () => {
+      const err = new APIClient.APIError('Something from the server.', { status: 123 });
+
+      const { context, message, reason } = handleError('Test.', err);
+      expect(context).toBe('Test.');
+      expect(message).toBe('Test. Something from the server.');
+      expect(reason).toBe('Something from the server.');
+    });
   });
 
+
   it('should detect when a user is offline', () => {
     const { context, message, reason } = handleError('Test.', new Error(), {
       checkUserOnline: () => false,

recipe-server/client/control/utils/api.js

@@ -54,7 +54,7 @@ export default class APIClient {
     // Throw if we get a non-200 response.
     if (!response.ok) {
       let message;
-      let data;
+      let data = {};
       let err;
 
       try {
@@ -65,6 +65,8 @@ export default class APIClient {
         err = error;
       }
 
+      data = { ...data, status: response.status };
+
       throw new APIClient.APIError(message, data, err);
     }
 

recipe-server/client/control/utils/handleError.js

@@ -1,4 +1,4 @@
-import { message } from 'antd';
+import { message as AntMessage } from 'antd';
 
 import APIClient from 'control/utils/api';
 
@@ -7,29 +7,65 @@ export const ERR_MESSAGES = {
   FORM_VALIDATION: 'Please correct the form items highlighted below.',
   NO_INTERNET: 'Check your internet connection and try again.',
   SERVER_FAILED: 'The server failed to respond. Please try again.',
+  NOT_LOGGED_IN: 'No active user session found, try logging in again.',
+  NO_PERMISSION: 'You do not have permission to perform that action.',
 };
 
-// String to search for to determine if a fetch failed. Unlikely to change.
-const fetchFailureMessage = 'Failed to fetch';
+// Search strings used to determine various types responses.
+const checkFetchFailure = ({ message = '' }) =>
+  message.indexOf('Failed to fetch') !== -1 || message.indexOf('NetworkError') !== -1;
+
+const checkLoginFailure = ({ message = '' }) => message.indexOf('credentials were not provided') > -1;
+const checkAPIFailure = error => error instanceof APIClient.APIError;
+
 
 const defaultMethods = {
   checkUserOnline: () => navigator.onLine,
-  notifyUser: errMsg => message.error(errMsg),
+  notifyUser: errMsg => AntMessage.error(errMsg, 10),
+};
+
+const handleAPIError = error => {
+  let message = '';
+
+  switch (error.data.status) {
+    case 400: // Bad Request
+      message = ERR_MESSAGES.FORM_VALIDATION;
+      break;
+
+    case 403: // Forbidden
+      message = checkLoginFailure(error) ? ERR_MESSAGES.NOT_LOGGED_IN : ERR_MESSAGES.NO_PERMISSION;
+      break;
+
+    case 500: // Internal Server Error
+      message = `${ERR_MESSAGES.SERVER_FAILED} (${error.message})`;
+      break;
+
+    // If it's a status we aren't expecting, simply pass the server error
+    // forward to the client.
+    default:
+      message = error.message;
+      break;
+  }
+
+  return message;
 };
 
 export default function handleError(context = 'Error!', error, methodOverrides = {}) {
   const methods = { ...defaultMethods, ...methodOverrides };
   let errMsg = '';
 
+  // This function can be used to trigger an error message without an actual Error
+  // object. If an Error is given, though, we can extract a more meaningful error message.
   if (error) {
+    errMsg = error.message || '';
     if (!methods.checkUserOnline()) {
       errMsg = ERR_MESSAGES.NO_INTERNET;
-    } else if (error.message.indexOf(fetchFailureMessage) > -1) {
+    } else if (checkFetchFailure(error)) {
       errMsg = ERR_MESSAGES.SERVER_FAILED;
-    } else if (error instanceof APIClient.APIError && error.data) {
+    } else if (checkAPIFailure(error)) {
+      errMsg = handleAPIError(error);
+    } else if (!(error instanceof Error)) {
       errMsg = ERR_MESSAGES.FORM_VALIDATION;
-    } else {
-      errMsg = error.message;
     }
   }