Ensure error handling logic is displaying the issue on the UI

openzim · Nov 23, 2023 · ec9852a · ec9852a
1 parent 83960b0
commit ec9852a
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 36 deletions.
diff --git a/api/src/routes/errors.py b/api/src/routes/errors.py
@@ -66,45 +66,41 @@ def register_handlers(app: Flask):
     def handler_validationerror(e):
         return make_response(jsonify({"message": e.messages}), HTTPStatus.BAD_REQUEST)
 
-
-# 400
-class BadRequest(Exception):
+class ExceptionWithMessage(Exception):
     def __init__(self, message: str = None):
         self.message = message
 
     @staticmethod
-    def handler(e):
-        if isinstance(e, BadRequest) and e.message is not None:
-            return make_response(jsonify({"error": e.message}), HTTPStatus.BAD_REQUEST)
-        return Response(status=HTTPStatus.BAD_REQUEST)
+    def handler(e, status: HTTPStatus):
+        if isinstance(e, ExceptionWithMessage) and e.message is not None:
+            return make_response(jsonify({"error": e.message}), status)
+        return make_response(status=status)
 
+# 400
+class BadRequest(ExceptionWithMessage):
+
+    @staticmethod
+    def handler(e):
+        return super().handler(e, HTTPStatus.BAD_REQUEST)
 
 # 401
-class Unauthorized(Exception):
-    def __init__(self, message: str = None):
-        self.message = message
+class Unauthorized(ExceptionWithMessage):
 
     @staticmethod
     def handler(e):
-        if isinstance(e, Unauthorized) and e.message is not None:
-            return make_response(jsonify({"error": e.message}), HTTPStatus.UNAUTHORIZED)
-        return Response(status=HTTPStatus.UNAUTHORIZED)
+        return super().handler(e, HTTPStatus.UNAUTHORIZED)
 
 
 # 404
-class NotFound(Exception):
-    def __init__(self, message: str = None):
-        self.message = message
+class NotFound(ExceptionWithMessage):
 
     @staticmethod
     def handler(e):
-        if isinstance(e, NotFound) and e.message is not None:
-            return make_response(jsonify({"error": e.message}), HTTPStatus.NOT_FOUND)
-        return Response(status=HTTPStatus.NOT_FOUND)
-
+        return super().handler(e, HTTPStatus.NOT_FOUND)
 
 # 500
-class InternalError(Exception):
+class InternalError(ExceptionWithMessage):
+
     @staticmethod
     def handler(e):
-        return Response(status=HTTPStatus.INTERNAL_SERVER_ERROR)
+        return super().handler(e, HTTPStatus.INTERNAL_SERVER_ERROR)
diff --git a/api/src/routes/requests.py b/api/src/routes/requests.py
@@ -129,6 +129,7 @@ def post(self, *args, **kwargs):
         success, status, resp = query_api("POST", "/schedules/", payload=payload)
         if not success:
             logger.error(f"Unable to create schedule via HTTP {status}: {resp}")
+            logger.debug(resp)
             raise InternalError(f"Unable to create schedule via HTTP {status}: {resp}")
 
         # request a task for that newly created schedule
@@ -140,12 +141,12 @@ def post(self, *args, **kwargs):
         if not success:
             logger.error(f"Unable to request {schedule_name} via HTTP {status}")
             logger.debug(resp)
-            raise InternalError(f"Unable to request schedule via HTTP {status}: {resp}")
+            raise InternalError(f"Unable to request schedule via HTTP {status}): {resp}")
 
         try:
             task_id = resp.get("requested").pop()
             if not task_id:
-                raise ValueError("task_id is False")
+                raise InternalError("task_id is False")
         except Exception as exc:
             raise InternalError(f"Couldn't retrieve requested task id: {exc}")
 

diff --git a/ui/src/components/NewRequest.vue b/ui/src/components/NewRequest.vue
@@ -233,7 +233,7 @@
                   throw "Didn't receive task_id";
               })
               .catch(function (error) {
-                parent.alertError("Unable to create schedule:\n" + Constants.standardHTTPError(error.response));
+                parent.alertError("Unable to request ZIM creation:<br />" + Constants.standardHTTPError(error.response));
               })
               .then(function () {
                 parent.toggleLoader(false);

diff --git a/ui/src/constants.js b/ui/src/constants.js
@@ -98,19 +98,16 @@ export default {
       599: "Network Connect Timeout Error",
     };
 
-    if (response === undefined) { // no response
-                                  //usually due to browser blocking failed OPTION preflight request
-      return "Cross-Origin Request Blocked: preflight request failed."
+    if (response === undefined) {
+      // no response is usually due to browser blocking failed OPTION preflight request
+      return "Unknown error, no response ; probably Cross-Origin Request Blocked."
     }
-    let status_text = response.statusText ? response.statusText : statuses[response.status];
-    if (response.status == 400) {
-      if (response.data && response.data.error)
-        status_text += "<br />" + JSON.stringify(response.data.error);
-      if (response.data && response.data.error_description)
-        status_text += "<br />" + JSON.stringify(response.data.error_description);
-      if (response.data && response.data.message)
-        status_text += "<br />" + JSON.stringify(response.data.message);
+    // If error is provided, display it (do not display error code since this is too technical)
+    if (response.data && response.data.error) {
+      return response.data.error;
     }
+    // Last resort, display only available information
+    let status_text = response.statusText ? response.statusText : statuses[response.status];
     return response.status + ": " + status_text + ".";
   },
 };