refactor(albums): improve SQLite error handling and logging

backend/app/routes/albums.py

@@ -1,5 +1,11 @@
 from fastapi import APIRouter, HTTPException, status, Body, Path
 import uuid
+
+from typing import Callable, TypeVar
+from typing_extensions import ParamSpec
+from functools import wraps
+import sqlite3
+from app.logging.setup_logging import get_logger
 from app.schemas.album import (
     GetAlbumsResponse,
     CreateAlbumRequest,
@@ -38,6 +44,38 @@
     album_util_create_from_memory,
 )
 
+logger = get_logger(__name__)
+
+P = ParamSpec("P")
+R = TypeVar("R")
+
+
+def handle_route_exceptions(
+    error_title: str, error_message: str
+) -> Callable[[Callable[P, R]], Callable[P, R]]:
+    def decorator(func: Callable[P, R]) -> Callable[P, R]:
+        @wraps(func)
+        def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
+            try:
+                return func(*args, **kwargs)
+            except HTTPException:
+                raise
+            except Exception as e:
+                logger.error(f"Error in {func.__name__} route: {e}")
+                raise HTTPException(
+                    status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+                    detail=ErrorResponse(
+                        success=False,
+                        error=error_title,
+                        message=error_message,
+                    ).model_dump(),
+                )
+
+        return wrapper
+
+    return decorator
+
+
 router = APIRouter()
 
 
@@ -63,6 +101,9 @@ def _internal_error(message: str) -> HTTPException:
 
 # GET /albums/ - Get all albums (including locked ones)
 @router.get("/", response_model=GetAlbumsResponse)
+@handle_route_exceptions(
+    "Internal Server Error", "An unexpected error occurred while fetching albums."
+)
 def get_albums():
     """Get all albums. Always returns both locked and unlocked albums."""
     albums = db_get_all_albums()
@@ -94,6 +135,9 @@ def get_albums():
 
 # POST /albums/ - Create a new album
 @router.post("/", response_model=CreateAlbumResponse)
+@handle_route_exceptions(
+    "Internal Server Error", "An unexpected error occurred while creating the album."
+)
 def create_album(body: CreateAlbumRequest):
     existing_album = db_get_album_by_name(body.name)
     if existing_album:
@@ -105,8 +149,8 @@ def create_album(body: CreateAlbumRequest):
             album_id, body.name, body.description, body.is_locked, body.password
         )
         return CreateAlbumResponse(success=True, album_id=album_id)
-    except Exception as e:
-        raise _internal_error(f"Failed to create album: {e}") from e
+    except sqlite3.IntegrityError:
+        raise _album_exists(body.name)
 
 
 # POST /albums/from-memory - Create an album from a curated memory
@@ -147,8 +191,6 @@ def create_album_from_memory(
         ) from e
     except AlbumNameTakenError as e:
         raise _album_exists(body.name) from e
-    except Exception as e:
-        raise _internal_error(f"Failed to create album from memory: {e}") from e
 
     return CreateAlbumFromMemoryResponse(
         success=True,
@@ -159,6 +201,9 @@ def create_album_from_memory(
 
 # GET /albums/{album_id} - Get specific album details
 @router.get("/{album_id}", response_model=GetAlbumResponse)
+@handle_route_exceptions(
+    "Internal Server Error", "An unexpected error occurred while fetching the album."
+)
 def get_album(album_id: str = Path(...)):
     album = db_get_album(album_id)
     if not album:
@@ -169,38 +214,31 @@ def get_album(album_id: str = Path(...)):
             ).model_dump(),
         )
 
-    try:
-        # Get image count for the album
-        image_ids = db_get_album_images(album_id)
-        image_count = len(image_ids)
-
-        is_locked = album["is_locked"]
-        album_obj = Album(
-            album_id=album["album_id"],
-            album_name=album["album_name"],
-            description=album["description"] or "",
-            is_locked=is_locked,
-            # Same reasoning as the listing: the cover gives away the contents.
-            cover_image_path=(None if is_locked else db_get_album_cover_path(album_id)),
-            image_count=image_count,
-            created_at=album["created_at"],
-            updated_at=album["updated_at"],
-        )
-        return GetAlbumResponse(success=True, data=album_obj)
-    except Exception as e:
-        raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=ErrorResponse(
-                success=False,
-                error="Internal Server Error",
-                message=f"Failed to fetch album: {str(e)}",
-            ).model_dump(),
-        )
+    # Get image count for the album
+    image_ids = db_get_album_images(album_id)
+    image_count = len(image_ids)
+
+    is_locked = album["is_locked"]
+    album_obj = Album(
+        album_id=album["album_id"],
+        album_name=album["album_name"],
+        description=album["description"] or "",
+        is_locked=is_locked,
+        cover_image_path=(None if is_locked else db_get_album_cover_path(album_id)),
+        image_count=image_count,
+        created_at=album["created_at"],
+        updated_at=album["updated_at"],
+    )
+    return GetAlbumResponse(success=True, data=album_obj)
 
 
 # PUT /albums/{album_id} - Update Album
 @router.put("/{album_id}", response_model=SuccessResponse)
+@handle_route_exceptions(
+    "Failed to Update Album", "An unexpected error occurred while updating the album."
+)
 def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...)):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -238,18 +276,17 @@ def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...)
             album_id, body.name, body.description, body.is_locked, body.password
         )
         return SuccessResponse(success=True, msg="Album updated successfully")
-    except Exception as e:
-        raise HTTPException(
-            status_code=status.HTTP_400_BAD_REQUEST,
-            detail=ErrorResponse(
-                success=False, error="Failed to Update Album", message=str(e)
-            ).model_dump(),
-        )
+    except sqlite3.IntegrityError:
+        raise _album_exists(body.name)
 
 
 # DELETE /albums/{album_id} - Delete an album
 @router.delete("/{album_id}", response_model=SuccessResponse)
+@handle_route_exceptions(
+    "Failed to Delete Album", "An unexpected error occurred while deleting the album."
+)
 def delete_album(album_id: str = Path(...)):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -261,26 +298,22 @@ def delete_album(album_id: str = Path(...)):
             ).model_dump(),
         )
 
-    try:
-        db_delete_album(album_id)
-        return SuccessResponse(success=True, msg="Album deleted successfully")
-    except Exception as e:
-        raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=ErrorResponse(
-                success=False, error="Failed to Delete Album", message=str(e)
-            ).model_dump(),
-        )
+    db_delete_album(album_id)
+    return SuccessResponse(success=True, msg="Album deleted successfully")
 
 
 # GET /albums/{album_id}/images - Get all images in an album
 @router.post("/{album_id}/images/get", response_model=GetAlbumImagesResponse)
+@handle_route_exceptions(
+    "Failed to Retrieve Images", "An unexpected error occurred while retrieving images."
+)
 # GET requests do not accept a body by default.
 # Since we need to send a password securely, switching this to POST -- necessary.
 # Open to suggestions if better approach possible.
 def get_album_images(
     album_id: str = Path(...), body: GetAlbumImagesRequest = Body(...)
 ):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -312,21 +345,17 @@ def get_album_images(
                 ).model_dump(),
             )
 
-    try:
-        image_ids = db_get_album_images(album_id)
-        return GetAlbumImagesResponse(success=True, image_ids=image_ids)
-    except Exception as e:
-        raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=ErrorResponse(
-                success=False, error="Failed to Retrieve Images", message=str(e)
-            ).model_dump(),
-        )
+    image_ids = db_get_album_images(album_id)
+    return GetAlbumImagesResponse(success=True, image_ids=image_ids)
 
 
 # POST /albums/{album_id}/images - Add images to an album
 @router.post("/{album_id}/images", response_model=SuccessResponse)
+@handle_route_exceptions(
+    "Failed to Add Images", "An unexpected error occurred while adding images."
+)
 def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body(...)):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -353,9 +382,9 @@ def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body(
         return SuccessResponse(
             success=True, msg=f"Added {len(body.image_ids)} images to album"
         )
-    except Exception as e:
+    except (ValueError, TypeError) as e:
         raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            status_code=status.HTTP_400_BAD_REQUEST,
             detail=ErrorResponse(
                 success=False, error="Failed to Add Images", message=str(e)
             ).model_dump(),
@@ -364,7 +393,11 @@ def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body(
 
 # DELETE /albums/{album_id}/images/{image_id} - Remove image from album
 @router.delete("/{album_id}/images/{image_id}", response_model=SuccessResponse)
+@handle_route_exceptions(
+    "Failed to Remove Image", "An unexpected error occurred while removing the image."
+)
 def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...)):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -381,9 +414,9 @@ def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...)
         return SuccessResponse(
             success=True, msg="Image removed from album successfully"
         )
-    except Exception as e:
+    except ValueError as e:
         raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            status_code=status.HTTP_404_NOT_FOUND,
             detail=ErrorResponse(
                 success=False, error="Failed to Remove Image", message=str(e)
             ).model_dump(),
@@ -392,9 +425,13 @@ def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...)
 
 # DELETE /albums/{album_id}/images - Remove multiple images from album
 @router.delete("/{album_id}/images", response_model=SuccessResponse)
+@handle_route_exceptions(
+    "Failed to Remove Images", "An unexpected error occurred while removing the images."
+)
 def remove_images_from_album(
     album_id: str = Path(...), body: ImageIdsRequest = Body(...)
 ):
+
     album = db_get_album(album_id)
     if not album:
         raise HTTPException(
@@ -416,15 +453,7 @@ def remove_images_from_album(
             ).model_dump(),
         )
 
-    try:
-        db_remove_images_from_album(album_id, body.image_ids)
-        return SuccessResponse(
-            success=True, msg=f"Removed {len(body.image_ids)} images from album"
-        )
-    except Exception as e:
-        raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=ErrorResponse(
-                success=False, error="Failed to Remove Images", message=str(e)
-            ).model_dump(),
-        )
+    db_remove_images_from_album(album_id, body.image_ids)
+    return SuccessResponse(
+        success=True, msg=f"Removed {len(body.image_ids)} images from album"
+    )

backend/tests/test_albums.py

@@ -694,3 +694,38 @@ def test_create_album_from_memory_duplicate_name(
             assert response.status_code == 409
             assert response.json()["detail"]["error"] == "Album Already Exists"
             mock_create.assert_not_called()
+
+
+class TestAlbumRouteErrors:
+    """Test suite for verifying that album routes correctly handle exceptions."""
+
+    def test_unexpected_exceptions_mapped_to_500(self):
+        """Verify that an unexpected exception returns a 500 error."""
+        from unittest.mock import patch
+
+        with patch("app.routes.albums.db_get_all_albums") as mock_get_all:
+            mock_get_all.side_effect = Exception("Database explosion")
+            response = client.get("/albums/")
+            assert response.status_code == 500
+            json_resp = response.json()
+            assert json_resp["detail"]["success"] is False
+            assert "unexpected error" in json_resp["detail"]["message"].lower()
+
+    def test_duplicate_album_integrity_error(self):
+        """Verify that a database IntegrityError returns a 409 conflict."""
+        import sqlite3
+        from unittest.mock import patch
+
+        with patch("app.routes.albums.db_get_album_by_name", return_value=None):
+            with patch(
+                "app.routes.albums.db_insert_album",
+                side_effect=sqlite3.IntegrityError("Unique constraint failed"),
+            ):
+                response = client.post(
+                    "/albums/",
+                    json={"name": "Duplicate", "description": "This should fail"},
+                )
+                assert response.status_code == 409
+                json_resp = response.json()
+                assert json_resp["detail"]["success"] is False
+                assert "album already exists" in json_resp["detail"]["error"].lower()

backend/tests/test_memory_signals_db.py

@@ -76,6 +76,7 @@ def test_db(monkeypatch: pytest.MonkeyPatch) -> Iterator[str]:
         "app.database.folders",
         "app.database.yolo_mapping",
         "app.database.albums",
+        "app.database.connection",
         "app.database.faces",
         "app.database.face_clusters",
         "app.database.videos",