Returning 500 internal error when CompleteableFuture throws exception

Question

I have a rest controller that has an endpoint:

@GET
@Path("/reindex-record")
public String reindexRecord(@QueryParam("id") String id) {
    if (StringUtils.isEmpty(id)) {
        CompletableFuture.runAsync(
            () -> runWithException(Reindexer::reindexAll));
    } else {
        CompletableFuture.runAsync(() -> runWithException(
            () -> Reindexer.reindexOne(id)));
    }

    // return "ok" or throw WebApplciationException from runWithException method below
}

and here is my wrapper method - both methods - reindexAll and reindexOne throw checked exceptions so decided to use wrapper method and interface:

public interface RunnableWithException {
    void run() throws Exception; 
}

private void runWithException(RunnableWithException task) {
    try {
        task.run();
    } catch (Exception e) {
        log.error("Error occured during async task execution", e);
        throw new WebApplicationException(
            Response.status(Response.Status.INTERNAL_SERVER_ERROR)
                .entity("Internal error occurred").build());
    }
}

The problem is that I want to run this task asnychronously using CompleteableFuture and give a response only after given task is done or if there was an error throw WebApplicationException with INTERNAL_SERVER_ERROR status.

How would you implement that in my use-case with if/else?

EDIT: As of now I have this method:

@GET
@Path("/reindex-record")
public String reindexRecord(@QueryParam("id") String id) throws ExecutionException,
    InterruptedException {
    CompletableFuture<Void> task;
    if (StringUtils.isEmpty(id)) {
        task = CompletableFuture.runAsync(
            () -> runWithException(Reindexer::reindexAll));
    } else {
        task = CompletableFuture.runAsync(() -> runWithException(
            () -> Reindexer.reindexOne(id)));
    }
    return task.thenApply(x -> "ok")
        .exceptionally(throwable -> {
            log.error("Error occured during async task execution", throwable);
            throw new WebApplicationException(Response.status(Response.Status.SERVICE_UNAVAILABLE)
                                                  .entity("Internal error occurred. Try again later")
                                                  .build());

        }).get();

}

But if error is thrown by any of Reindexer methods I'm still getting status 500 with data:

{
"code": 500,
"message": "There was an error processing your request. It has been logged (ID 03f09a62b62b1649)."

}

Instead of 503 defined in my exceptionally block. Using dropwizard with JAX-RS if that matters.

You can't. Either you wait/block. Or the client needs to send another request, to check if it has failed. — Lino, Jul 18 '18 at 10:34
I'm ok with blocking - those methods just send some stuff to the Rabbit queue so it's very fast operation but can throw some unexpected errors. — doublemc, Jul 18 '18 at 10:46
This may be helpful http://humansreadcode.com/spring-boot-completablefuture/ — Beri, Jul 18 '18 at 10:49

score 1 · Answer 1 · answered Jul 18 '18 at 10:52

You can change the body of your method to this:

@GET
@Path("/reindex-record")
public String reindexRecord(@QueryParam("id") String id) {
    final CompletableFuture<Void> future;
    if (StringUtils.isEmpty(id)) {
        future = CompletableFuture.runAsync(
            () -> runWithException(Reindexer::reindexAll));
    } else {
        future = CompletableFuture.runAsync(
            () -> runWithException(() -> Reindexer.reindexOne(id)));
    }

    // this will block
    future.get();

    return "ok";
}

By storing the future, you can then call the get() method on it, which will block until the future is finished.

From the javadoc of CompletableFuture.get():

Waits if necessary for this future to complete, and then returns its result.

It's not doing what I want actually. If exception is thrown by reindexAll then the user doesn't get INTERNAL_SERVER_ERROE — doublemc, Jul 18 '18 at 12:59

score 1 · Answer 2 · answered Jul 20 '18 at 08:50

The problem is that you are using exceptionally() for something it wasn't intended. CompletableFutureis intented to be used in a chain of CompletableFutures where the output of one feeds into the next one. What happens if one of the CompletableFutures throws an exception? You can use exceptionally to catch that and return a new fallback ComletableFuture for the next step of the chain to use instead.

You're not doing that, you just throw an WebApplicationException. The CompleteableFuture views that as a failure in the chain and wraps your WebApplicationException inside an ExecutionException. Dropwizard only sees the ExecutionException (it doesn't inspect any wrapped ones) and throws the generic 500 response.

The answer is just do the future.get(); in @Lino's answer, but wrapped in a try...catch block for ExecutionException, and then throw WebApplicationException from the catch.

try {
    // this will block
    future.get();
} catch (ExecutioException) {
    throw new WebApplicationException(
        Response.status(Response.Status.INTERNAL_SERVER_ERROR)
            .entity("Internal error occurred").build());
}

return "ok";

You might be able to shorten the whole throw new WebApplicationException(... to simple throw new InternalServerErrorException()

Returning 500 internal error when CompleteableFuture throws exception

2 Answers2