Improve fetch api handler to reject non-ok responses

jsapp/js/account/security/password/updatePasswordForm.component.tsx

@@ -8,6 +8,7 @@ import Button from 'js/components/common/button';
 import {fetchPatch} from 'js/api';
 import {endpoints} from 'js/api.endpoints';
 import {notify} from 'js/utils';
+import type {FailResponse} from 'js/dataInterface';
 
 const FIELD_REQUIRED_ERROR = t('This field is required.');
 
@@ -48,22 +49,13 @@ export default function UpdatePasswordForm() {
 
     // Verify password input must match the new password
     if (newPassword !== verifyPassword) {
-      setNewPasswordError(
-        t('This field must match the Verify Password field.')
-      );
+      setVerifyPasswordError(t("Passwords don't match"));
       hasErrors = true;
     }
 
     if (!hasErrors) {
       setIsPending(true);
 
-      // TODO: Handle error cases (such as 400 bad request)
-      //
-      // Currently this shows "changed password successfully" if the network
-      // succeeds and it gets any response from the server, even a 400.
-      //
-      // It needs to handle the case where a user types the wrong "current"
-      // password, or if the server rejects the update for some other reason.
       try {
         await fetchPatch(endpoints.ME_URL, {
           current_password: currentPassword,
@@ -75,18 +67,32 @@ export default function UpdatePasswordForm() {
         setVerifyPassword('');
         notify(t('changed password successfully'));
       } catch (error) {
+        const errorObj = error as FailResponse;
+
+        if (errorObj.responseJSON?.current_password) {
+          setCurrentPasswordError(errorObj.responseJSON.current_password[0]);
+        }
+        if (errorObj.responseJSON?.new_password) {
+          setNewPasswordError(errorObj.responseJSON.new_password[0]);
+        }
+
         setIsPending(false);
         notify(t('failed to change password'), 'error');
       }
     }
   }
 
+  function submitPasswordForm(evt: React.FormEvent<HTMLFormElement>) {
+    evt.preventDefault();
+    savePassword();
+  }
+
   if (!sessionStore.isLoggedIn) {
     return null;
   }
 
   return (
-    <form className={styles.foo}>
+    <form className={styles.root} onSubmit={submitPasswordForm}>
       <div className={styles.row}>
         <TextBox
           customModifiers='on-white'
@@ -134,8 +140,8 @@ export default function UpdatePasswordForm() {
           type='full'
           color='blue'
           size='m'
-          onClick={savePassword}
           label={t('Save Password')}
+          isSubmit
           isPending={isPending}
         />
       </div>

jsapp/js/api.ts

@@ -1,11 +1,21 @@
 // Thin kobo api wrapper around fetch
 import {ROOT_URL} from './constants';
 import type {Json} from './components/common/common.interfaces';
+import type {FailResponse} from 'js/dataInterface';
+import {notify} from 'js/utils';
 
 const JSON_HEADER = 'application/json';
 
-// TODO: This needs a way to check for errors such as 400, 500.
-// https://stackoverflow.com/questions/39297345/fetch-resolves-even-if-404
+// TODO: Figure out how to improve UX if there are many errors happening
+// simultaneously (other than deciding not to show them.)
+//
+// const notifyServerErrorThrottled = throttle(
+//   (errorMessage: string) => {
+//     notify(errorMessage, 'error');
+//   },
+//   500 // half second
+// );
+
 const fetchData = async <T>(path: string, method = 'GET', data?: Json) => {
   const headers: {[key: string]: string} = {
     Accept: JSON_HEADER,
@@ -25,7 +35,50 @@ const fetchData = async <T>(path: string, method = 'GET', data?: Json) => {
     headers,
     body: JSON.stringify(data),
   });
+
   const contentType = response.headers.get('content-type');
+
+  if (!response.ok) {
+    // This will be returned with the promise rejection. It can include that
+    // response JSON, but not all endpoints/situations will produce one.
+    const failResponse: FailResponse = {
+      status: response.status,
+      statusText: response.statusText,
+    };
+
+    // For these codes, reject the promise, and display a toast with HTTP status
+    if (
+      response.status === 401 ||
+      response.status === 403 ||
+      response.status === 404 ||
+      response.status >= 500
+    ) {
+      let errorMessage = t('An error occurred');
+      errorMessage += ' — ';
+      errorMessage += response.status;
+      errorMessage += ' ';
+      errorMessage += response.statusText;
+      notify(errorMessage, 'error');
+
+      if (window.Raven) {
+        window.Raven.captureMessage(errorMessage);
+      }
+      return Promise.reject(failResponse);
+    }
+
+    if (contentType && contentType.indexOf('application/json') !== -1) {
+      failResponse.responseText = await response.text();
+      try {
+        failResponse.responseJSON = JSON.parse(failResponse.responseText);
+      } catch {
+        // If the response text is not a proper JSON, we simply don't add it to
+        // the rejection object.
+      }
+    }
+
+    return Promise.reject(failResponse);
+  }
+
   if (contentType && contentType.indexOf('application/json') !== -1) {
     return (await response.json()) as Promise<T>;
   }

jsapp/js/components/common/button.tsx

@@ -62,7 +62,8 @@ interface ButtonProps {
   isFullWidth?: boolean;
   /** Additional class names. */
   classNames?: string[];
-  onClick: (event: any) => void;
+  /** You don't need to pass the callback for `isSubmit` option. */
+  onClick?: (event: any) => void;
   'data-cy'?: string;
 }
 

jsapp/js/components/languages/languagesListStore.ts

@@ -3,7 +3,7 @@ import type {
   PaginatedResponse,
   FailResponse,
 } from 'js/dataInterface';
-import {notify} from 'js/utils';
+import {handleApiFail} from 'js/utils';
 import {ROOT_URL} from 'js/constants';
 import languagesStore from './languagesStore';
 import type {ListLanguage} from './languagesStore';
@@ -56,7 +56,7 @@ export default class LanguagesListStore {
 
   private onAnyFail(response: FailResponse) {
     this.isLoading = false;
-    notify(response.responseText, 'error');
+    handleApiFail(response);
   }
 
   /** Gets the next page of results (if available). */

jsapp/js/components/reports/reports.es6

@@ -464,7 +464,7 @@ export default class Reports extends React.Component {
             <br />
             <code>
               {this.state.error.statusText}
-              {': ' + this.state.error.responseText}
+              {': ' + this.state.error.responseText || t('An error occurred')}
             </code>
           </bem.Loading__inner>
         </bem.Loading>

jsapp/js/components/support/helpBubbleStore.ts

@@ -1,7 +1,7 @@
 import throttle from 'lodash.throttle';
 import {makeAutoObservable, when} from 'mobx';
 import type {PaginatedResponse, FailResponse} from 'js/dataInterface';
-import {notify} from 'js/utils';
+import {handleApiFail} from 'js/utils';
 import {ROOT_URL} from 'js/constants';
 import sessionStore from 'js/stores/session';
 
@@ -85,7 +85,7 @@ class HelpBubbleStore {
 
   private onFetchMessagesFail(response: FailResponse) {
     this.isLoading = false;
-    notify(response.responseText, 'error');
+    handleApiFail(response);
   }
 
   public selectMessage(messageUid: string) {
@@ -150,7 +150,7 @@ class HelpBubbleStore {
   }
 
   private onPatchMessageFail(response: FailResponse) {
-    notify(response.responseText, 'error');
+    handleApiFail(response);
   }
 }
 

jsapp/js/dataInterface.ts

@@ -96,15 +96,26 @@ export interface ImportResponse {
 }
 
 export interface FailResponse {
+  /**
+   * This is coming from Back end and can have either the general `detail` or
+   * `error`, or a list of specific errors (e.g. for specific fields).
+   */
   responseJSON?: {
     detail?: string;
     error?: string;
+    [fieldName: string]: string[] | string | undefined;
   };
-  responseText: string;
+  responseText?: string;
   status: number;
   statusText: string;
 }
 
+/** Have a list of errors for different fields. */
+export interface PasswordUpdateFailResponse {
+  current_password: string[];
+  new_password: string[];
+}
+
 interface ProcessingResponseData {
   [questionName: string]: any;
   _id: number;

jsapp/js/stores.es6

@@ -24,6 +24,7 @@ import {
   notify,
   assign,
 } from 'utils';
+import { toast } from 'react-hot-toast';
 import {ANON_USERNAME} from 'js/constants';
 
 const cookies = new Cookies();
@@ -290,13 +291,20 @@ stores.allAssets = Reflux.createStore({
     }
   },
   onListAssetsCompleted: function(searchData, response) {
+    toast.dismiss('query_too_short');
     response.results.forEach(this.registerAsset);
     this.data = response.results;
     this.trigger(this.data);
   },
   onListAssetsFailed: function (searchData, response) {
-    notify(response?.responseJSON?.detail || t('failed to list assets'));
-  }
+    let iconStyle = 'warning';
+    let opts = {};
+    if (response?.responseJSON?.detail === t('Your query is too short')) {
+      iconStyle = 'empty';
+      opts.id = 'query_too_short'; // de-dupe and make dismissable on success
+    }
+    notify(response?.responseJSON?.detail || t('failed to list assets'), iconStyle, opts);
+  },
 });
 
 stores.selectedAsset = Reflux.createStore({

jsapp/js/utils.ts

@@ -14,6 +14,7 @@ import {Cookies} from 'react-cookie';
 // importing whole constants, as we override ROOT_URL in tests
 import constants from 'js/constants';
 import type {FailResponse} from './dataInterface';
+import type Raven from 'raven';
 
 export const LANGUAGE_COOKIE_NAME = 'django_language';
 
@@ -24,6 +25,8 @@ const cookies = new Cookies();
 /**
  * Pop up a notification with react-hot-toast
  * Some default options are set in the <Toaster/> component
+ *
+ * Also log messages to browser console to help with debugging.
  */
 export function notify(msg: Toast['message'], atype = 'success', opts?: ToastOptions): Toast['id'] {
   // To avoid changing too much, the default remains 'success' if unspecified.
@@ -41,26 +44,33 @@ export function notify(msg: Toast['message'], atype = 'success', opts?: ToastOpt
     }
   }
 
+  /* eslint-disable no-console */
   switch (atype) {
 
     case 'success':
+      console.log('[notify] ✅ ' + msg);
       return toast.success(msg, opts);
 
     case 'error':
+      console.error('[notify] ❌ ' + msg);
       return toast.error(msg, opts);
 
     case 'warning':
+      console.warn('[notify] ⚠️ ' + msg);
       return toast(msg, Object.assign({icon: '⚠️'}, opts));
 
     case 'empty':
+      console.log('[notify] 📢 ' + msg);
       return toast(msg, opts); // No icon
 
     // Defensively render empty if we're passed an unknown atype,
     // in case we missed something.
     //   e.g. notify('mystery!', '?') //
     default:
+      console.log('[notify] 📢 ' + msg);
       return toast(msg, opts); // No icon
   }
+  /* eslint-enable no-console */
 }
 
 // Convenience functions for code readability, consolidated here
@@ -95,7 +105,8 @@ export function handleApiFail(response: FailResponse) {
   }
 
   if (!message) {
-    message = `An unexpected error occurred ${response.status} ${response.statusText}`;
+    message = t('An error occurred');
+    message += ` — ${response.status} ${response.statusText}`;
   }
 
   notify.error(message);
@@ -199,11 +210,13 @@ export function buildUserUrl(username: string): string {
 declare global {
   interface Window {
     log: () => void;
+    Raven?: Raven.Client;
   }
 }
 
 export const log = (function () {
   const innerLogFn = function (...args: any[]) {
+    // eslint-disable-next-line no-console
     console.log.apply(console, args);
     return args[0];
   };

package.json

@@ -100,6 +100,7 @@
     "@types/lodash.zip": "^4.2.7",
     "@types/mocha": "^9.1.1",
     "@types/qrcode.react": "^1.0.2",
+    "@types/raven": "^2.5.4",
     "@types/react": "^16.14.13",
     "@types/react-document-title": "^2.0.5",
     "@types/react-dom": "^16.9.14",