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

Change in noRollbackFor transaction behavior with H2 database 2.x #28679

Closed
kiranshm973 opened this issue Jun 22, 2022 · 11 comments
Closed

Change in noRollbackFor transaction behavior with H2 database 2.x #28679

kiranshm973 opened this issue Jun 22, 2022 · 11 comments
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx)

Comments

@kiranshm973
Copy link

kiranshm973 commented Jun 22, 2022

Affects: 5.3.x

Issue identified while upgrading from Spring 5.0.x to 5.3.x

public class CustomException extends RuntimeException {
   
   public CustomException(String message, Throwable th) {
       super(message, th);
   }
}
public interface UserDAO {
   void updateUser();
}
@Repository
public class UserDAOImpl implements UserDAO {
   
   // inject entity manager (hibernate provider underneath)

   @Override
   public void updateUser() {
       // update db throws some runtime exception (org.hibernate.HibernateException for example)
   }
}
public interface MyService {
   void makeUpdate();
}
@Service
public class MyServiceImpl implements MyService {

   @Autowired
   UserDAO userDAO;

   @Transactional(noRollbackFor = CustomException.class)
   public void makeUpdate() {
        try {
            userDAO.updateUser();
        } catch (Exception ex) {
            Throwable rootCause = org.apache.commons.lang3.exception.ExceptionUtils.getRootCause(ex);
            if (rootCause instanceof SQLException) {
                 // get sql error code (specific error codes used in db stored procedures)
                 if (errorCode == x) {
                     throw new CustomException(ex.getMessage(), ex);
                 }
            }
            throw ex;
        }
   }
}

The above setup works as expected in Spring 5.0.x where the transaction is not rolled back when CustomException is thrown. But on upgrading to 5.3.x, an exception thrown in UserDAO marks the transaction for rollback. This looks to be working in Spring 5.0.x because Spring does not track / parse class UserDAO and method updateUser for @Transactional. So there is NO TransactionInfo (within TransactionAspectSupport) created before invoking the updateUser method and hence no attempt to mark the transaction for rollback.

In 5.3.x, Spring is able to source TransactionAttribute for UserDAO because SpringTransactionAnnotationParser identifies it as a candidate for @Transactional and hence TransactionInfo is created before the method is invoked, and the transaction is marked for rollback when RuntimeException is thrown.

This would work if updateUser method in UserDAO is annotated with @Transactional(noRollbackFor...), but I would like to start transaction in the service layer.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 22, 2022
@sbrannen sbrannen added the in: data Issues in data modules (jdbc, orm, oxm, tx) label Jun 22, 2022
@kiranshm973
Copy link
Author

created a sample project to demonstrate the issue https://github.com/ksm-973/spring-transaction-rollback-issue/tree/master/demo

Update springBootVersion in gradle.properties to 2.0.0.RELEASE (uses spring 5.0.4) and run UserServiceTest -> Test runs successfully
Update springBootVersion in gradle.properties to 2.7.0 (uses spring 5.3.20) and run UserServiceTest -> Test fails

@sbrannen
Copy link
Member

sbrannen commented Jun 23, 2022

Thanks for providing the demo project. We'll look into it.


As a side note, test classes and test methods have to be public with JUnit 4. Thus, anyone else trying out this project will need to add the missing public modifiers.

@sbrannen
Copy link
Member

FYI: if you set the Spring Boot version to 2.6.9 (in gradle.properties) and the Spring Framework version to 5.3.21 (in build.gradle as below), the test will still pass.

ext['spring-framework.version'] = '5.3.21'

Thus, it does not appear to be an issue with the core Spring Framework itself.

However, I have confirmed that the test fails as soon as you upgrade Spring Boot from 2.6.9 to 2.7.0.

@sbrannen sbrannen changed the title Spring upgrade 5.0.x to 5.3.x breaks transaction rollback behavior Change in noRollbackFor transaction behavior with Spring Boot 2.7 Jun 23, 2022
@sbrannen
Copy link
Member

After further analysis, I have determined that the upgrade to H2 2.x is causing the issue.

Running the test with Spring Boot 2.7.0, Spring Framework 5.3.21 and H2 1.4.200 passes.

ext['spring-framework.version'] = '5.3.21'
ext['h2.version'] = '1.4.200'

Switching h2.version to 2.1.212 causes the test to fail.

@sbrannen sbrannen changed the title Change in noRollbackFor transaction behavior with Spring Boot 2.7 Change in noRollbackFor transaction behavior with H2 database 2.x Jun 23, 2022
@kiranshm973
Copy link
Author

kiranshm973 commented Jun 23, 2022

Thanks for looking into this. Original issue was identified in a stand alone spring app which uses SQL Server database.

Spring: 5.3.20
JPA: 2.1
Hibernate: 5.2.17
sql jdbc driver: mssql-jdbc:8.2.0.jre8

Issue was possibly caused by the change for this #22420

@sbrannen sbrannen self-assigned this Jun 24, 2022
@sbrannen
Copy link
Member

The issue with the demo application is not an issue with Spring Framework or Spring Boot.

Rather, the issue pertains to Hibernate's support for H2 2.0 and the fact that the test catches and swallows all exceptions and that the service catches all exceptions. The latter two issues mask the first issue.

If you inspect the actual exceptions and log, you will see that there are numerous SQL exceptions. The following is the initial cause.

Caused by: org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "create table [*]user (id integer generated by default as identity, name varchar(255), primary key (id))"; expected "identifier"; SQL statement:
create table user (id integer generated by default as identity, name varchar(255), primary key (id)) [42001-214]

To fix the problem, you need to declare the user table with enclosing backticks as follows

@Entity
@Table(name = "`user`")
public class User {
  // ...
}

After doing that, your test should pass with Spring Boot 2.7.0, Spring Framework 5.3.21, and H2 2.1.214.

In light of that, I am closing this issue as invalid.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2022
@sbrannen
Copy link
Member

Thanks for looking into this. Original issue was identified in a stand alone spring app which uses SQL Server database.

Spring: 5.3.20 JPA: 2.1 Hibernate: 5.2.17 sql jdbc driver: mssql-jdbc:8.2.0.jre8

If you are able to provide us a sample application that reproduces the problem, feel free to share additional information in this issue and we will consider reopening it.

Thanks

@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 24, 2022
@kiranshm973
Copy link
Author

kiranshm973 commented Jun 28, 2022

Thank you!

I overlooked few details but I have now updated the demo project with the code setup where the tests work with 5.0.x but fails with 5.3.x.

https://github.com/kiranshm973/spring-transaction-rollback-issue/tree/master/demo

Issue seems to be happening when a @Transactional method is overridden. I agree the usage of Transactional annotation in the demo project is not optimal but wanted to point you to the behavior.

@sbrannen sbrannen reopened this Jul 1, 2022
@sbrannen sbrannen added status: waiting-for-triage An issue we've not yet triaged or decided on and removed status: invalid An issue that we don't feel is valid labels Jul 1, 2022
@sbrannen
Copy link
Member

sbrannen commented Jul 2, 2022

I think the change in behavior might be a result of commit 42d6d7e, due to the switch from getMergedAnnotationAttributes to findMergedAnnotationAttributes, but I have yet to verify that.

I just wanted to leave that note here in case somebody else investigates in depth before I do.

@sbrannen sbrannen removed their assignment Jul 13, 2022
@snicoll
Copy link
Member

snicoll commented Oct 2, 2023

I've updated the sample to a supported version and I am not sure I understand it, nor how the referred commit would be related. The createUser calls invoke the DAO with a transaction that should not rollback on NoRollbackException. I can see that the transaction source is parsed and a RuleBasedTransactionAttribute is created with the correct value for the exception.

However when the exception in the DAO is thrown, the transaction source is the one from the DAO, not the one that started the transaction. At this stage, I'd need some help from @jhoeller as I am not sure whether that's legit and what to investigate next.

And just to be clear @kiranshm973, this has nothing to do with an upgrade to H2, right?

@snicoll
Copy link
Member

snicoll commented Dec 28, 2023

Closing due to the lack of requested feedback.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2023
@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx)
Projects
None yet
Development

No branches or pull requests

4 participants