diff --git a/backend/app/database/connection.py b/backend/app/database/connection.py index 599526dc1..d82ad6950 100644 --- a/backend/app/database/connection.py +++ b/backend/app/database/connection.py @@ -2,6 +2,9 @@ from contextlib import contextmanager from typing import Generator from app.config.settings import DATABASE_PATH +from app.logging.setup_logging import get_logger + +logger = get_logger(__name__) @contextmanager @@ -25,7 +28,8 @@ def get_db_connection() -> Generator[sqlite3.Connection, None, None]: try: yield conn conn.commit() - except Exception: + except Exception as e: + logger.error(f"Database transaction failed, rolling back: {e}") conn.rollback() raise finally: diff --git a/backend/app/database/face_clusters.py b/backend/app/database/face_clusters.py index ba9f4ba4a..9ba273263 100644 --- a/backend/app/database/face_clusters.py +++ b/backend/app/database/face_clusters.py @@ -1,6 +1,9 @@ import sqlite3 from typing import Optional, List, Dict, TypedDict, Union from app.config.settings import DATABASE_PATH +from app.logging.setup_logging import get_logger + +logger = get_logger(__name__) # Type definitions ClusterId = str @@ -60,10 +63,10 @@ def db_delete_all_clusters(cursor: Optional[sqlite3.Cursor] = None) -> int: if own_connection: conn.commit() return deleted_count - except Exception: + except sqlite3.Error as e: if own_connection: conn.rollback() - print("Error deleting all clusters.") + logger.error(f"Error deleting all clusters: {e}") raise finally: if own_connection: @@ -114,9 +117,10 @@ def db_insert_clusters_batch( if own_connection: conn.commit() return cluster_ids - except Exception: + except sqlite3.Error as e: if own_connection: conn.rollback() + logger.error(f"Error inserting clusters batch: {e}") raise finally: if own_connection: diff --git a/backend/app/database/faces.py b/backend/app/database/faces.py index 0e43f7117..4099e7aec 100644 --- a/backend/app/database/faces.py +++ b/backend/app/database/faces.py @@ -3,6 +3,9 @@ import numpy as np from typing import Optional, List, Dict, Union, TypedDict from app.config.settings import DATABASE_PATH +from app.logging.setup_logging import get_logger + +logger = get_logger(__name__) # Type definitions FaceId = int @@ -312,14 +315,22 @@ def db_update_face_cluster_ids_batch( conn = sqlite3.connect(DATABASE_PATH) cursor = conn.cursor() + # 1. Prepare update data outside the DB transaction. + # mapping.get() calls could raise AttributeError if mappings are malformed; + # these are not sqlite3 errors and must be caught before the transaction. try: - # Prepare update data as tuples (cluster_id, face_id) - update_data = [] - for mapping in face_cluster_mapping: - face_id = mapping.get("face_id") - cluster_id = mapping.get("cluster_id") - update_data.append((cluster_id, face_id)) + update_data = [ + (mapping.get("cluster_id"), mapping.get("face_id")) + for mapping in face_cluster_mapping + ] + except (AttributeError, KeyError, TypeError) as e: + if own_connection: + conn.close() + logger.error(f"Failed to prepare face cluster update data: {e}") + raise + # 2. Database transaction — only pure DB operations here, so sqlite3.Error is safe. + try: cursor.executemany( """ UPDATE faces @@ -331,10 +342,10 @@ def db_update_face_cluster_ids_batch( if own_connection: conn.commit() - except Exception: + except sqlite3.Error as e: if own_connection: conn.rollback() - print("Error updating face cluster IDs in batch.") + logger.error(f"Error updating face cluster IDs in batch: {e}") raise finally: if own_connection: diff --git a/backend/app/database/metadata.py b/backend/app/database/metadata.py index d431f6e2b..7e208ae15 100644 --- a/backend/app/database/metadata.py +++ b/backend/app/database/metadata.py @@ -3,6 +3,9 @@ import json from typing import Optional, Dict, Any from app.config.settings import DATABASE_PATH +from app.logging.setup_logging import get_logger + +logger = get_logger(__name__) def db_create_metadata_table() -> None: @@ -68,13 +71,23 @@ def db_update_metadata( Returns: True if the metadata was updated, False otherwise """ + # 1. Prepare/serialize data outside the DB transaction. + # json.dumps can raise TypeError/ValueError for non-serializable objects; + # these are not sqlite3 errors and must be caught separately. + try: + metadata_json = json.dumps(metadata) + except (TypeError, ValueError) as e: + logger.error(f"Failed to serialize metadata: {e}") + raise + own_connection = cursor is None - if own_connection: - conn = sqlite3.connect(DATABASE_PATH) - cursor = conn.cursor() + conn = None + # 2. Database transaction / owned connection setup. try: - metadata_json = json.dumps(metadata) + if own_connection: + conn = sqlite3.connect(DATABASE_PATH) + cursor = conn.cursor() # Delete all existing rows and insert new one cursor.execute("DELETE FROM metadata") @@ -84,12 +97,12 @@ def db_update_metadata( if own_connection: conn.commit() return success - except Exception as e: + except sqlite3.Error as e: if own_connection: - conn.rollback() - - print(f"Error updating metadata: {e}") + if conn is not None: + conn.rollback() + logger.error(f"Error updating metadata: {e}") raise finally: - if own_connection: + if own_connection and conn is not None: conn.close()