Future.onSuccess swallowing exceptions

[galegofer]

Hi there,

I am having a kind of issue or misusage, when I do something like:

Future.fromCompletableFuture(sendDocumentUploadedEvent(event))
        .onSuccess(result -> validateUpload(event, timestamps)) // throws exception
        .get();

if any exceptions happens inside the .onSuccess it's not able to leave the block being the Future successful, and the caller not being able to realized about the situation.

But if you do:

Future.fromCompletableFuture(sendDocumentUploadedEvent(event))
        .map(result -> {
            validateUpload(event, timestamps); // throws exception
            return result; 
        )
        .get();

In this case the exception is not lost, and we get the exception leaving the map block code.

My question is , is this the behavior expected for the .onSuccess method?, I understand the .onSuccess idea is mainly for log purposes, but I don't get why to eat the exceptions.

[danieldietrich]

Yes, this is expected behavior. Scala for example makes it more clear. There onComplete (resp. onSuccess) returns Unit, which is similar to void in Java. We return the same Future instance in order to allow chaining of multiple actions.

However, Scala does not completely swallow exceptions. They happen within another Thread and are printed to System.err I think. If you want to handle errors, you need to wrap your action in a Try.

We might think about exposing exceptions to the error output stream. How does Java's ForkJoinTask behave here?

---

Scala example:

[galegofer]

I think in this case we shouldn't just shallow the exception but some how log it, or make it noticeable to the developer.

I am not familiar with the ForkJoinTask behavior but with CompletableFuture.thenAccept, if you get an exception on the running thread it gets propagated.

Also I think we should update javadoc for onComplete, because right now is not stating that it may shallow the exceptions.

Performs the action once the Future is complete.
Parameters:
action - An action to be performed when this future is complete.
Returns:
this Future
Throws:
[NullPointerException](https://docs.oracle.com/javase/8/docs/api/java/lang/NullPointerException.html?is-external=true) - if action is null.

[danieldietrich]

@galegofer That makes sense, thanks!

[galegofer]

Thanks to you for creating this amazing library, lets's hope Vavr 1.0.0 will see the light ;)