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, exception bubbles up to controller
doBegin() ok do work fails doCommit() is NOT called doRollback() IS called and is ok
Scenario 3 "fails in doCommit()" -- works as expected, exception bubbled up to controller
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
Scenario 4 "fails in doCommit() AND doRollback()" -- doesn't work, wrong exception IllegalStateException gets bubbled up
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 when completionStatus == 2. This occurs because the syncs have already been cleared and TransactionSynchronizationManager::getSynchronizations() expects the syncs to still be there or it throws an exception.
https://github.com/spring-projects/spring-framework/blob/main/spring-tx/src/main/java/org/springframework/transaction/reactive/TransactionSynchronizationManager.java#L229
We get the incorrect IllegalStateException bubbling up to the controller.
Seems like maybe the intention was to cover this failure in one of these tests? But neither really cover both the commit and rollback failing:
https://github.com/spring-projects/spring-framework/blob/main/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerTests.java#L273-L316
EDIT: As a test hack, I used AspectJ to bypass AbstractReactiveTransactionManager::triggerAfterCompletion() and that lets the correct exception bubble up.
I'm thinking, in AbstractReactiveTransactionManager::triggerAfterCompletion() you need to add another check for completionStatus != 2?
