From 91e988ff119b7b1615888ec9d6d4eac2f1833d14 Mon Sep 17 00:00:00 2001
From: jasquat <2487833+jasquat@users.noreply.github.com>
Date: Thu, 18 Apr 2024 18:11:44 +0000
Subject: [PATCH] better-fe-error-handling (#1406)

* update error handling in HttpService to handle non-json responses better w/ burnettk

* fixed cypress tests

---------

Co-authored-by: jasquat <jasquat@users.noreply.github.com>
---
 .../src/services/HttpService.ts               | 54 +++++++++++--------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/spiffworkflow-frontend/src/services/HttpService.ts b/spiffworkflow-frontend/src/services/HttpService.ts
index 86c6485d..f2a0663d 100644
--- a/spiffworkflow-frontend/src/services/HttpService.ts
+++ b/spiffworkflow-frontend/src/services/HttpService.ts
@@ -1,3 +1,4 @@
+/* eslint-disable max-classes-per-file */
 import { BACKEND_BASE_URL } from '../config';
 import { objectIsEmpty } from '../helpers';
 import UserService from './UserService';
@@ -36,6 +37,13 @@ export class UnauthenticatedError extends Error {
   }
 }
 
+export class UnexpectedResponseError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'UnexpectedResponseError';
+  }
+}
+
 const makeCallToBackend = ({
   path,
   successCallback,
@@ -74,47 +82,51 @@ backendCallProps) => {
 
   const updatedPath = path.replace(/^\/v1\.0/, '');
 
-  let isSuccessful = true;
-  // this fancy 403 handling is like this because we want to get the response body.
-  // otherwise, we would just throw an exception.
-  let is403 = false;
   fetch(`${BACKEND_BASE_URL}${updatedPath}`, httpArgs)
     .then((response) => {
       if (response.status === 401) {
         throw new UnauthenticatedError('You must be authenticated to do this.');
-      } else if (response.status === 403) {
-        is403 = true;
-        isSuccessful = false;
-      } else if (!response.ok) {
-        isSuccessful = false;
       }
-      return response.json();
+      return response.text().then((result: any) => {
+        return { response, text: result };
+      });
     })
     .then((result: any) => {
-      if (isSuccessful) {
-        successCallback(result);
-      } else if (is403) {
+      let jsonResult = null;
+      try {
+        jsonResult = JSON.parse(result.text);
+      } catch (error) {
+        if (error instanceof SyntaxError) {
+          throw new UnexpectedResponseError(
+            `Received unexpected response from server. Status: ${result.response.status}: ${result.response.statusText}. Body: ${result.text}`
+          );
+        }
+        throw error;
+      }
+      if (result.response.status === 403) {
         if (onUnauthorized) {
-          onUnauthorized(result);
+          onUnauthorized(jsonResult);
         } else if (UserService.isPublicUser()) {
           window.location.href = '/public/sign-out';
         } else {
           // Hopefully we can make this service a hook and use the error message context directly
           // eslint-disable-next-line no-alert
-          alert(result.message);
-        }
-      } else {
-        let message = 'A server error occurred.';
-        if (result.message) {
-          message = result.message;
+          alert(jsonResult.message);
         }
+      } else if (!result.response.ok) {
         if (failureCallback) {
-          failureCallback(result);
+          failureCallback(jsonResult);
         } else {
+          let message = 'A server error occurred.';
+          if (jsonResult.message) {
+            message = jsonResult.message;
+          }
           console.error(message);
           // eslint-disable-next-line no-alert
           alert(message);
         }
+      } else {
+        successCallback(jsonResult);
       }
     })
     .catch((error) => {