7

Question: how can I directly throw a custom exception from .exceptionally()?

List<CompletableFuture<Object>> futures =
    tasks.stream()
        .map(task -> CompletableFuture.supplyAsync(() -> businessLogic(task))
        .exceptionally(ex -> {
                if (ex instanceof BusinessException) return null;

                //TODO how to throw a custom exception here??
                throw new BadRequestException("at least one async task had an exception");
        }))
        .collect(Collectors.toList());

try {
    List<Object> results = futures.stream()
        .map(CompletableFuture::join)
        .collect(Collectors.toList());
} catch (CompletionException e) {
        if (e.getCause() instanceof RuntimeException) {
              throw (RuntimeException) e.getCause();
        }
        throw new RuntimeException(e.getCause());
}

Problem: I just always get a CompletionException whose ex.getCause() is instanceof BadRequestException.

Is that possible at all?

membersound
  • 81,582
  • 193
  • 585
  • 1,120
  • I don't think it's possible, every exception will be wrapped into a `CompletionException` – Eugene Feb 28 '18 at 13:10
  • `CompletableFuture` will eventuall call `encodeThrowable(Throwable x)` where `x` is your exception and return `new AltResult((x instanceof CompletionException) ? x : new CompletionException(x));` - hence you'll always get a `CompletionException`. The only thing I see to directly get your exception would be to make it extend `CompletionException`. – Thomas Feb 28 '18 at 13:17
  • So I'd have to wrap the code above in an additional `try {..} catch (CompletionException e)` and rethrow the underlying exception outside? Please see my update: that makes it very boilerplate, but there is probably no better solution? – membersound Feb 28 '18 at 13:52
  • You can also use `ex.addSuppressed(new BadRequestException("at least one async task had an exception"))` and then access `Arrays.stream(e.getSuppressed()).filter(...)` – HRgiger Feb 28 '18 at 14:02
  • Thb that's not less boilerplate... – membersound Feb 28 '18 at 14:11
  • 1
    Why are you constructing a new `RuntimeException` to wrap the cause, when the `CompletionException` is already a runtime exception wrapping the cause? Just use `catch(CompletionException e) { throw e.getCause() instanceof RuntimeException? (RuntimeException)e.getCause(): e; }` – Holger Mar 01 '18 at 11:12
  • Because the wrapped exception (=the cause) might not be a `RuntimeException`itself... – membersound Mar 01 '18 at 11:19
  • 1
    It looks like you missed the point of my comment. Throwing a `RuntimeException` wrapping whatever throwable is no improvement over throwing an `ExecutionException` wrapping the same throwable. – Holger Mar 01 '18 at 11:25
  • 1
    You just reversed your entire question. So you want to handle `BusinessException` by replacing with `null` and handle any other exception by throwing `BadRequestException`? That should be reflected by the `catch` clause as well. But just say… is this really what you want? It contradicts all your previous comments. – Holger Mar 01 '18 at 14:58

2 Answers2

11

As said by Didier L, exceptions thrown by the functions (or generally exceptions that completed a CompletableFuture) are always wrapped in a CompletionException (unless they are already a CompletionException or CancellationException).

But note that your code becomes much simpler when not even trying to translate the exception via exceptionally:

List<CompletableFuture<Object>> futures =
    tasks.stream()
        .map(task -> CompletableFuture.supplyAsync(() -> businessLogic(task)))
        .collect(Collectors.toList());
try {
    List<Object> results = futures.stream()
        .map(CompletableFuture::join)
        .collect(Collectors.toList());
} catch (CompletionException e) {
    throw e.getCause() instanceof BusinessException?
        new BadRequestException("at least one async task had an exception"): e;
}

or

… catch (CompletionException e) {
    throw e.getCause() instanceof BusinessException?
        new BadRequestException("at least one async task had an exception"):
        e.getCause() instanceof BusinessException? (RuntimeException)e.getCause(): e;
}

Since exceptionally’s primary purpose is translating an exception to a non-exceptional result value, using it for translating the exception to another thrown exception was not the best fit anyway and it also needed an instanceof. So performing this translation in the catch clause saves you from another translation step.

Holger
  • 285,553
  • 42
  • 434
  • 765
  • Well but as you see above I only want to rethrow the exception if instanceof `BusinessException', and ignore (=`return null`) otherwise in exceptionally. – membersound Mar 01 '18 at 14:03
  • 1
    In your question’s code, the `catch` clause rethrows all unexpected exceptions rather than swallowing them. Why do you want to substitute them with `null` at one place (not discussing the code smell of that), but rethrow at another? That’s inconsistent. – Holger Mar 01 '18 at 14:32
  • See that in `.exceptionally()` I'm swallowing any exceptions except `BusinessExecption`! Meaning: I want to ignore any exceptions but `BusinessExceptions`. Those should be rethrown outside of the future (and not being rethrown as a nested `CompletionException`). If I won't analyze the exception inside `.exceptionally()`, I would not be able to continue the futures if an exception can be ignored. Thus I cannot simply catch all of them outside. – membersound Mar 01 '18 at 14:34
  • 1
    Exactly. “any except a particular one” means “anything unexpected”, including `RuntimeException`s (indicating bugs) or `Error`s (indicating serious problems with the environment). Swallowing those is a big code smell. But besides that, these problems may also occur at other places in your chain than in the `Supplier`, hence may not get swallowed but rethrown by you `catch` clause. That’s inconsistent. – Holger Mar 01 '18 at 14:38
  • Ok I see the point. Then my example is maybe not chosen at it's best. So let me rewrite it that `BusinessException` is the one to be ignored, and all unexpected errors should be rethrown. – membersound Mar 01 '18 at 14:42
  • 2
    But that would be so easy to solve by wrapping `businessLogic` without needing any effort anywhere else: `CompletableFuture.supplyAsync(() -> { try { return businessLogic(task); } catch(BusinessException ex) { return null; } })`… There’s no way how `CompletableFuture` could make that simpler. – Holger Mar 01 '18 at 15:03
  • Ok, I just thought catching and handling the exception should take place in `.exceptionall()`, and not in `supplyAsync()` itself. – membersound Mar 01 '18 at 15:49
  • 2
    That’s tricky, as the function passed to `exceptionally` receives a `Throwable`, but is only allowed to throw unchecked exceptions, so re-throwing is quiet limited. As said in the answer, `exceptionally` is intended to (always) replace an exception with a fall-back value rather than re-throwing. Building a re-throwing feature atop of that would be very complicated. See also [this answer](https://stackoverflow.com/a/44411450/2711488). – Holger Mar 01 '18 at 15:54
  • 1
    Ok if so, I'm probably misusing `exceptionally()` here and will try-catch my businessexception inside the supplier directly. – membersound Mar 01 '18 at 20:29
4

This is not possible. The Javadoc of join() clearly states:

Returns the result value when complete, or throws an (unchecked) exception if completed exceptionally. To better conform with the use of common functional forms, if a computation involved in the completion of this CompletableFuture threw an exception, this method throws an (unchecked) CompletionException with the underlying exception as its cause.

(emphasis is mine)

Holger
  • 285,553
  • 42
  • 434
  • 765
Didier L
  • 18,905
  • 10
  • 61
  • 103