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

Clarify rollback-on consistency between DefaultTransactionAttribute and TransactionTemplate [SPR-14994] #19560

Closed
spring-projects-issues opened this issue Dec 8, 2016 · 0 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Dec 8, 2016

Gary Russell opened SPR-14994 and commented

The DefaultTransactionAttribute.rollbackOn() javadoc says it is "consistent with the TransactionTemplate 's default behavior"...

/**
 * The default behavior is as with EJB: rollback on unchecked exception.
 * Additionally attempt to rollback on Error.
 * <p>This is consistent with TransactionTemplate's default behavior.
 */
@Override
public boolean rollbackOn(Throwable ex) {
	return (ex instanceof RuntimeException || ex instanceof Error);
}

Since #11027, this is no longer strictly correct, in that a rollback will occur for any exception there.

Of course, as seen in that JIRA attachment, actually throwing a checked exception is a bit convoluted (reflection on a ctor that throws a checked exception), so perhaps this is ok.

However when looking at the code it appears incorrect and I had to dig back into the commit history to understand what's going on; perhaps adding a comment to the template code where we catch Exception (now Throwable since #18901) would help.

Or, Juergen Hoeller, feel free to just close this if you don't think any changes are warranted.


Affects: 4.2.8, 4.3.4

Issue Links:

  • #11027 TransactionTemplate leaks transactions when TransactionCallback propagates undeclared checked exception
  • #18901 After-completion callback not triggered for custom Throwable subclass

Referenced from: commits 953bc18, 0296c7c, 2de488e

Backported to: 4.2.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.