1

This question is specific to Java and CompletableFuture.

If I have an async method like the one below,

CompletableFuture<String> get() {
    /* Step #1: Do some sync prep work */
    String s = doSomethingSync();

    /* Step #2: Do something async (returning CompletableFuture) */
    return doSomethingAsync(s);
}

If code in step #1 throws, callers of get() will get an exception before getting the CompletableFuture it returns, whereas if code inside the CompletableFuture returned in step #2 throws, callers will only get an exception when they interact with the returned CompletableFuture.

This suggests that callers of get() should write some intricate exception handling code to tackle both cases.

Here's an example of another async method, invokeGet(), that invokes get() and returns the length of the String it returns:

CompletableFutre<Integer> InvokeGet() {
    try {
        CompletableFuture future = get();
        return furure.handle((result, throwable) -> {
           if (throwable != null) {
               /* Handle exception thrown in step #2 in get(), e.g. rethrow */
           } else {
               return result.length();
           }
        });
    } catch (Exception e) {
        /* Handle exception thrown in step #1 in get() */
        /* Return some value or throw */
    }
}

My question is:

Is get() poorly written because it requires its callers to do this kind of intricate exception handling, or is this a usual and common pattern? Should async methods returning CompletableFutures confine themselves to returning faulted futures in case of errors so their callers don't have to write such error-handling code?

Ahmed A.Hamid
  • 258
  • 1
  • 2
  • 12
  • 2
    Create a `CompletableFuture` that either succeeds or is in error for the first part of your work. `thenApplyAsync` the async part. `return`. I would expect a method that returns a `CompletableFuture` to return one in an error condition if there is an error - I would not expect it to throw. Is short: yes it is badly written, and yes they should return faulted futures. – Boris the Spider Aug 16 '18 at 18:45
  • Also, any code that has `throws Exception` or `catch (Exception e)` is inherently wrong. Catching _all_ exceptions is always a very bad idea. – Boris the Spider Aug 17 '18 at 17:12
  • @BoristheSpider Even if it rethrows or wraps-and-throws? – Ahmed A.Hamid Aug 17 '18 at 17:26
  • Would would you do with a `NullPointerException` rewrapped? – Boris the Spider Aug 17 '18 at 17:27
  • I see. This code snippet was only meant for illustrative purposes but I remain grateful for the additional useful input. – Ahmed A.Hamid Aug 17 '18 at 17:31

2 Answers2

0

In short, it depends on your own implementation but it could be improved. Having the calling thread handle the exception could be beneficial in a case where you want to ignore, log, or react to the exception through a different thread (basic example here). However, a lot of patterns (seen in this posts answer) I've seen would have you contain the async function with a try-catch-block and re-throw a more useful to your application exception to be handled by the parent thread which I think is a bit better.

If you are looking for different exception handling, see this article for examples of different handling.

Impurity
  • 1,037
  • 2
  • 16
  • 31
0

I think it is appropriate to throw exceptions for argument validation, as the caller is not supposed to handle those – an illegal argument is a bug that has to be fixed.

However it is probably best to put other exceptions inside the returned CompletableFuture, so that the caller can use the standard CompletableFuture exception handling and chaining. Similarly, you wouldn't return a null future but a future completed with null. See also CompletableFuture already completed with an exception.

Didier L
  • 18,905
  • 10
  • 61
  • 103