SQ-925: Refactor DivBase API's exception handling

packages/divbase-api/src/divbase_api/exceptions.py

@@ -1,31 +1,60 @@
 """
 DivBase API custom exceptions.
 
-All exceptions in this module should inherit from DivBaseAPIException, so we can catch for that externally.
+All exceptions in this module should inherit from DivBaseAPIException.
 """
 
+import logging
 from pathlib import Path
 
 from fastapi import status
 
 
 class DivBaseAPIException(Exception):
-    """Base exception for all DivBase errors."""
+    """
+    Base exception for all DivBaseAPI errors
+
+    Each subclass declares how `exception_handlers.divbase_api_exception_handler` should treat it:
+    - log_level: What log level to use
+    - include_exc_info: whether to include the exception traceback in the logs.
+    - frontend_redirect_url: if set, frontend (non-API) requests are redirected here instead of shown the
+      generic error page.
+    - frontend_message: if set, frontend requests see this instead of `message` (API responses always see
+      `message`) - used when the raw message is too internal/technical for a human-facing page.
+    """
+
+    log_level: int = logging.INFO
+    include_exc_info: bool = False
+    frontend_redirect_url: str | None = None
+    frontend_message: str | None = None
 
     def __init__(self, message: str, status_code: int, headers: dict[str, str] | None = None):
         self.message = message
         self.status_code = status_code
         self.headers = headers or {}
         super().__init__(self.message)
 
+    @property
+    def error_type(self) -> str:
+        """Returned in API responses as the (error) type."""
+        return type(self).__name__
+
 
 class AuthenticationError(DivBaseAPIException):
+    log_level = logging.INFO
+    include_exc_info = False
+    frontend_redirect_url = "/login"
+
     def __init__(self, message: str = "Authentication required"):
         default_headers = {"WWW-Authenticate": "Bearer"}
         super().__init__(message=message, status_code=status.HTTP_401_UNAUTHORIZED, headers=default_headers)
 
 
 class AuthorizationError(DivBaseAPIException):
+    log_level = logging.INFO
+    include_exc_info = False
+    frontend_redirect_url = "/login"
+
     def __init__(self, message: str = "Authorization required"):
         default_headers = {"WWW-Authenticate": "Bearer"}
         super().__init__(message=message, status_code=status.HTTP_403_FORBIDDEN, headers=default_headers)
@@ -34,6 +63,9 @@ def __init__(self, message: str = "Authorization required"):
 class UserRegistrationError(DivBaseAPIException):
     """Exception for failed registration of new user or update of existing user."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(
         self,
         internal_logging_message: str,  # for logs, can be more detailed
@@ -44,70 +76,95 @@ def __init__(
 
 
 class ProjectNotFoundError(DivBaseAPIException):
+    log_level = logging.DEBUG
+    include_exc_info = False
+    frontend_message = "Project not found or you don't have access."
+
     def __init__(self, message: str = "Project not found or you don't have access"):
         super().__init__(message=message, status_code=status.HTTP_404_NOT_FOUND)
 
 
 class ProjectMemberNotFoundError(DivBaseAPIException):
+    log_level = logging.INFO
+    include_exc_info = False
+    frontend_redirect_url = "/projects"
+
     def __init__(self, message: str = "Project member not found"):
         super().__init__(message=message, status_code=status.HTTP_404_NOT_FOUND)
 
 
 class UserNotFoundError(DivBaseAPIException):
+    log_level = logging.DEBUG
+    include_exc_info = False
+
     def __init__(self, message: str = "User not found"):
         super().__init__(message=message, status_code=status.HTTP_404_NOT_FOUND)
 
 
 class ProjectMemberAlreadyExistsError(DivBaseAPIException):
+    log_level = logging.DEBUG
+    include_exc_info = False
+
     def __init__(self, message: str = "User is already a member of this project"):
         super().__init__(message=message, status_code=status.HTTP_409_CONFLICT)
 
 
 class ProjectCreationError(DivBaseAPIException):
-    def __init__(self, message: str = "Project creation failed"):
-        super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
-
+    log_level = logging.WARNING
+    include_exc_info = True
 
-class TooManyObjectsInRequestError(DivBaseAPIException):
-    """Raise when e.g. too many files are requested to be downloaded in a single request."""
-
-    def __init__(self, message: str = "Too many objects to work on in a single request"):
+    def __init__(self, message: str = "Project creation failed"):
         super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
 
 
 class ProjectVersionCreationError(DivBaseAPIException):
     """Raised when there is an error creating a new project version."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, message: str = "Failed to create a new project version"):
         super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
 
 
 class ProjectVersionAlreadyExistsError(DivBaseAPIException):
     """Raised when attempting to create a project version that already exists."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(
-        self, message: str = "A project version with the specified name already exists, please choose a different name."
+        self, message: str = "A project version with this name already exists, please choose a different name."
     ):
         super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
 
 
 class ProjectVersionNotFoundError(DivBaseAPIException):
     """Raised when a project version is not found."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, message: str = "Could not find the specified project version"):
         super().__init__(message=message, status_code=status.HTTP_404_NOT_FOUND)
 
 
 class ProjectVersionSoftDeletedError(DivBaseAPIException):
     """Raised when a user tries to modify a project version that is soft-deleted."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, message: str):
         super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
 
 
 class VCFDimensionsEntryMissingError(DivBaseAPIException):
     """Raised when there are no entries in the VCF dimensions db table."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, project_name: str | None = None):
         if project_name:
             message = (
@@ -127,6 +184,9 @@ def __init__(self, project_name: str | None = None):
 class DimensionsUpdateAlreadyInProcessError(DivBaseAPIException):
     """Raised when a user tries to queue a new VCF dimensions update task for a project that already has a queued or running update task."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, project_name: str, ongoing_task_id: int):
         message = (
             f"A VCF dimensions update task (with id: {ongoing_task_id}) is already in process for the project '{project_name}'. \n"
@@ -136,19 +196,12 @@ def __init__(self, project_name: str, ongoing_task_id: int):
         super().__init__(message=message, status_code=status.HTTP_409_CONFLICT)
 
 
-class TaskNotFoundInBackendError(DivBaseAPIException):
-    """Raised when a task ID exists in the task_history table in the database but not in the results backend."""
-
-    def __init__(
-        self,
-        message: str = "Task ID not found in results backend. It may have been purged during cleanup of old task records.",
-    ):
-        super().__init__(message=message, status_code=status.HTTP_410_GONE)
-
-
 class DownloadedFileChecksumMismatchError(DivBaseAPIException):
     """Raised when a worker downloads a file but the calculated checksum does not match the checksum provided by s3."""
 
+    log_level = logging.ERROR
+    include_exc_info = True
+
     def __init__(self, file_path: Path, calculated_checksum: str, expected_checksum: str):
         message = (
             f"Downloaded file checksum mismatch for file '{file_path.name}'. "
@@ -158,17 +211,33 @@ def __init__(self, file_path: Path, calculated_checksum: str, expected_checksum:
         super().__init__(message=message, status_code=status.HTTP_409_CONFLICT)
 
 
+class TooManyObjectsInRequestError(DivBaseAPIException):
+    """Raise when e.g. too many files are requested to be downloaded in a single request."""
+
+    log_level = logging.WARNING
+    include_exc_info = False
+
+    def __init__(self, message: str = "Too many objects to work on in a single request"):
+        super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
+
+
 class ObjectDoesNotExistError(DivBaseAPIException):
     """Raised when an S3 object/key does not exist in the project's bucket."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, key: str, bucket_name: str):
-        message = f"The file/object '{key}' does not exist in the project '{bucket_name}'. "
+        message = f"The file/object '{key}' does not exist in the project's store: '{bucket_name}'. "
         super().__init__(message=message, status_code=status.HTTP_404_NOT_FOUND)
 
 
 class TSVFileNotFoundInProjectError(DivBaseAPIException):
     """Raised when a TSV doesn't exist in project storage."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, filename: str | None = None, project_name: str | None = None):
         self.filename = filename
         self.project_name = project_name
@@ -189,13 +258,19 @@ def __init__(self, filename: str | None = None, project_name: str | None = None)
 class QueueClosedError(DivBaseAPIException):
     """Raised when the queue is closed for new tasks (e.g. before a maintenance window)."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, message: str):
         super().__init__(message=message, status_code=status.HTTP_400_BAD_REQUEST)
 
 
 class PATLimitExceededError(DivBaseAPIException):
     """Raised when a user has reached the maximum number of active personal access tokens."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(
         self,
         message: str = "You have reached the maximum of 5 active tokens. Please revoke one before creating a new one.",
@@ -206,5 +281,8 @@ def __init__(
 class PATDuplicateNameError(DivBaseAPIException):
     """Raised when a user already has an active (not soft-deleted) personal access token with the same name."""
 
+    log_level = logging.INFO
+    include_exc_info = False
+
     def __init__(self, message: str = "You already have an active token with this name."):
         super().__init__(message=message, status_code=status.HTTP_409_CONFLICT)

packages/divbase-api/src/divbase_api/middleware.py

@@ -13,7 +13,7 @@
 from urllib.parse import urlparse
 
 import structlog
-from fastapi import FastAPI
+from fastapi import FastAPI, status
 from fastapi.middleware.gzip import GZipMiddleware
 from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
 from starlette.middleware.trustedhost import TrustedHostMiddleware
@@ -94,8 +94,8 @@ async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -
                 "If you're not sure how to do that, you can find instructions on how to upgrade here: "
                 f"{api_settings.general.mkdocs_site_url}/user-guides/installation"
             )
-            body = {"detail": message, "type": "cli_version_outdated_error"}
-            return JSONResponse(content=body, status_code=400)
+            body = {"detail": message, "type": "CLIVersionOutdatedError"}
+            return JSONResponse(content=body, status_code=status.HTTP_400_BAD_REQUEST)
 
         response = await call_next(request)
         return response

packages/divbase-api/src/divbase_api/templates/404.html

@@ -15,7 +15,7 @@ <h1 class="display-4 fw-bold text-primary mb-3">404</h1>
                 <h2 class="h4 mb-3">Page Not Found</h2>
 
                 <p class="text-muted mb-4">
-                    Sorry, the page you are looking for doesn't exist or has been moved.
+                    {{ error_message | default("Sorry, the page you are looking for doesn't exist or has been moved.") }}
                 </p>
 
                 <div class="d-flex flex-column flex-sm-row gap-2 justify-content-center">

tests/e2e_integration/cli_commands/test_auth_cli.py

@@ -260,7 +260,7 @@ def test_login_with_outdated_cli_version_fails(monkeypatch):
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert "400" in str(result.exception)
-    assert "cli_version_outdated_error" in str(result.exception)
+    assert "CLIVersionOutdatedError" in str(result.exception)
 
 
 def test_any_command_with_outdated_cli_version_fails(monkeypatch):
@@ -275,7 +275,7 @@ def test_any_command_with_outdated_cli_version_fails(monkeypatch):
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert "400" in str(result.exception)
-    assert "cli_version_outdated_error" in str(result.exception)
+    assert "CLIVersionOutdatedError" in str(result.exception)
 
 
 def test_jwt_session_takes_priority_over_pat(disable_keyring_backend, monkeypatch):

tests/e2e_integration/cli_commands/test_dimensions_cli.py

@@ -158,7 +158,7 @@ def test_dimensions_update_blocked_when_task_already_in_progress_for_same_projec
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert "409" in str(result.exception)
-    assert "dimensions_update_task_already_in_process_error" in str(result.exception)
+    assert "DimensionsUpdateAlreadyInProcessError" in str(result.exception)
 
 
 def test_dimensions_update_allowed_when_ongoing_task_is_for_different_project(
@@ -286,7 +286,7 @@ def test_show_vcf_dimensions_task_when_file_missing(
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert project_name in str(result.exception)
-    assert "vcf_dimensions_entry_missing_error" in str(result.exception)
+    assert "VCFDimensionsEntryMissingError" in str(result.exception)
 
 
 def test_get_dimensions_info_returns_empty(

tests/e2e_integration/cli_commands/test_middleware.py

@@ -48,7 +48,7 @@ def test_cli_version_middleware_rejects_outdated_cli_header():
     """
     outdated_response = httpx.get(HEALTH_URL, headers={CLI_VERSION_HEADER_KEY: "0.0.0"}, timeout=10)
     assert outdated_response.status_code == 400
-    assert outdated_response.json()["type"] == "cli_version_outdated_error"
+    assert outdated_response.json()["type"] == "CLIVersionOutdatedError"
     outdated_request_id = outdated_response.headers["X-Request-ID"]
     _assert_valid_request_id(outdated_request_id)
 
@@ -77,7 +77,7 @@ def test_cli_version_header_is_optional():
         HEALTH_URL, headers={CLI_VERSION_HEADER_KEY: "not.a.valid.version"}, timeout=10
     )
     assert malformed_header_response.status_code == 400
-    assert malformed_header_response.json()["type"] == "cli_version_outdated_error"
+    assert malformed_header_response.json()["type"] == "CLIVersionOutdatedError"
     malformed_request_id = malformed_header_response.headers["X-Request-ID"]
     _assert_valid_request_id(malformed_request_id)
 

tests/e2e_integration/cli_commands/test_personal_access_tokens.py

@@ -65,15 +65,15 @@ def assert_401_error(result):
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert "401" in str(result.exception)
-    assert "authentication_error" in str(result.exception)
+    assert "AuthenticationError" in str(result.exception)
 
 
 def assert_403_error(result):
     """Helper to assert that a CLI result is an 403 error."""
     assert result.exit_code != 0
     assert isinstance(result.exception, DivBaseAPIError)
     assert "403" in str(result.exception)
-    assert "authorization_error" in str(result.exception)
+    assert "AuthorizationError" in str(result.exception)
 
 
 @pytest.fixture

tests/e2e_integration/cli_commands/test_queue_status.py

@@ -72,7 +72,7 @@ def test_regular_users_cannot_submit_tasks_when_queue_closed(
         assert result.exit_code != 0
         assert isinstance(result.exception, DivBaseAPIError)
         assert "400" in str(result.exception)
-        assert "queue_closed_error" in str(result.exception)
+        assert "QueueClosedError" in str(result.exception)
 
 
 def test_admin_users_can_submit_tasks_when_queue_closed(
@@ -87,7 +87,7 @@ def test_admin_users_can_submit_tasks_when_queue_closed(
         # We just want the error to not be from the queue being closed
         # This check happens early enough, that a badly formulated query does not matter
         if result.exit_code != 0:
-            assert "queue_closed_error" not in str(result.exception)
+            assert "QueueClosedError" not in str(result.exception)
 
 
 def test_regular_users_can_submit_tasks_when_queue_not_yet_closed(
@@ -102,4 +102,4 @@ def test_regular_users_can_submit_tasks_when_queue_not_yet_closed(
         # We just want the error to not be from the queue being closed
         # This check happens early enough, that a badly formulated query does not matter
         if result.exit_code != 0:
-            assert "queue_closed_error" not in str(result.exception)
+            assert "QueueClosedError" not in str(result.exception)