Skip to content

fix(database): improve SQLite error handling and logging in images.py and folders.py#1316

Open
Dotify71 wants to merge 2 commits into
AOSSIE-Org:mainfrom
Dotify71:fix/database-error-handling
Open

fix(database): improve SQLite error handling and logging in images.py and folders.py#1316
Dotify71 wants to merge 2 commits into
AOSSIE-Org:mainfrom
Dotify71:fix/database-error-handling

Conversation

@Dotify71

@Dotify71 Dotify71 commented Jun 12, 2026

Copy link
Copy Markdown

Resolves #1307

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for database operations to log database-specific failures and retain existing rollback behavior.
    • Narrowed exception scopes so non-database errors are no longer silently caught, improving reliability and observability.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e096428f-906c-47de-a2d7-11dc0213b95e

📥 Commits

Reviewing files that changed from the base of the PR and between 7ad6e62 and d8e33d6.

📒 Files selected for processing (1)
  • backend/app/database/folders.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/app/database/folders.py

Walkthrough

Exception handlers across two DB modules are narrowed from catching generic Exception to sqlite3.Error. Folder module adds a module-level logger and three folder batch functions now log sqlite errors; twelve image database functions restrict their except blocks to sqlite3.Error while preserving rollback/logging behavior.

Changes

SQLite-Specific Exception Handling

Layer / File(s) Summary
Folder batch operation error handling
backend/app/database/folders.py
Adds get_logger import and logger = get_logger(__name__); db_insert_folders_batch, db_delete_folders_batch, and db_update_ai_tagging_batch now catch sqlite3.Error and call logger.error(...) before rollback and re-raise.
Image database operation error handling
backend/app/database/images.py
Twelve functions (db_bulk_insert_images, db_get_all_images, db_update_image_tagged_status, db_insert_image_classes_batch, db_get_images_by_folder_ids, db_delete_images_by_ids, db_toggle_image_favourite_status, db_get_images_by_date_range, db_get_images_near_location, db_get_images_by_year_month, db_get_images_with_location, db_get_all_images_for_memories) change except Exception to except sqlite3.Error, preserving existing logging/rollback/return behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

Python

Poem

🐰 I hopped through code where errors hide,

SQLite whispers, now caught with pride,
Logger set to mark the trace,
Rollbacks keep the DB in place,
A tidy burrow, safe inside.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: improving SQLite error handling and logging in the two database files (images.py and folders.py).
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #1307: narrowing Exception to sqlite3.Error in both files and adding logger.error() calls before rollback operations.
Out of Scope Changes check ✅ Passed All changes are directly related to the objectives in issue #1307; no out-of-scope modifications detected in the error handling and logging improvements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
backend/app/database/folders.py (1)

54-56: ⚡ Quick win

Use bare raise instead of raise e to preserve traceback context.

Catching and re-raising with raise e resets traceback framing; a bare raise keeps original exception context for debugging.

Suggested patch
-        raise e
+        raise
@@
-        raise e
+        raise
@@
-        raise e
+        raise

Also applies to: 183-185, 312-314

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/folders.py` around lines 54 - 56, In the except blocks
that catch sqlite3.Error (the shown block using "except sqlite3.Error as e:" and
the other occurrences catching sqlite3.Error around the indicated ranges), keep
the conn.rollback() but replace "raise e" with a bare "raise" so the original
traceback/context is preserved; look for the except sqlite3.Error as e: handlers
(they reference conn and variable e) and change the re-raise to "raise" in each
handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/app/database/folders.py`:
- Around line 54-56: The except sqlite3.Error handlers in
backend.app.database.folders.py (the three blocks that currently do "except
sqlite3.Error as e: conn.rollback(); raise e") must log the error before rolling
back and re-raising; update each handler to call logger.error with a clear
message and the exception (e.g., "Failed to <operation> on folders DB: %s", e)
immediately before conn.rollback(), keeping the subsequent rollback and raise
intact so diagnostics include the actual DB error.

---

Nitpick comments:
In `@backend/app/database/folders.py`:
- Around line 54-56: In the except blocks that catch sqlite3.Error (the shown
block using "except sqlite3.Error as e:" and the other occurrences catching
sqlite3.Error around the indicated ranges), keep the conn.rollback() but replace
"raise e" with a bare "raise" so the original traceback/context is preserved;
look for the except sqlite3.Error as e: handlers (they reference conn and
variable e) and change the re-raise to "raise" in each handler.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 583c2458-6aa7-4d33-b43a-bbbea70fdff3

📥 Commits

Reviewing files that changed from the base of the PR and between 0d0cbec and 7ad6e62.

📒 Files selected for processing (2)
  • backend/app/database/folders.py
  • backend/app/database/images.py

Comment thread backend/app/database/folders.py
…tion handlers

Each sqlite3.Error handler now logs the failure before rolling back,
ensuring consistent error reporting across all database modules.

Addresses CodeRabbit review on AOSSIE-Org#1316
@Dotify71

Dotify71 commented Jun 12, 2026

Copy link
Copy Markdown
Author

Thanks for catching that @coderabbitai! Added logger.error(...) in all three handlers in folders.py before the rollback - so DB failures are now properly logged before being re-raised. Pushed in the latest commit.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat backend/app/database/folders.py

Length of output: 13541



@Dotify71 Verified — all three handlers in folders.py now correctly log before rollback:

  • db_insert_folders_batchlogger.error(f"Error inserting folders batch: {e}")
  • db_delete_folders_batchlogger.error(f"Error deleting folders batch: {e}")
  • db_update_ai_tagging_batchlogger.error(f"Error updating AI tagging batch: {e}")

The order (log → rollback → re-raise) and the narrowed sqlite3.Error catch are both correct. Looks great! 🐇 ✨


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@Dotify71 Happy to help! The fix looks great — logging before rollback is exactly the right order for observability. The PR is now consistent with the patterns in faces.py and face_clusters.py as targeted by #1307. Good work! 🐇 ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: improve SQLite error handling and logging in images.py and folders.py

1 participant