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,11 @@
 // 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
 const fetchData = async <T>(path: string, method = 'GET', data?: Json) => {
   const headers: {[key: string]: string} = {
     Accept: JSON_HEADER,
@@ -25,7 +25,42 @@ const fetchData = async <T>(path: string, method = 'GET', data?: Json) => {
     headers,
     body: JSON.stringify(data),
   });
+
   const contentType = response.headers.get('content-type');
+
+  // For server issues, we simply reject with no useful data. We expect the UI
+  // to not react with any notification or some other indicator.
+  if (
+    response.status === 401 ||
+    response.status === 403 ||
+    response.status === 404 ||
+    response.status >= 500
+  ) {
+    notify(t('Server error'));
+    return Promise.reject();
+  }
+
+  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,
+    };
+
+    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

@@ -56,7 +56,7 @@ export default class LanguagesListStore {
 
   private onAnyFail(response: FailResponse) {
     this.isLoading = false;
-    notify(response.responseText, 'error');
+    notify(response.responseText || t('Unknown error'), 'error');
   }
 
   /** 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('Unknown error')}
             </code>
           </bem.Loading__inner>
         </bem.Loading>

jsapp/js/components/support/helpBubbleStore.ts

@@ -85,7 +85,7 @@ class HelpBubbleStore {
 
   private onFetchMessagesFail(response: FailResponse) {
     this.isLoading = false;
-    notify(response.responseText, 'error');
+    notify(response.responseText || t('Unknown error'), 'error');
   }
 
   public selectMessage(messageUid: string) {
@@ -150,7 +150,7 @@ class HelpBubbleStore {
   }
 
   private onPatchMessageFail(response: FailResponse) {
-    notify(response.responseText, 'error');
+    notify(response.responseText || t('Unknown error'), 'error');
   }
 }
 

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;