CompletableFuture swallows exceptions?

Question

I've been playing around with CompletableFuture and noticed a strange thing.

String url = "http://google.com";

CompletableFuture<String> contentsCF = readPageCF(url);
CompletableFuture<List<String>> linksCF = contentsCF.thenApply(_4_CompletableFutures::getLinks);

linksCF.thenAccept(list -> {
    assertThat(list, not(empty()));
});

linksCF.get();

If, in my thenAccept call, the assertion fails, the exception is not propagated. I tried something even uglier then:

linksCF.thenAccept(list -> {
    String a = null;
    System.out.println(a.toString());
});

nothing happens, no exception is propagated. I tried using methods like handle and others related to exceptions in CompletableFutures, but failed - none is propagating the exception as expected.

When I debugged the CompletableFuture, it does catch the exception like this:

final void internalComplete(T v, Throwable ex) {
    if (result == null)
        UNSAFE.compareAndSwapObject
            (this, RESULT, null,
             (ex == null) ? (v == null) ? NIL : v :
             new AltResult((ex instanceof CompletionException) ? ex :
                           new CompletionException(ex)));
    postComplete(); // help out even if not triggered
}

and nothing else.

I'm on JDK 1.8.0_05 x64, Windows 7.

Am I missing something here?

Solution 1

The problem is you never request to receive the results of your call to linksCF.thenAccept(..).

Your call to linksCF.get() will wait for the results of the execution in your chain. But it will only return the results of then linksCF future. This doesn't include the results of your assertion.

linksCF.thenAccept(..) will return a new CompletableFuture instance. To get the exception thrown call get() or check the exception status with isCompletedExceptionally() on the newly return CompletableFuture instance.

CompletableFuture<Void> acceptedCF = linksCF.thenAccept(list -> {
    assertThat(list, not(empty()));
});

acceptedCF.exceptionally(th -> {
    // will be executed when there is an exception.
    System.out.println(th);
    return null;
});
acceptedCF.get(); // will throw ExecutionException once results are available

Alternative?

CompletableFuture<List<String>> appliedCF = linksCF.thenApply(list -> {
    assertThat(list, not(empty()));
    return list;
});

appliedCF.exceptionally(th -> {
    // will be executed when there is an exception.
    System.out.println(th);
    return Coolections.emptyList();
});
appliedCF.get(); // will throw ExecutionException once results are available

Solution 2

Although the question is basically already answered by Gregor Koukkoullis (+1), here is a MCVE that I created to test this.

There are several options for obtaining the actual exception that caused the problem internally. However, I don't see why calling get on the future that is returned by thenAccept should be an issue. In doubt, you could also use thenApply with the identity function and use a nice fluent pattern, like in

List<String> list = 
    readPage().
    thenApply(CompletableFutureTest::getLinks).
    thenApply(t -> {
        // check assertion here
        return t;
    }).get();

But maybe there's a particular reason why you want to avoid this.

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.function.Supplier;

public class CompletableFutureTest
{
    public static void main(String[] args) 
        throws InterruptedException, ExecutionException
    {
        CompletableFuture<String> contentsCF = readPage();
        CompletableFuture<List<String>> linksCF = 
            contentsCF.thenApply(CompletableFutureTest::getLinks);

        CompletableFuture<Void> completionStage = linksCF.thenAccept(list -> 
        {
            String a = null;
            System.out.println(a.toString());
        });        

        // This will NOT cause an exception to be thrown, because
        // the part that was passed to "thenAccept" will NOT be
        // evaluated (it will be executed, but the exception will
        // not show up)
        List<String> result = linksCF.get();
        System.out.println("Got "+result);


        // This will cause the exception to be thrown and
        // wrapped into an ExecutionException. The cause
        // of this ExecutionException can be obtained:
        try
        {
            completionStage.get();
        }
        catch (ExecutionException e)
        {
            System.out.println("Caught "+e);
            Throwable cause = e.getCause();
            System.out.println("cause: "+cause);
        }

        // Alternatively, the exception may be handled by
        // the future directly:
        completionStage.exceptionally(e -> 
        { 
            System.out.println("Future exceptionally finished: "+e);
            return null; 
        });

        try
        {
            completionStage.get();
        }
        catch (Throwable t)
        {
            System.out.println("Already handled by the future "+t);
        }

    }

    private static List<String> getLinks(String s)
    {
        System.out.println("Getting links...");
        List<String> links = new ArrayList<String>();
        for (int i=0; i<10; i++)
        {
            links.add("link"+i);
        }
        dummySleep(1000);
        return links;
    }

    private static CompletableFuture<String> readPage()
    {
        return CompletableFuture.supplyAsync(new Supplier<String>() 
        {
            @Override
            public String get() 
            {
                System.out.println("Getting page...");
                dummySleep(1000);
                return "page";
            }
        });
    }

    private static void dummySleep(int ms)
    {
        try
        {
            Thread.sleep(ms);
        }
        catch (InterruptedException e)
        {
            e.printStackTrace();
            Thread.currentThread().interrupt();
        }
    }
}

Solution 3

If, in my thenAccept call, the assertion fails, the exception is not propagated.

The continuation that you register with thenAccept() is a separate task from the linksCF future. The linksCF task completed successfully; there is no error for it to report. It has its final value. An exception thrown by linksCF should only indicate a problem producing the result of linksCF; if some other piece of code that consumes the result throws, that does not indicate a failure to produce the result.

To observe an exception that happens in a continuation, you must observe the CompletableFuture of the continuation.

correct. but 1) I should not be forced to call get() - one of the points of the new constructs; 2) it's wrapped in an ExecutionException

What if you wanted to hand the result off to multiple, independent continuations using thenAccept()? If one of those continuations were to throw, why should that impact the parent, or the other continuations?

If you want to treat linksCF as a node in a chain and observe the result (and any exceptions) that happen within the chain, then you should call get() on the last link in the chain.

You can avoid the checked ExecutionException by using join() instead of get(), which will wrap the error in an unchecked CompletionException (but it is still wrapped).

Solution 4

The answers here helped me to manage exception in CompletableFuture, using "exceptionnaly" method, but it missed a basic example, so here is one, inspired from Marco13 answer:

/**
 * Make a future launch an exception in the accept.
 *
 * This will simulate:
 *  - a readPage service called asynchronously that return a String after 1 second
 *  - a call to that service that uses the result then throw (eventually) an exception, to be processed by the exceptionnaly method.
 *
 */
public class CompletableFutureTest2
{
    public static void main(String[] args)
        throws InterruptedException, ExecutionException
    {
        CompletableFuture<String> future = readPage();

        CompletableFuture<Void> future2 = future.thenAccept(page->{
            System.out.println(page);
            throw new IllegalArgumentException("unexpected exception");
        });

        future2.exceptionally(e->{
          e.printStackTrace(System.err);
          return null;
        });

    }

    private static CompletableFuture<String> readPage()
    {

      CompletableFuture<String> future = new CompletableFuture<>();
      new Thread(()->{
        try {
          Thread.sleep(1000);
        } catch (InterruptedException e) {
        }

        // FUTURE: normal process
        future.complete("page");

      }).start();
        return future;
    }


}

The mistake to avoid is to call "exceptionnaly" on the 1st future (the variable future in my code) instead of the future returned by the "thenAccept" which contains the lambda that may throw an exception (the variable future2 in my code). .

View more solutions

Comments

[maciej]

that won't do it. future is still in progress when we're asking about its exceptional completion.

[maciej]

That doesn't help either: acceptedCF.whenComplete((Void v, Throwable t) -> { if (t != null) { fail(); } }); Thread.sleep(5000); [i seem to have problems formatting code in comments...] I am still not able to see a way of propagating the exception to the outside world.

[maciej]

I am aware of all the calls you're suggesting - still, none of them makes my tests fail. Which is not the biggest issue - the issue is - i want the outside world to be able to get the exception. Is there no better way than trying to grab a reference to the exception from outside of the lambda and rethrow manually?

[Gregor Koukkoullis]

When you call acceptedCF.get() the exception will be thrown once processed. At least for me.

[maciej]

correct. but 1) I should not be forced to call get() - one of the points of the new constructs; 2) it's wrapped in an ExecutionException

[Gregor Koukkoullis]

I see. Maybe your call to thenAccept() to do the assertion is not optimal. You could do the assertion in a thenApply(). This way the caller who reads the List<String> will see the exception?

[maciej]

all in all, I accept your premise - this does make my tests break. but it's far from ideal and imposes trade-offs.

[maciej]

with regard to this issue and exceptions, thenAccept and thenApply are the same thing - just different approach processing of the result. no other difference.

[Gregor Koukkoullis]

Of course it is the same thing regarding processing. But there is a big difference regarding the return value. You would like to have the outside world see the exception as I understand and you don't like to call get() on the acceptedFuture. The outside world might like to get the List<String> and don't care about the acceptedFuture. So if you use thenApply() the outside world can transparently access the List<String> via get() and still see the exception. But I think I don't understand how you use the future from the outside world and your test case.

[Gregor Koukkoullis]

Regarding your comment in the code: I think the part passed to thenAccept() is actually executed when calling linksCF.get(); The results are just not "evaluated". Might make no difference in that case, but be of importance in others.

[Marco13]

(For other confused readers: The last quote was from my answer ;-)). Of course there may be cases where one would like to add multiple consumers, and thus, not consider it as a "chain", but rather as a "tree". But the comment referred only to the original example.

[maciej]

Gregor - thanks for sticking up with me on this discussion. It looks like my confusion was caused by expecting something else from those constructs than they really offer. Also, the number of options offered by CF was a bit overwhelming to tackle all at once. I wanted to use them in a way they were not designed for. I have a clearer view now.

[maciej]

that pretty much sums it up - I cannot live without get() (in my use case), whereas I wanted to avoid it for a variety of reasons.