Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves transaction rollback handling #395

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

jlavelle
Copy link
Member

@jlavelle jlavelle commented Dec 5, 2024

I added exception handling around the success event SQL execution in finishTransaction in withTransaction. If a SqlExecutionError occurs, it will run the rollback callback and rethrow the exception.

I also added libpq transaction status checks to beginTransaction and finishTransaction in withTransaction. If the status is unexpected for a given transaction event, withTransaction will throw an UnexpectedTransactionStatusError.

Copy link
Contributor

@jbrechtel jbrechtel left a comment

Choose a reason for hiding this comment

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

I'm in favor of this change in general. I agree with @telser re: combining the concepts of Orville's transaction state and LibPQ's and handling each LibPQ state explicitly.

I added exception handling around the success event SQL execution
in `finishTransaction` in `withTransaction`. If a `SqlExecutionError`
occurs, it will run the rollback callback and rethrow the exception.

I also added libpq transaction status checks to `beginTransaction` and
`finishTransaction` in `withTransaction`. If the status is unexpected
for a given transaction event, `withTransaction` will throw an
`UnexpectedTransactionStatusError`.
@jlavelle jlavelle force-pushed the nebula/improve-transaction-rollbacks branch from a6c350e to e08919c Compare December 19, 2024 22:50
@jlavelle jlavelle requested a review from telser December 19, 2024 23:20
Copy link
Contributor

@telser telser left a comment

Choose a reason for hiding this comment

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

I think this is going to be a good backstop against connections being in unexpected states. Under usual circumstances that shouldn't happen, but great to have the handling.

@jlavelle jlavelle merged commit e08919c into main Dec 23, 2024
20 checks passed
@jlavelle jlavelle deleted the nebula/improve-transaction-rollbacks branch December 23, 2024 15:16
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.

4 participants