Merge pull request #18041 from dannon/extra-exception-logging

[24.0] Adds logging of messageExceptions in the fastapi exception handler.
galaxyproject · May 3, 2024 · 0a784e4 · 0a784e4
2 parents 7408ebb + e5c59e1
commit 0a784e4
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py
@@ -16,6 +16,8 @@
 and messages.
 """
 
+from typing import Optional
+
 from .error_codes import (
     error_codes_by_name,
     ErrorCode,
@@ -30,7 +32,7 @@ class MessageException(Exception):
     # Error code information embedded into API json responses.
     err_code: ErrorCode = error_codes_by_name["UNKNOWN"]
 
-    def __init__(self, err_msg=None, type="info", **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", **extra_error_info):
         self.err_msg = err_msg or self.err_code.default_error_message
         self.type = type
         self.extra_error_info = extra_error_info
@@ -64,7 +66,7 @@ class AcceptedRetryLater(MessageException):
     err_code = error_codes_by_name["ACCEPTED_RETRY_LATER"]
     retry_after: int
 
-    def __init__(self, msg, retry_after=60):
+    def __init__(self, msg: Optional[str] = None, retry_after=60):
         super().__init__(msg)
         self.retry_after = retry_after
 
@@ -136,7 +138,7 @@ class ToolMissingException(MessageException):
     status_code = 400
     err_code = error_codes_by_name["USER_TOOL_MISSING_PROBLEM"]
 
-    def __init__(self, err_msg=None, type="info", tool_id=None, **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", tool_id=None, **extra_error_info):
         super().__init__(err_msg, type, **extra_error_info)
         self.tool_id = tool_id
 
@@ -152,7 +154,7 @@ class ToolInputsNotReadyException(MessageException):
 
 
 class ToolInputsNotOKException(MessageException):
-    def __init__(self, err_msg=None, type="info", *, src: str, id: int, **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", *, src: str, id: int, **extra_error_info):
         super().__init__(err_msg, type, **extra_error_info)
         self.src = src
         self.id = id

diff --git a/lib/galaxy/webapps/base/api.py b/lib/galaxy/webapps/base/api.py
@@ -198,6 +198,11 @@ async def validate_exception_middleware(request: Request, exc: RequestValidation
 
     @app.exception_handler(MessageException)
     async def message_exception_middleware(request: Request, exc: MessageException) -> Response:
+        # Intentionally not logging traceback here as the full context will be
+        # dispatched to Sentry if configured.  This just makes logs less opaque
+        # when one sees a 500.
+        if exc.status_code >= 500:
+            log.info(f"MessageException: {exc}")
         return get_error_response_for_request(request, exc)
 
 

diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py
@@ -2557,11 +2557,13 @@ def populate_module_and_state(
         step_args = param_map.get(step.id, {})
         step_errors = module_injector.compute_runtime_state(step, step_args=step_args)
         if step_errors:
-            raise exceptions.MessageException(step_errors, err_data={step.order_index: step_errors})
+            raise exceptions.MessageException(
+                "Error computing workflow step runtime state", err_data={step.order_index: step_errors}
+            )
         if step.upgrade_messages:
             if allow_tool_state_corrections:
                 log.debug('Workflow step "%i" had upgrade messages: %s', step.id, step.upgrade_messages)
             else:
                 raise exceptions.MessageException(
-                    step.upgrade_messages, err_data={step.order_index: step.upgrade_messages}
+                    "Workflow step has upgrade messages", err_data={step.order_index: step.upgrade_messages}
                 )