Merge pull request #377 from kikkomep/CU-861nbh2vt_Improve-handling-o…

…f-nonexistent-workflow-version-submission

feat: improve error handling

lifemonitor/api/controllers.py

@@ -504,6 +504,8 @@ def process_workflows_post(body, _registry=None, _submitter_id=None,
         return lm_exceptions.report_problem(403, "Forbidden", extra_info={"exception": str(e)},
                                             detail=messages.not_authorized_registry_access.format(registry.name)
                                             if registry else messages.not_authorized_workflow_access)
+    except lm_exceptions.ROCrateNotFoundException as e:
+        return lm_exceptions.report_problem(404, "RO-Crate not found", detail=str(e))
     except lm_exceptions.WorkflowVersionConflictException:
         return lm_exceptions.report_problem(409, "Workflow version conflict",
                                             detail=messages.workflow_version_conflict

lifemonitor/api/models/registries/registry.py

@@ -227,6 +227,8 @@ def _requester(self, user, method: str, *args, **kwargs):
                     errors.append(str(e))
         if response.status_code == 401 or response.status_code == 403:
             raise lm_exceptions.NotAuthorizedException(details=response.content)
+        if response.status_code == 404:
+            raise lm_exceptions.ROCrateNotFoundException(details=response.content, resource=response.url)
         raise lm_exceptions.LifeMonitorException(errors=[str(e) for e in errors])
 
     def get_index(self, user: auth_models.User) -> List[RegistryWorkflow]:

lifemonitor/api/models/repositories/base.py

@@ -267,7 +267,7 @@ def generate_config(self, ignore_existing=False,
 
     def write_zip(self, target_path: str):
         if not self.metadata:
-            raise IllegalStateException(detail="Missing RO Crate metadata")
+            raise IllegalStateException(detail="Missing RO-Crate metadata")
         return self.metadata.write_zip(target_path)
 
     def write(self, target_path: str, overwrite: bool = False) -> None:

lifemonitor/errors.py

@@ -24,8 +24,6 @@
 
 from flask import Blueprint, escape, render_template, request, url_for
 
-from lifemonitor.utils import validate_url
-
 # Config a module level logger
 logger = logging.getLogger(__name__)
 
@@ -55,11 +53,23 @@ def parametric_page():
         return handle_500()
 
 
+def handle_error(e: Exception):
+    status = getattr(e, 'status', 500)
+    try:
+        handler = getattr(error_handlers, f"handle_{status}")
+        logger.debug(f"Handling error code: {status}")
+        return handler(e)
+    except ValueError as e:
+        if logger.isEnabledFor(logging.DEBUG):
+            logger.debug(f"Error handling error code: {e}")
+        return handle_500(e)
+
+
 @blueprint.route("/400")
 def handle_400(e: Exception = None, description: str = None):
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: Page not found",
+            "title": getattr(e, 'title', None) or "LifeMonitor: Page not found",
             "code": "404",
             "description": description if description
             else str(e) if e and logger.isEnabledFor(logging.DEBUG)
@@ -72,14 +82,15 @@ def handle_400(e: Exception = None, description: str = None):
 def handle_404(e: Exception = None):
     resource = request.args.get("resource", None, type=str)
     logger.debug(f"Resource not found: {resource}")
+    from lifemonitor.utils import validate_url
     if resource and not validate_url(resource):
         logger.error(f"Invalid URL: {resource}")
         return handle_400(description="Invalid URL")
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: Page not found",
+            "title": getattr(e, 'title', None) or "LifeMonitor: Page not found",
             "code": "404",
-            "description": str(e)
+            "description": getattr(e, 'detail', None) or str(e)
             if e and logger.isEnabledFor(logging.DEBUG)
             else "Page not found",
             "resource": resource,
@@ -91,13 +102,14 @@ def handle_404(e: Exception = None):
 def handle_405(e: Exception = None):
     resource = request.args.get("resource", None, type=str)
     logger.debug(f"Method not allowed for resource {resource}")
+    from lifemonitor.utils import validate_url
     if not validate_url(resource):
         return handle_400(decription="Invalid URL")
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: Method not allowed",
+            "title": getattr(e, 'title', None) or "LifeMonitor: Method not allowed",
             "code": "404",
-            "description": str(e)
+            "description": getattr(e, 'detail', None) or str(e)
             if e and logger.isEnabledFor(logging.DEBUG)
             else "Method not allowed for this resource",
             "resource": escape(resource),
@@ -107,11 +119,11 @@ def handle_405(e: Exception = None):
 
 @blueprint.route("/429")
 def handle_429(e: Exception = None):
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: API rate limit exceeded",
+            "title": getattr(e, 'title', None) or "LifeMonitor: API rate limit exceeded",
             "code": "429",
-            "description": str(e)
+            "description": getattr(e, 'detail', None) or str(e)
             if e and logger.isEnabledFor(logging.DEBUG)
             else "API rate limit exceeded",
         }
@@ -120,11 +132,11 @@ def handle_429(e: Exception = None):
 
 @blueprint.route("/500")
 def handle_500(e: Exception = None):
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: Internal Server Error",
+            "title": getattr(e, 'title', None) or "LifeMonitor: Internal Server Error",
             "code": "500",
-            "description": str(e)
+            "description": getattr(e, 'detail', None) or str(e)
             if e and logger.isEnabledFor(logging.DEBUG)
             else "Internal Server Error: the server encountered a temporary error and could not complete your request",
         }
@@ -133,22 +145,25 @@ def handle_500(e: Exception = None):
 
 @blueprint.route("/502")
 def handle_502(e: Exception = None):
-    return handle_error(
+    return __handle_error__(
         {
-            "title": "LifeMonitor: Bad Gateway",
+            "title": getattr(e, 'title', None) or "LifeMonitor: Bad Gateway",
             "code": "502",
-            "description": str(e)
+            "description": getattr(e, 'detail', None) or str(e)
             if e and logger.isEnabledFor(logging.DEBUG)
             else "Internal Server Error: the server encountered a temporary error and could not complete your request",
         }
     )
 
 
-def handle_error(error: Dict[str, str]):
+def __handle_error__(error: Dict[str, str]):
     back_url = request.args.get("back_url", url_for("auth.profile"))
     # parse Accept header
     accept = request.headers.get("Accept", "text/html")
-    if "application/json" in accept:
+    content_type = request.headers.get("Content-Type")
+    if "application/json" == accept \
+            or 'application/json' == content_type \
+            or 'application/problem+json' == content_type:
         # return error as JSON
         return error, error.get("code", 500)
     try:

lifemonitor/exceptions.py

@@ -21,7 +21,6 @@
 import logging
 
 import connexion
-
 from flask import Response, request
 from werkzeug.exceptions import HTTPException
 
@@ -158,15 +157,27 @@ def __init__(self, detail=None,
 
 class NotValidROCrateException(LifeMonitorException):
 
-    def __init__(self, detail="Not valid RO Crate",
+    def __init__(self, detail="Not valid RO-Crate",
                  type="about:blank", status=400, instance=None, **kwargs):
         super().__init__(title="Bad request",
                          detail=detail, status=status, **kwargs)
 
 
+class ROCrateNotFoundException(LifeMonitorException):
+
+    def __init__(self, detail="RO-Crate not found",
+                 type="about:blank", status=404, resource=None, **kwargs):
+        super().__init__(title="Bad request",
+                         detail=detail, status=status, **kwargs)
+        self.resource = resource
+
+    def __str__(self):
+        return f"Unable to find the RO-Crate {self.resource}"
+
+
 class DecodeROCrateException(LifeMonitorException):
 
-    def __init__(self, detail="Unable to decode RO Crate",
+    def __init__(self, detail="Unable to decode RO-Crate",
                  type="about:blank", status=400, instance=None, **kwargs):
         super().__init__(title="Bad request",
                          detail=detail, status=status, **kwargs)
@@ -216,9 +227,12 @@ def handle_exception(e: Exception):
     if logger.isEnabledFor(logging.DEBUG):
         logger.exception(e)
     if isinstance(e, LifeMonitorException):
+        from .errors import handle_error
+        if request.accept_mimetypes.best == "text/html":
+            return handle_error(e)
         return Response(response=e.to_json(),
                         status=e.status,
-                        mimetype="application/problem+json")
+                        mimetype=request.accept_mimetypes.best)
     if isinstance(e, HTTPException):
         return report_problem(status=e.code,
                               title=e.__class__.__name__,
@@ -243,6 +257,9 @@ def report_problem(status, title, detail=None, type=None, instance=None, extra_i
     """
     Returns a `Problem Details <https://tools.ietf.org/html/draft-ietf-appsawg-http-problem-00>`_ error response.
     """
+    if request.accept_mimetypes.best == "text/html":
+        from .errors import handle_error
+        return handle_error(LifeMonitorException(title=title, detail=detail, status=status))
     if not type:
         type = 'about:blank'
 

lifemonitor/lang/messages.py

@@ -31,8 +31,10 @@
 not_authorized_registry_access = "User not authorized to access the registry '{}'"
 not_authorized_workflow_access = "User not authorized to get workflow data"
 input_data_missing = "One or more input data are missing"
-decode_ro_crate_error = "Unable to decode the RO Crate: it should be encoded using base64"
-invalid_ro_crate = "RO Crate processing exception"
+decode_ro_crate_error = "Unable to decode the RO-Crate: it should be encoded using base64"
+invalid_ro_crate = "RO-Crate processing exception"
+ro_crate_not_found = "RO-Crate not found"
+registry_ro_crate_not_found = "Registry RO-Crate not found"
 workflow_not_found = "Workflow '{}' not found"
 workflow_version_not_found = "Workflow '{}' (ver.{}) not found"
 workflow_version_conflict = "Workflow '{}' (ver.{}) already exists"

lifemonitor/utils.py

@@ -515,7 +515,7 @@ def __enter__(self):
         # written into a local file and a local roc_link will be returned.
         logger.debug("Entering ROCrateLinkContext: %r", self.rocrate_or_link)
         if validate_url(self.rocrate_or_link):
-            logger.debug("RO Crate param is a link: %r", self.rocrate_or_link)
+            logger.debug("RO-Crate param is a link: %r", self.rocrate_or_link)
             return self.rocrate_or_link
         if self.rocrate_or_link:
             if os.path.isdir(self.rocrate_or_link) or os.path.isfile(self.rocrate_or_link):
@@ -535,7 +535,7 @@ def __enter__(self):
             except Exception as e:
                 logger.debug(e)
                 raise lm_exceptions.DecodeROCrateException(detail=str(e))
-        logger.debug("RO Crate link is undefined!!!")
+        logger.debug("RO-Crate link is undefined!!!")
         return None
 
     def __exit__(self, type, value, traceback):