Bug in AbstractReactiveTransactionManager?

[SledgeHammer01]

Hi,

I'm working on a Spring / Hibernate Reactive integration since r2dbc isn't very useful at the moment without ORM support. My integration does use Mutiny / Vert.x under the covers due to that being a Hibernate Reactive thing, but that's irrelevant for this issue.

It's pretty much done and working, but during building out my unit tests, I ran across a bug in AbstractReactiveTransactionManager error handling that also affects R2dbcTransactionManager.

Scenario 1 "happy path" -- works as expected

doBegin() ok
do work ok
doCommit() ok

Scenario 2 "fails in 'work'" -- works as expected

doBegin() ok
do work fails
doCommit() is NOT called
doRollback() IS called and is ok

Exception is bubbled up to controller

Scenario 3 "fails in doCommit()" -- works as expected (covered here: https://github.com/spring-projects/spring-framework/blob/main/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerTests.java#L223)

doBegin() ok
do work ok
doCommit() fails
doRollback() IS called and is ok

Exception is bubbled up to controller.

Scenario 4 "fails in doCommit() AND doRollback()" -- doesn't work

doBegin() ok
do work ok
doCommit() fails
doRollback() fails

This scenario causes the code to end up here:

https://github.com/spring-projects/spring-framework/blob/main/spring-tx/src/main/java/org/springframework/transaction/reactive/AbstractReactiveTransactionManager.java#L619

and eventually we get an exception here ("Transaction synchronization is not active"):

https://github.com/spring-projects/spring-framework/blob/main/spring-tx/src/main/java/org/springframework/transaction/reactive/TransactionSynchronizationManager.java#L229

And we get the incorrect IllegalStateException bubbling up to the controller.

EDIT: As a test hack, I used AspectJ to bypass AbstractReactiveTransactionManager::triggerAfterCompletion() and that lets the exception bubble up.

I'm thinking, in AbstractReactiveTransactionManager::triggerAfterCompletion() you need to add another check for completionStatus != 2?

[mipo256]

Thank you for reaching out @SledgeHammer01!

I have a couple of notes here:

1. AbstractReactiveTransactionManager is not really the part of spring-data-jdbc/spring-data-r2dbc, but rather it is the part of the spring-r2dbc module in the spring-framework
2. Nevertheless, the minimal reproducible example would help very much
3. What is the expected behavior as you see it @SledgeHammer01?

I guess the issue should be re-opened in the spring-framework, but the final decision is on the project's lead CC: @schauder

[SledgeHammer01]

Ok, thanks, I'll reopen there.