fix: improve SQLite error handling in get_db_connection

[ajithhraj]

Fixes #1102

Changes

• Replace broad Exception catch with sqlite3.Error for precise DB error handling
• Add logging to surface database failures with context
• Preserve existing rollback and re-raise behavior

Why

Catching generic Exception hides SQLite-specific errors and makes debugging harder. This change improves code clarity and aligns with Python best practices.

Summary by CodeRabbit

• Chores
  • Enhanced error logging for database operations to improve system monitoring and troubleshooting capabilities
  • Implemented dedicated error handling for database exceptions to ensure proper error tracking and recovery
  • Improved connection lifecycle management with guaranteed resource cleanup to enhance system stability and prevent resource leaks

[coderabbitai]

Walkthrough

The change improves database error handling in the connection module by replacing generic exception catching with SQLite-specific error handling, adding logging for failed transactions, and guaranteeing connection closure via a finally block.

Changes

Cohort / File(s)	Summary
Database Connection Error Handling backend/app/database/connection.py	Refines exception handling from broad Exception to SQLite-specific sqlite3.Error, adds module-level logging with error messages on database failures, and ensures connection closure with finally block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Python

Poem

🐰 A database bug, now caught with care,
SQLite errors logged with flair,
No more broad exceptions thrown,
The connection closes, never alone,
Better debugging, crystal clear!

🚥 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 describes the main change: improving SQLite error handling in the get_db_connection function, which is the primary focus of the changeset.
Linked Issues check	✅ Passed	The pull request fulfills all coding requirements from issue #1102: replaces broad Exception with sqlite3.Error, adds logging for database failures, and preserves rollback/re-raise semantics.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with the objectives in issue #1102; minor formatting tweaks to PRAGMA statements are functionally equivalent and within scope of connection.py 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
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

backend/app/database/connection.py (1)

20-33: ⚠️ Potential issue | 🟠 Major

Move PRAGMA statements into try-block and add generic Exception handler for proper rollback semantics.

The function's docstring promises "rolls back on failure," but the current implementation only rolls back for sqlite3.Error. Non-SQLite exceptions raised within the with get_db_connection() block (e.g., RuntimeError, custom exceptions) will bypass rollback and leave transactions uncommitted. Additionally, PRAGMA statements on lines 21–25 execute before the try block, so their errors won't be logged through the error handler.

Move all PRAGMA statements into the try block and add a generic Exception handler to ensure rollback is invoked for all failure modes, not just SQLite-specific errors.

Suggested fix

 conn = sqlite3.connect(DATABASE_PATH)
-# --- Strict enforcement of all relational and logical rules ---
-conn.execute("PRAGMA foreign_keys = ON;")          # Enforce FK constraints
-conn.execute("PRAGMA ignore_check_constraints = OFF;")  # Enforce CHECK constraints
-conn.execute("PRAGMA recursive_triggers = ON;")    # Allow nested triggers
-conn.execute("PRAGMA defer_foreign_keys = OFF;")   # Immediate FK checking
-conn.execute("PRAGMA case_sensitive_like = ON;")   # Make LIKE case-sensitive
 try:
+    # --- Strict enforcement of all relational and logical rules ---
+    conn.execute("PRAGMA foreign_keys = ON;")  # Enforce FK constraints
+    conn.execute("PRAGMA ignore_check_constraints = OFF;")  # Enforce CHECK constraints
+    conn.execute("PRAGMA recursive_triggers = ON;")  # Allow nested triggers
+    conn.execute("PRAGMA defer_foreign_keys = OFF;")  # Immediate FK checking
+    conn.execute("PRAGMA case_sensitive_like = ON;")  # Make LIKE case-sensitive
     yield conn
     conn.commit()
 except sqlite3.Error as e:
     logger.error("Database error occurred, rolling back transaction: %s", e)
     conn.rollback()
     raise
+except Exception:
+    conn.rollback()
+    raise
 finally:
     conn.close()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/database/connection.py` around lines 20 - 33, The PRAGMA
statements (conn.execute(...) lines) must be moved inside the existing try block
in the get_db_connection context manager so any errors from those statements are
caught and logged; also add a generic Exception except block in addition to the
existing sqlite3.Error handler to call conn.rollback(), log the error via
logger.error (including the exception object), and re-raise, ensuring any
non-sqlite exceptions also trigger rollback and proper cleanup in
get_db_connection where conn is used.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@backend/app/database/connection.py`:
- Around line 20-33: The PRAGMA statements (conn.execute(...) lines) must be
moved inside the existing try block in the get_db_connection context manager so
any errors from those statements are caught and logged; also add a generic
Exception except block in addition to the existing sqlite3.Error handler to call
conn.rollback(), log the error via logger.error (including the exception
object), and re-raise, ensuring any non-sqlite exceptions also trigger rollback
and proper cleanup in get_db_connection where conn is used.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a0fc7a18-e71b-42d8-8b3d-bedaab454efd

📥 Commits

Reviewing files that changed from the base of the PR and between 8274ebd and a81b1a6.

📒 Files selected for processing (1)

• backend/app/database/connection.py

[ajithhraj]

Hi @SIDDHANTCOOKIE, thank you for reviewing! I noticed you reopened the PR — please let me know if any changes are needed and I'll update it right away!

[SIDDHANTCOOKIE]

Closing this as unreviewed issue and duplicate of pr #1151