Vavr: Why are Checked* classes declared to throw Throwable

Created on 11 Oct 2017  路  28Comments  路  Source: vavr-io/vavr

All Throwable instances must either be an Exception or an Error. The javadoc for Error states that:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.

However, because Vavr lambda classes generally declare to throw Throwable, developers end up writing this sort of thing:

public void doSomething(CheckedRunnable thing) throws MyException {
  try {
    thing.run();
  } catch (SomeExpectedException e) {
     // recover/log/rethrow/whatever
  } catch (Throwable e) { // have to catch because CheckedRunnable declares to throw
    throw new IllegalStateException("I only expected SomeExpectedException", e);
  }
}

But we've now ended up with any Error being caught and handled by the application. Obviously this can be fixed by doing something like:

  } catch (Exception e) {
    throw new IllegalStateException("I only expected SomeExpectedException", e);
  } catch (Throwable t) {
    throw (Error) t; // we know this must now be an Error
  }

But as Error doesn't have to (or indeed, based on the javadoc, shouldn't) be caught, we really shouldn't have to be doing this.

This would be avoided if the Checked* signatures instead were:

@FunctionalInterface
public interface CheckedRunnable {
    void run() throws Exception;
}

And the resulting calling code would be safer.

Aside: I'd actually prefer that the signatures were something like:

@FunctionalInterface
public interface CheckedRunnable<E extends Exception> {
    void run() throws E;
}

or perhaps, to remind users that a lambda may throw a RuntimeException:

@FunctionalInterface
public interface CheckedRunnable<E extends Exception> {
    void run() throws E, RuntimeException;
}
question

Most helpful comment

@danieldietrich I concur with others. Throwable is just the most common super-type to all exceptions but that doesn't mean it should be caught because it includes Errors which should _never_ be caught.

The VERY first sentence states this:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions. The ThreadDeath error, though a "normal" condition, is also a subclass of Error because most applications should not try to catch it.

You're the owner @danieldietrich of vavr and if you wanna make it scala-envious that's your choice. But I think you are dismissing a lot of veteran experience and bluntly stated advice from the original Java documentation going back to 1.0.

All 28 comments

I don't like how Throwable is caught either. It's can hide real significant problems - problems like what you point out. using Try.of(() -> connectToDb()) makes sense to catch errors and try again or us a circuit breaker. But an ExceptionInInitializerError means that a static block in a class threw an error and the class failed to load. Based experience it's nearly impossible to determine why (the underlying problem usually vanishes).

Errors are _bad_ and are by designed meant to not be caught:

  1. OutOfMemoryError - You can't increase JVM Heap space from inside the JVM, so now what?
  2. UnsatifiedLinkError - An attempt at a native library load failed. Was the library missing? Is it binary incompatible (which, based on experience reports as not found).
  3. NoSuchMethodError - Version compatibility error, always. This is non-recoverable.

The list goes on.

Yes I am not sure why everything is designed around Throwable's. sounds like a wrong design for me. Throwable catch both exceptions and errors.

@mrpotes
Suggestion with adding generic type wouldn't work well, as once you start to combine these functions together you will end up with Exception or Throwable just because in Java it is not possible to deduce resulting type (union of exception X1 and its subtypes with exception X2 and its subtypes) of exception that can be thrown.

As for catching Throwable in Checked* this is mostly because Vavr closely resembles behaviour of Scala's Try

@KTannenberg I am not sure why we should follow Scala's Try? After all, this is a Java library.
I don't believe one can ignore the points raised by @chb0github

Catching Throwable is probably the most horrible thing in my view since it catches all Exceptions and Errors that are not meant to be caught that said if you have a better solution please suggest. But given the way it is right now it makes exception handling really hard. And Cyclops seems to have better Exception handling mechanisms.

Cyclops requires you to explicitly define all exceptions that can be throw by your code below, because of that they don't need java to figure out types on its own.

I can agree, that catching Throwable is simple in Scala because it doesn't have silly concept of checked/unchecked exceptions, so you just deal with these Throwable subtypes you do care and let other stuff be rethrown.

Unfortunately there are libraries that misuse Errors and use them in places where some Exception should have been used.

@KTannenberg Sure but again Vavr is not a Scala library right I can understand the motivations and the theory drawn from Scala or in General functional programming perspective but at the same time building the library in Java that doesn't work well for exceptions is also not the right thing to do. For example I have the following question here https://github.com/vavr-io/vavr/issues/2142 and it just a pain to even try and come up with a solution and so far there is no solution for my question. As a user of the library it doesn't give a good experience.

"Cyclops requires you to explicitly define all exceptions that can be throw by your code below, because of that they don't need java to figure out types on its own."

Sure that works right! anything wrong with that?!

Finally, please suggest if you have a better approach. It just doesn't make a lot of sense to say Scala does things in a certain way so Java should do the same. Catching Throwable is a serious concern for Java programmers. If this library is designed to help Java Programmers then I think it should make appropriate changes or additions. My 2 cents.

@KTannenberg Also why you say checked/unchecked exceptions are silly? How can one enforce the client of an API to think about how to deal with Exceptions at compile time? This in my experience is very useful since I need to think about how to handle a certain Exception at compile time and as a consequence this saves lot of debugging time.

If the argument is that dealing everything at run time leads to less boilerplate code then comparison really comes down is static typing vs dynamic typing. And both has pros and cons.

Dynamic typing in general takes more debugging time than static typing. And static typing is very useful to catch errors early and takes less debugging time.

I'm not saying that Cyclops did anything wrong, in fact this is probably easiest, most user-friendly and reliable solution that they choose for their version of `Try.

Also there is nothing like "everybody should everything the way it is done in scala", in fact scala have a lot of problems as well. Why I was mentioning scala in a first place is because initially javaslang (previous name of vavr) was heavily inspired by scala's std library.

@KTannenberg Agreed. Please suggest alternatives.

@mrpotes shall we implement the interface you suggested and make it part of the next release? can someone make aware of release process for Vavr?

probably @danieldietrich Should speak to the release process and if we will allow the changes. I mean, we can talk all day but unless someone forks the code or you know a PR will make it's way in to the core then it's all academic.

Vavr doesn't need to _be_ a java implementation of Scala's immutable libraries. The functionality vavr provides fills a very real gap in the java ecosystem and can/should vary to be ideal for the language. The Scala immutable libs are a fantastic inspiration but should be taken as only that. Not following idiomatic java will/does have ramifications everywhere.

@kant111 @KTannenberg @chb0github @mrpotes Thank you for the great discussion. I decided not to use a generic type parameter for the exception type mainly for one reason:

For simple cases it might work well but in general a call might throw multiple exception, like InterruptedException and IOException at the same time. We are not able to define a common exception in a practicable way using Java's type system - we will always end up with Throwable.

In fact Java is one of the rare languages that have checked exception, most language do not and it is generally seen as a error in language design - at least it is controversial discussed.

Using Try, we are aware about the presence of exceptions. There are many useful methods to handle such cases - recently I added more onFailure variants. I think the current approach is sufficient. Also I think having only one specific exception type is not sufficient.

I like this discussion (and hope it goes on) but I will close this ticket because we will not change the way we use exception types.

@danieldietrich I'm not sure I agree with your assertion that "we will always end up with Throwable" - as per the original comment, you will always end up with Exception - you shouldn't ever end up with Throwable unless there is some poorly written code trying to handle Errors.

I totally get the reasons for not using generics, and that's fine, but I really think you should consider changing Throwable everywhere to Exception. For me, vavr becomes a waste of time and effort without this.

Perhaps I could challenge you to give an example that shows why Throwable is actually required in vavr, where Exception would not be sufficient?

@danieldietrich I concur with others. Throwable is just the most common super-type to all exceptions but that doesn't mean it should be caught because it includes Errors which should _never_ be caught.

The VERY first sentence states this:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions. The ThreadDeath error, though a "normal" condition, is also a subclass of Error because most applications should not try to catch it.

You're the owner @danieldietrich of vavr and if you wanna make it scala-envious that's your choice. But I think you are dismissing a lot of veteran experience and bluntly stated advice from the original Java documentation going back to 1.0.

https://stackoverflow.com/questions/613954/the-case-against-checked-exceptions

1) Catching Throwable's is outright madness. There are enough examples pointed out here on why.
2) Checked vs Unchecked exceptions really just boils down to is static type vs dynamic type of a language. Sure unchecked exception results in better expressive code. so does any dynamic type language with lots of syntax sugar than say any static type language. Python is way more expressive than any static type language (you name it).

I personally like static type enforcement because it helps me debug things way faster instead of relying on a programmer's comments. How many programmers do we know in the industry care to write comments? Moreover isn't it more safe to rely on a compiler than relying on some random programmer?

so This is just a preference. It's like asking is English better than German or German better than English.

@kant111

1) Try can catch more than more type - specifying one type is not sufficient / it is useless. Here is an example where the most common type is Exception

import io.vavr.control.Try;

import java.util.Arrays;
import java.util.List;
import java.util.concurrent.*;

import static io.vavr.API.println;

public class Test {

    public static void main(String[] args) {

        final ExecutorService executorService = ForkJoinPool.commonPool();
        final List<Callable<String>> myTasks = Arrays.asList(
                () -> "Ok!",
                () -> { throw new Error("Meep!");
        });

        { // Java variant 1
            List<Future<String>> res = null;
            try {
                res = executorService.invokeAll(myTasks);
            } catch (InterruptedException | NullPointerException | RejectedExecutionException e) {
                // e is of type Exception
                if (e instanceof InterruptedException) {
                    System.out.println("InterruptedException: " + e);
                }
                if (e instanceof NullPointerException) {
                    System.out.println("NullPointerException: " + e);
                }
                if (e instanceof RejectedExecutionException) {
                    System.out.println("RejectedExecutionException: " + e);
                }
            }
            if (res != null) {
                System.out.println("Success");
            }
        }

        { // Java variant 2
            List<Future<String>> res = null;
            try {
                res = executorService.invokeAll(myTasks);
            } catch (InterruptedException e) {
                System.out.println("InterruptedException: " + e);
            } catch (NullPointerException e) {
                System.out.println("NullPointerException: " + e);
            } catch (RejectedExecutionException e) {
                System.out.println("RejectedExecutionException: " + e);
            }
            if (res != null) {
                System.out.println("Success");
            }
        }

        { // Vavr
            Try.of(() -> executorService.invokeAll(myTasks))
                    .onFailure(InterruptedException.class, e -> println("InterruptedException: " + e))
                    .onFailure(NullPointerException.class, e -> println("NullPointerException: " + e))
                    .onFailure(RejectedExecutionException.class, e -> println("RejectedExecutionException: " + e))
                    .onSuccess(res -> println("Success!"));
        }
    }
}

2) @chb0github You argue about some kind of _convention_. I agree that Error should not be catched, but in practice 3rd party libs extend Error where it is really an Exception. There are tons of examples, e.g. in Calypso - an important trading system that exists since the 90s. But there are tons of other examples.

3) Please note that we define Errors that are not recoverable. E.g. Try rethrows OutOfMemory exceptions (beside others)

I understand your thoughts but as I said - we will keep it as-is. As always: if in doubt, align to Scala. This is our design premise.

@danieldietrich Yes, Thanks for adding those onFailure variants I have to wait until 1.0 I guess. It does look better but looks is not everything. instanceof has a performance impact as well as you may already know. variant #2 is quite good as also to be honest.

Also, Why do we care about systems or libraries that are built in 90's?! Any better examples?!
Try rethrows OutOfMemory exceptions (beside others): This indeed seems convoluted and a big stretch.

I think you should Advertise Vavr as "Scala inspired Java library" or something. I initially thought the library draws core concepts from Functional programming in general not just from Scala. I didn't know Vavr falls back on Scala heavily. If I knew I would have avoided using Vavr library because If I am so deeply in love with Scala then I might as well use Scala than using a Scala inspired library :)

Anyways since I don't see much room for discussion. I am out of this thread as well! Good luck!

@danieldietrich I'm trying to understand this issue a bit better, so here's my question.

What is the intended purpose of the Checked* functional interfaces?

To me, from the moment I saw those classes, it was pretty self-evident that they are declared like this with the intended purpose that we can use lambdas that throw checked exceptions throughout the vavr API without cluttering our own lambdas with exception handling code. That's pretty nice and helpful, but the people in this thread are right in the sense that, in order to accomplish the above goal, you only need to declare to throw Exception and you pretty much allowed all those lambdas to remain uncluttered from catching exceptions.

It there any other reason for the existence of the Checked* interfaces besides the above that I missed? Because if there's none, I'll have to agree that declaring to throw Throwable instead of Exception is unnecessary and actually harmful in this case. The harm I see in declaring to throw Throwable is that, there are practically two direct subclasses of Throwable: Exception (a checked exception type) and Error (an unchecked one). Java doesn't force you to catch Errors, on the contrary, it advises you in the docs not to. But if a method declares to throw Throwable you have no other choice but to catch Throwable, even if you intend to do anything meaningful with the Exception subclass branch only. Declaring to throw Throwable forces you to do an instanceof check on the throwable and rethrow it in case it's an Error subclass and you don't want to deal with it, which is probably the overwhelming majority of the cases. Declaring to throw Throwable makes reusing the Checked* interfaces in client code counterproductive. Of course, we can always create our own version of those functional interfaces with throwing just Exception, but then we cannot reuse those instances both in our own client code and through the vavr API.

In the rare cases when some API abuses the java exception class hierarchy by directly extending Throwable instead of Exception (both cases result in a throwable that needs to be catched), I think we could live with those very rare cases where we would need to explicitly write lambda expressions that are forced to handle that special Throwable subclass. Those cases would be _very_ rare, and one could even argue that it's actually beneficial that it shows the user of such an API of the bad design choice, and maybe provide an incentive to refactor the offending API or use an alternative one if that's possible at all.

EDIT: from looking at the code, I see that the Try intends to wrap all Throwables in a Failure, except 4 explicit exception types that are considered fatal and were decided to let them propagate by using the sneaky throws trick. I'd like to point out, that changing the _signature_ of the throwing methods in the Checked* interfaces would still allow the same functionality, practically Try would be unaffected, because you can always catch a broader exception type from what is declared, so it wouldn't affect the current behavior of Try. The question here is more like: what are we _forced_ to catch when interfacing with the Checked* interfaces.

I think the whole purpose of this thread is to make the Checked* interfaces more reusable in other contexts besides interfacing with vavr code, and _declaring_ to throw Exception instead of Throwable would do that without sacrificing any of the design goals of either Try or the Checked* interfaces themselves. It's all about what are we _forcing_ on client code using instances of these interfaces while _not limiting_ internal vavr code to do anyting less that what it currently does.

PS: sorry for writing a novel ^^ 馃槼

Hi @nfekete, thank you for the nice evening-read 馃槃

You made good points. Especially I like the separation of concerns of regarding the functional interfaces and the sum types like Try.

I think it is a good idea to ignore the rare cases where 3rd party library implementors made horrible design decisions by using sub-classes of Error as non-fatal exceptions.

Thx for being determined in this case!

It feels good to make useful contribution to an awesome library like vavr is, so the pleasure is mine :)

The whole project is based on contributions. The changes might look small but the effort in terms of _thoughts_ are huge. This kind of _energy_ doesn't get lost - it makes the library useful and also simple I think.

Based on https://github.com/vavr-io/vavr/pull/2360 https://github.com/vavr-io/vavr/pull/2352 I see the agreement reached in this thread so far has not or only partially led to the fix of making it Exception instead of Throwable?

Does this mean that a Try underlying failure will remain Throwable, and that it's up to the client application to fix this by pushing the Throwable back into a more constrained type (i.e. Exception)? Or should I deal with things differently?

I understand the tensions of fixing the API vs not breaking existing code and can't oversee the tradeoffs in this situation, but I was hoping this Throwable thing would be abandoned before 1.0...

So... there still seem to be unclarity.

I want to showcase different scenarios in order to show that it is not a good idea to...

1) Catch a specific exception type like "the other functional library Abc..."

(Look at FAIL! comments)

screenshot 2019-02-12 at 23 32 04

2) Just catch Exception and rethrow remaining throwables wrapped as RuntimeException

I understand that we could ignore Errors in the type signature because they are treated as unchecked exceptions and catched automatically by Try (if not considered fatal).

But let me be clear, there do not only exist Error and Exception as subclasses of Throwable.

馃馃徏馃挜 JAVA DOES ALLOW SUBCLASSES OF THROWABLE 馃挜馃馃徏

// Someone thought it is a good idea to build a distinct sub-tree of business exceptions
class MyDistinctBusinessException extends Throwable {
}

// Someone thought it is a good idea to build a distinct sub-tree of internal errors
class MyDistinctInternalException extends Throwable {
}

I know that it isn't good practice to extend Throwable but it is possible and therefore it happens.

We gain nothing by handling only Exception on our type signatures. The developer has to make instanceof checks nonetheless. You are still able just to use one of:

Try#recover(Class<X> exceptionType, CheckedFunction<? super X, ? extends T> recoveryFunction)

Try#recoverWith(Class<X> exceptionType, CheckedFunction<? super X, ? extends Try<? extends T>> recoveryFunction))

Maybe there will be also a

Try#onFailure(Class<X> exceptionType, Consumer<? super X> action)

where X extends Throwable.

When using Try, it should not make any difference for you:

  • Recover the exceptions you expect
  • Handle all remaining (technical exception) by logging them. You have to! Or do you want to just ignore any unexpected RuntimeExceptions?

Please don't miss that io.vavr.CheckedFunction0..8 will throw Exception (not Throwable)!

You can use these CheckedFunctions outside of the context of Try.


@feliksik

and that it's up to the client application to fix this by pushing the Throwable back into a more constrained type

Please give me an example. Where do you need to push the Throwable back into a more constrained type? The 'recover' methods will be able to give you the expected exception type (see above).

Please give me an example. Where do you need to push the Throwable back into a more constrained type? The 'recover' methods will be able to give you the expected exception type (see above).

I think this happens when I want to go back to throwing exceptions, which most likely only happens in the context of some non-vavr calling context; e.g. the code below where I must use exceptions to properly implement the interface:

    // note guava CacheLoader is declared with "throws Exception"
    class MyEntityLoader extends jersey.repackaged.com.google.common.cache.CacheLoader<UUID, MyEntity> {
        public MyEntity load(UUID key) {

            return underlyingRepo.lookupMyEntity(key)
                    .getOrElseThrow(f -> 
                             // have to wrap here, to make it not a Throwable... but 
                            new RuntimeException());
        }
    }

But I guess this is an easy solution, and some interface might require RuntimeException, IOException or some other constrained type... And indeed, as we cannot make vavr use a genericized exception type, I guess it's impossible to solve it, or not worth it....

@feliksik thank you for the example!

Vavr 1.0 will wrap the original exception in a NonFatalException (which extends RuntimeException).

See source.

(Don't get irritated because of the @Deprecated annotation. That is still in the flow/work in progress.)

Look ma, Scala lang contains a custom Exception that extends Throwable:

Screenshot 2019-03-23 at 18 02 38

(However, it should not be caught...)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

manu-m picture manu-m  路  6Comments

noorulhaq picture noorulhaq  路  5Comments

civitz picture civitz  路  6Comments

roookeee picture roookeee  路  5Comments

skestle picture skestle  路  3Comments