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

FIX: Rethrow error in commit path of callbacks #3147

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Aug 8, 2023

Actual behaviour
If you have callbacks in your transaction and they fail due any reason, the error is just added to the log and subsequent callbacks will be invoked and finally the transaction will be commited

Expected behaviour
If a callback in the commit path (preCommit/postCommit) has a problem, the exception should be thrown up to the caller and the transaction should NOT be committed, if a preCommit hook fails.

In our situation, we performed a DB.save on an entity. The preCommit hook on that entity performed some DB access in an nested transaction. The JDBC driver failed with an error and the nested transaction perfomed a roll-back. As there were no save-points involved, the whole transaction is rolled back. For the caller the DB.save seems to be successful. We discovered the error, when we reviewed the logs and see that there was a problem.

Our suggestion is, that exceptions, that happen in a "preCommit" or "postCommit" hook should always be rethrown up to the caller.
For "preRollback" and "postRollback" we are a bit unsure, and we keep the behaviour as it is (log exception and continue)

rPraml added a commit to FOCONIS/ebean that referenced this pull request Aug 10, 2023
@rbygrave rbygrave added the bug label Aug 14, 2023
@rbygrave rbygrave added this to the 13.20.2 milestone Aug 14, 2023
@rbygrave rbygrave merged commit 1decbe0 into ebean-orm:master Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants