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

Returning a CompletableFuture.failure() from an async, transactional method should mark the transaction as roll back #30018

Closed
odrotbohm opened this issue Feb 23, 2023 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@odrotbohm
Copy link
Member

Returning a value from an @Async method usually involves a CompletableFuture wrapper of the value. If such a method is declared like this:

@Async
@Transactional
CompletableFuture<…> doSomething() {

  return someCondition
    ? CompletableFuture.completedFuture(…)
    : CompletedFuture.failedFuture(…);
}

The failed CF could be the result of handling an exception as we might not want to bubble up the async execution exception handling chain, but rather hand into the client so that it can react to the failure by e.g. ….exceptionally(…).

Unfortunately, for transactional methods, returning that failed CF instance would not roll back the transaction, as that currently only reacts to exceptions being thrown and Vavr Failure instances. Failed CF instances are actually very similar to the latter, which is why it would be nice if our transaction handling worked the same when returning failed CFs.

@jhoeller jhoeller self-assigned this Feb 23, 2023
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Feb 23, 2023
@jhoeller jhoeller added this to the 6.1.0-M1 milestone Feb 23, 2023
@edyda99
Copy link
Contributor

edyda99 commented Mar 14, 2023

Hello,

If the project maintainers decide to implement this feature, I would be happy to take on the task of implementing it in a way that considers future implementations other than vavr and CompletableFuture.

I understand that this is a task for the core team, but I have a good understanding of this part of the code and know exactly where to implement the feature. Additionally, I will make sure to thoroughly test my code to ensure that it doesn't introduce any new bugs or issues.

@jhoeller
Copy link
Contributor

jhoeller commented May 2, 2023

@edyda99 this has been in the works between Ollie and me already, leading up to the creation of this issue in February, and the end result of this is about to be committed today. Thanks for volunteering, in any case!

The current revision of this code is aligned with JDK 19/20's Future.resultNow/exceptionNow default methods which operate under similar assumptions, having been introduced with structured concurrency in mind. In that sense, our Future treatment here can also be seen as part of our wider JDK 21 alignment in Spring Framework 6.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants