0

Having the following scenario using Spring Framework

@Transactional
processRequest() {
    createOrder();
}

@Transactional
createOrder() {
    ...
    try {
        saveRow();
    } catch (SaveNotAllowedException e) {
        // Handle the expected problem
        ...
        log.info("Save was not allowed...");

        // We have to do this otherwise we get UnexpectedRollbackException
        TransactionAspectSupport.currentTransactionStatus().setRollbackOnly();
    }
}

@Transactional(rollbackFor = SaveNotAllowedException.class)
saveRow() throws SaveNotAllowedException {
    // Updating row in database is not allowed because of some business condition
    throw new SaveNotAllowedException();
}

Please note: Methods are public and in different classes. Parameters and irrelevant stuff omitted. SaveNotAllowedException is checked exception.

On the method saveRow I declare that I expect rollback on the checked exception. In the method createOrder I catch that exception and do a relevant work to process that case. But since this is an expected rollback, I would expect that Spring also treats it as expected rollback and gives me a go.

Instead what happens is that Spring throws UnexpectedRollbackException when method createOrder returns. To solve the problem I have to add setRollbackOnly() to the catch block. This tells Spring that I really expect the expected exception to do the rollback :-). From the Javadoc I get the feeling that the setRollbackOnly is supposed to be used for some internal use and is not really the official way to do it.

The same problem happens for unchecked exceptions (extending RuntimeException) e.g. EmptyResultDataAccessException. Documentation says that the rollback is expected and automatic for unchecked exceptions. OK, but then why does it still give UnexpectedRollbackException in that case?

The question is why does Spring treat the expected exception as unexpected? What is the official recommended solution to treat this case?

The documentation is not clear on this. Having read some explanation here UnexpectedRollbackException - a full scenario analysis and here Transaction marked as rollback only: How do I find the cause does not shed any more light on it.

Some people suggest this is a special case. To my understanding this is complete opposite, this is actually the usual scenario. Having business code, calling some method throwing business exception and checking it to alter the behavior in expected way seems pretty standard to me. So why I have to do a special step calling obscure API setRollbackOnly() to save me from crashing?

Community
  • 1
  • 1
Sevio Wizatti
  • 23
  • 4
  • 1
    Because of your `catch`, the transaction is already marked as rollback only, but your transaction (the outer `@Transactional` ) never sees that because you are catching the exception. For that everything is ok and it tries to commit then runs into the fact the transaction is rollback only and that leads to an error. – M. Deinum Mar 06 '17 at 19:16
  • The outer `@Transactional` does not have the declared rollback exception so I don't see why this would be/should be relevant. I would imagine the aop wrapper would see the inner `@Transactional` having declared rollback exception and at the same time performing rollback because of this exception equals having expected rollback automatically, ergo not complaining about unexpected rollback. – Sevio Wizatti Mar 07 '17 at 09:08
  • 1
    The point is there is only 1 transaction but due to your rules on your inner `@Transactional` it already sets it to rollback only. Because the outer `@Transactional` never sees the exception (due to your catch) it tries to commit, resulting in an error. You should remove your inner `@Transactional` or just rethrow the exception to have it rolled back properly. The main point is your catch block is destroying proper tx handling. – M. Deinum Mar 07 '17 at 10:47
  • Removing inner `@Transactional` disables rollback and commits the transaction silently, which is not what you want. – Sevio Wizatti Mar 10 '17 at 15:22

1 Answers1

1

The expected exception is treated as unexpected because the decision about it is made in AOP proxy only when returning from the last @Transactional method, the one which created the transaction. The proxy must see the exception being propagated and having declared rollback for it.

To prevent the UnexpectedRollbackException the checked exception must be caught in a code which is outside of transaction, i.e. in the code which is called after the proxy closes the transaction.

In the original question example, the transaction starts on processRequest() which is called from a framework and can't obviously throw any checked exceptions. So in this case the exception must be caught before.

Besides the already presented solution with setRollbackOnly() in the question there are two more solutions, which don't require use of setRollbackOnly().

Please note: Methods are public and in different classes. Parameters and irrelevant stuff omitted. Exceptions are checked exceptions.

Solution 1.

Create new transaction on createorder() using @Transactional(propagation = Propagation.REQUIRES_NEW, rollbackFor = SaveNotAllowedException.class). Catch the exception in the processRequest().

@Transactional
processRequest() {
    ...
    try {
      createOrder();
    } catch (SaveNotAllowedException e) {
        // Handle the expected problem
        ...
        log.info("Save was not allowed...");
    }

    try {
      backupOrder();
    } catch (AnotherException e) {
        ...
    }       
}

@Transactional(propagation = Propagation.REQUIRES_NEW, 
rollbackFor = SaveNotAllowedException.class)
createOrder() throws SaveNotAllowedException {
    ...
    saveRow();
}

@Transactional(propagation = Propagation.REQUIRES_NEW, 
rollbackFor = AnotherException.class)
backupOrder() throws AnotherException {
    ...
}

@Transactional
saveRow() throws SaveNotAllowedException {
    // Updating row in database is not allowed because of some business condition
    throw new SaveNotAllowedException();
}

Solution 2.

Remove @Transactional from processRequest(). Declare rollback on createorder() using @Transactional(rollbackFor = SaveNotAllowedException.class). Catch the exception in the processRequest().

processRequest() {
    ...
    try {
      createOrder();
    } catch (SaveNotAllowedException e) {
        // Handle the expected problem
        ...
        log.info("Save was not allowed...");
    }

    try {
      backupOrder();
    } catch (AnotherException e) {
        ...
    }       
}

@Transactional(rollbackFor = SaveNotAllowedException.class)
createOrder() throws SaveNotAllowedException {
    ...
    saveRow();
}

@Transactional(rollbackFor = AnotherException.class)
backupOrder() throws AnotherException {
    ...
}

@Transactional
saveRow() throws SaveNotAllowedException {
    // Updating row in database is not allowed because of some business condition
    throw new SaveNotAllowedException();
}

But solutions 1 and 2 are probably not what you want. Likely you want to have the whole process (createOrder() and backupOrder()) in one transaction and not create a new ones. Alternative would be to create another transactional helper service method called from processRequest() which would call createOrder and backupOrder in one transaction. We would need to create as many of those helper methods as many combinations of transactions we have.

It is sad that these intricate details are not mentioned in Spring documentation. The closest information is here http://docs.spring.io/spring-framework/docs/current/spring-framework-reference/html/transaction.html#tx-propagation-required but it is in need of more elaboration or at least rewording.

Sevio Wizatti
  • 23
  • 4