Ensure error handling logic is displaying the issue on the UI

Author: benoit74

api/src/routes/errors.py

@@ -67,44 +67,40 @@ 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, status: HTTPStatus):
+        if isinstance(e, ExceptionWithMessage) and e.message is not None:
+            return make_response(jsonify({"error": e.message}), status)
+        return Response(status=status)
+
+
+# 400
+class BadRequest(ExceptionWithMessage):
     @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)
+        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)

api/src/routes/requests.py

@@ -129,7 +129,15 @@ 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}")
-            raise InternalError(f"Unable to create schedule via HTTP {status}: {resp}")
+            message = f"Unable to create schedule via HTTP {status}: {resp}"
+            if status == http.HTTPStatus.BAD_REQUEST:
+                # if Zimfarm replied this is a bad request, then this is most probably
+                # a bad request due to user input so we can track it like a bad request
+                raise InternalError()
+                # raise BadRequest(message)
+            else:
+                # otherwise, this is most probably an internal problem in our systems
+                raise InternalError(message)
 
         # request a task for that newly created schedule
         success, status, resp = query_api(
@@ -138,23 +146,22 @@ def post(self, *args, **kwargs):
             payload={"schedule_names": [schedule_name], "worker": TASK_WORKER},
         )
         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}")
+            logger.error(f"Unable to request {schedule_name} 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}")
 
         # remove newly created schedule (not needed anymore)
         success, status, resp = query_api("DELETE", f"/schedules/{schedule_name}")
         if not success:
-            logger.error(f"Unable to remove schedule {schedule_name} via HTTP {status}")
-            logger.debug(resp)
-
+            logger.error(f"Unable to remove schedule {schedule_name} via HTTP {status}: {resp}")
         return make_response(jsonify({"id": str(task_id)}), http.HTTPStatus.CREATED)
 
 

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);

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 due to CORS issue
+      return "Unknown response; probably CORS issue."
     }
-    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 + ".";
   },
 };