CODEWITHSUNDEEP
springspring-bootjava-8completable-future
riccardo.cardin
riccardo.cardin

Reputation: 8353

Spring controller advice does not correctly handle a CompletableFuture completed exceptionally

I am using Spring Boot 1.5, and I have a controller that executes asynchronously, returning a CompletableFuture<User>.

@RestController
@RequestMapping("/users")
public class UserController {

    @Autowired
    private final UserService service;

    @GetMapping("/{id}/address")
    public CompletableFuture<Address> getAddress(@PathVariable String id) {
        return service.findById(id).thenApply(User::getAddress);
    }
}

The method UserService.findById can throw a UserNotFoundException. So, I develop dedicated controller advice.

@ControllerAdvice(assignableTypes = UserController .class)
public class UserExceptionAdvice {
    @ExceptionHandler(UserNotFoundException.class)
    @ResponseStatus(HttpStatus.NOT_FOUND)
    @ResponseBody
    public String handleUserNotFoundException(UserNotFoundException ex) {
        return ex.getMessage();
    }
}

The problem is that tests are not passing returning an HTTP 500 status and not a 404 status in case of an unknown user request to the controller.

What's going on?

Upvotes: 11

Views: 5790

Answers (2)

SurprisedFrog
SurprisedFrog

Reputation: 11

For those of you who still run into trouble with this : even though Spring correctly unwraps the ExecutionException, it doesn't work if you have a handler for the type "Exception", which gets chosen to handle ExecutionException, and not the handler for the underlying cause.

The solution : create a second ControllerAdvice with the "Exception" handler, and put @Order(Ordered.HIGHEST_PRECEDENCE) on your regular handler. That way, your regular handler will go first, and your second ControllerAdvice will act as a catch all.

Upvotes: 1

riccardo.cardin
riccardo.cardin

Reputation: 8353

The problem is due to how a completed exceptionally CompletableFuture handles the exception in subsequent stages.

As stated in the CompletableFuture javadoc

[..] if a stage's computation terminates abruptly with an (unchecked) exception or error, then all dependent stages requiring its completion complete exceptionally as well, with a CompletionException holding the exception as its cause. [..]

In my case, the thenApply method creates a new instance of CompletionStage that wraps with a CompletionException the original UserNotFoundException :(

Sadly, the controller advice does not perform any unwrapping operation. Zalando developers also found this problem: Async CompletableFuture append errors

So, it seems to be not a good idea to use CompletableFuture and controller advice to implement asynchronous controllers in Spring.

A partial solution is to remap a CompletableFuture<T> to a DeferredResult<T>. In this blog, an implementation of a possible Adapter was given.

public class DeferredResults {

    private DeferredResults() {}

    public static <T> DeferredResult<T> from(final CompletableFuture<T> future) {
        final DeferredResult<T> deferred = new DeferredResult<>();
        future.thenAccept(deferred::setResult);
        future.exceptionally(ex -> {
            if (ex instanceof CompletionException) {
                deferred.setErrorResult(ex.getCause());
            } else {
                deferred.setErrorResult(ex);
            }
            return null;
        });
        return deferred;
    }
}

So, my original controller would change to the following.

@GetMapping("/{id}/address")
public DeferredResult<Address> getAddress(@PathVariable String id) {
    return DeferredResults.from(service.findById(id).thenApply(User::getAddress));
}

I cannot understand why Spring natively supports CompletableFuture as return values of a controller, but it does not handle correctly in controller advice classes.

Hope it helps.

Upvotes: 12

Related Questions