From b824b4fc67c92df4f35edf03d47c765ce21c7e60 Mon Sep 17 00:00:00 2001 From: benoit74 Date: Thu, 23 Nov 2023 14:15:10 +0100 Subject: [PATCH] Ensure error handling logic is displaying the issue on the UI --- api/src/routes/errors.py | 40 ++++++++++++++------------------ api/src/routes/requests.py | 14 ++++++++--- ui/src/components/NewRequest.vue | 2 +- ui/src/constants.js | 19 +++++++-------- 4 files changed, 38 insertions(+), 37 deletions(-) diff --git a/api/src/routes/errors.py b/api/src/routes/errors.py index 861f5bf..afa65da 100644 --- 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 index 704f651..5a2b4d4 100644 --- a/api/src/routes/requests.py +++ b/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}") + logger.debug(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 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( @@ -140,12 +148,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 index 468a811..01fa8d7 100644 --- 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:
" + Constants.standardHTTPError(error.response)); }) .then(function () { parent.toggleLoader(false); diff --git a/ui/src/constants.js b/ui/src/constants.js index 7c0f205..3c31d6b 100644 --- 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 += "
" + JSON.stringify(response.data.error); - if (response.data && response.data.error_description) - status_text += "
" + JSON.stringify(response.data.error_description); - if (response.data && response.data.message) - status_text += "
" + 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 + "."; }, };