Ensure reactive transaction rollback on commit error #30096
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonbasle
added
in: data
This change fixes a situation where error handling was skipped during `processCommit()` in case the `doCommit()` failed. The error handling was set up via an `onErrorResume` operator that was nested inside a `then(...)`, applied to an inner `Mono.empty()`. As a consequence, it would never receive an error signal (effectively decoupling the onErrorResume from the main chain). This change simply moves the error handling back one level up. It also simplifies the `doCommit` code a bit by getting rid of the steps that artificially introduce a `Mono<Object>` return type, which is not really needed. A pre-existing test was missing the fact that the rollback didn't occur, which is now fixed. Another dedicated test is introduced building upon the `ReactiveTestTransactionManager` class. Closes spring-projectsgh-28968 Closes spring-projectsgh-30096
simonbasle
force-pushed
the
28968-r2dbcRollbackOnCommitError
branch
from
8abee3d
to
c27e297
Compare
sdeleuze
approved these changes
Mar 10, 2023
sdeleuze
pushed a commit
to sdeleuze/spring-framework
that referenced
this pull request
Mar 10, 2023
This change fixes a situation where error handling was skipped during `processCommit()` in case the `doCommit()` failed. The error handling was set up via an `onErrorResume` operator that was nested inside a `then(...)`, applied to an inner `Mono.empty()`. As a consequence, it would never receive an error signal (effectively decoupling the onErrorResume from the main chain). This change simply moves the error handling back one level up. It also simplifies the `doCommit` code a bit by getting rid of the steps that artificially introduce a `Mono<Object>` return type, which is not really needed. A pre-existing test was missing the fact that the rollback didn't occur, which is now fixed. Another dedicated test is introduced building upon the `ReactiveTestTransactionManager` class. Closes spring-projectsgh-30096
|
Thanks @mp911de and @simonbasle for fixing this! |
mdeinum
pushed a commit
to mdeinum/spring-framework
that referenced
this pull request
Jun 29, 2023
This change fixes a situation where error handling was skipped during `processCommit()` in case the `doCommit()` failed. The error handling was set up via an `onErrorResume` operator that was nested inside a `then(...)`, applied to an inner `Mono.empty()`. As a consequence, it would never receive an error signal (effectively decoupling the onErrorResume from the main chain). This change simply moves the error handling back one level up. It also simplifies the `doCommit` code a bit by getting rid of the steps that artificially introduce a `Mono<Object>` return type, which is not really needed. A pre-existing test was missing the fact that the rollback didn't occur, which is now fixed. Another dedicated test is introduced building upon the `ReactiveTestTransactionManager` class. Closes spring-projectsgh-30096
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change fixes a situation where error handling was skipped during
processCommit()in case thedoCommit()failed. The error handlingwas set up via an
onErrorResumeoperator that was nested inside athen(...), applied to an innerMono.empty(). As a consequence,it would never receive an error signal (effectively decoupling the
onErrorResume from the main chain).
This change simply moves the error handling back one level up. It also
simplifies the
doCommitcode a bit by getting rid of the steps thatartificially introduce a
Mono<Object>return type, which is not reallyneeded.