Skip to content

Fix transaction context manager to re-raise exceptions after rollback#230

Merged
jogardi merged 1 commit into
kennethreitz:masterfrom
jogardi:fix/reraise-transaction-exception
Feb 8, 2026
Merged

Fix transaction context manager to re-raise exceptions after rollback#230
jogardi merged 1 commit into
kennethreitz:masterfrom
jogardi:fix/reraise-transaction-exception

Conversation

@jogardi

@jogardi jogardi commented Feb 8, 2026

Copy link
Copy Markdown
Collaborator

The except block was swallowing exceptions silently after rolling back the transaction. Added raise to propagate the exception to the caller.

Fixes this issue #195 (comment)

Summary by CodeRabbit

  • Bug Fixes
    • Database transaction error handling now properly propagates exceptions to callers after rollback operations, instead of suppressing them. This ensures errors are visible and can be handled appropriately by the application.

The except block was swallowing exceptions silently after rolling back
the transaction. Added `raise` to propagate the exception to the caller.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Feb 8, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The Database.transaction context manager's exception handling was modified to re-raise exceptions after performing a rollback, rather than suppressing them. This ensures errors are propagated to the caller while maintaining transaction cleanup.

Changes

Cohort / File(s) Summary
Exception Handling
records.py
Modified Database.transaction context manager to re-raise exceptions after rollback instead of suppressing them.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A hop, a skip, through error's door,
Now exceptions leap once more!
The transaction rolls back with grace,
Then raises the truth to caller's face. 🐰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: adding exception re-raising in the transaction context manager after rollback.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
records.py (1)

344-346: Consider narrowing the bare except: to except BaseException:.

The bare except: is functionally equivalent to except BaseException: but is less explicit. A minor pre-existing style nit — using except BaseException: makes the intent clearer (i.e., rollback should happen even for KeyboardInterrupt/SystemExit). If rollback is only desired for regular exceptions, except Exception: would be more appropriate.

Optional: make the except clause explicit
-        except:
+        except BaseException:
             tx.rollback()
             raise

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.

@jogardi jogardi merged commit a2c0785 into kennethreitz:master Feb 8, 2026
1 of 7 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant