Provider.get() should throw a subclass of RuntimeException, not a RuntimeException

[GoogleCodeExporter]

I think that even if the specs mention that catching this exception
shouldn't be the responsibility of the callers, it might be the
responsibility of the caller to catch anything that went wrong in their
code, such as an IllegalArgumentException in the constructor thrown by the
provider. 

Therefore shouldn't it be interesting to create a subclass of
RuntimeException, like javax.inject.InjectionException?

This would allow the programmer to have a specific behavior in the case of
an injection failure. For instance:

Provider<T> provider = ...;
try {
  return provider.get();
} catch (InjectionException e) {
  throw new InternalError("Oops, forgot to check this injection case", e);
} catch (RuntimeException e) {
  // Sadly swallow the exception
  return null;
}

Furthermore, I don't remind having seen a single method in the JavaSE
throwing a RuntimeException: only subclasses of it (but I might be mistaken
or I did not delve enough the javadoc).

Finally, very few are the packages that don't define their own exception,
just to say: "Hello! _We_ have a problem: not the other package you might
think of".

Original issue reported on code.google.com by ogregoire on 7 Sep 2009 at 11:00

[GoogleCodeExporter]

Is this a so bad idea? It seems that nobody cares to comment.

Original comment by ogregoire on 9 Sep 2009 at 5:11

[GoogleCodeExporter]

In Guice 1.0, we sometimes threw ProvisionException and we sometimes threw 
application exceptions. In the wild, code that needed 
to recover from provision failures looked like this:

  public void entertain(Fozzy bear) {
    KnockKnockJoke joke;
    try {
      joke = jokeProvider.get();
    } catch (MySqlDatabaseException e) {
      // no big deal
      joke = WHY_DID_THE_CHICKEN_CROSS_THE_ROAD;
    }
    joke.tell(bear);
  }

This code breaks encapsulation and depends on the implementation details of the 
provider. This was fine until we changed the 
MySQL joke database to a Bigtable, and our fault-tolerant try/catch was no 
longer tolerating faults.

In Guice 2, I attempted to fix this by changing Provider.get() to always throw 
ProvisionException if something threw in userland. 
The client code became this:

  public void entertain(Fozzy bear) {
    KnockKnockJoke joke;
    try {
      joke = jokeProvider.get();
    } catch (ProvisionException e) {
      // still no big deal
      joke = WHY_DID_THE_CHICKEN_CROSS_THE_ROAD;
    }
    joke.tell(bear);
  }

This is convenient because now it survives changes to the provider 
behind-the-scenes. But it's also terrible: if our 
BigtableJokeProvider has a programming bug and always throws 
NullPointerExceptions, they're silently swallowed. 

To recap
  * Throwing user exceptions is bad because it violates encapsulation
  * Wrapping user exceptions is bad because clients can swallow programming bugs. Much as I hate to admit it, "catch 
(ProvisionException e)" is no better than "catch (Exception e)"

For what it's worth, ProvisionException does include the original exception for 
clients who want to figure out exactly what 
broke.

Original comment by limpbizkit on 9 Sep 2009 at 10:46

[GoogleCodeExporter]

(I'm going to take several arguments taken from the book Effective Java 2nd 
Edition
by J. Bloch who you should know since he also works for Google.)

First, we all agree: only unchecked exceptions are to be used, since they 
concern
only programming issues, and that usually a wrong injection is not recoverable. 
The
reason why the unchecked exception occurred is to be programmatically fixed, 
while
the reason why a checked exception occurred can be foreseen and worked-around
preventively. (item 58)

Well the part I don't agree with is the argument "wrapping user exceptions is 
bad".
The main reason why I don't agree is that a user wants to know why the error
occurred. One reason could be that indeed his class fails. But the software 
providing
the injection isn't always Guice or Spring, and can also fail in some cases. 
This
case isn't completely covered by the javax.inject package. It just states: 
guys, do a
try{...}catch(RuntimeException e) {...}, we don't care. No, this is incorrect
Let's see to a package that has a lot of implementations... org.xml.sax, or
org.w3c.dom. All implementations of these packages use the provided exceptions. 
I
don't see why this package (of which the purpose is to be a standard package) 
does
not allow a specific exception for its implementations to be caught.
Let's say I use Guice and develop, and catch the ProvisionException. But I find 
Guice
not complete and want to switch to web-beans. What? I'll have to change all my 
code
to change it to support WebbeansOrWhateverInjectionException: not only the jar 
file
and adapt the configuration? This is not the least surprise principle I would 
expect.

A solution for catching the user exception is - like you explain - the 
possibility to
wrap it. So, as you explain Guice does, the provider (of javax.inject) can 
include
the original exception! So by doing this, we have a fully working 
encapsulation, a
respect to the item 61 of Eff Java, and the price is: "clients can swallow
programming bugs". If I admitted the price, I'd say... "hum let's think about 
it".
But I don't: because the client can _always_ swallow programming bugs, and the
exception is still an unchecked exception, so it still means "programming 
error".
The real downside of all this is only the fact that checked exceptions are 
wrapped in
an unchecked exception, but it is anyway if the thrown exception is an instance 
of
RuntimeException like written in the doc.

Logically also: if I can't inject an object, I want to know that I can't, then 
check
why. That's why the exception encapsulation exists: "this error occurred 
because...".

Some might think about the item 60: "favor the use of standard exceptions". But
javax.inject is going to be a standard: it is the goal of JSR330. There 
shouldn't be
any reason why RuntimeException is preferred over InjectionException (or
ProvisionException).

Finally, with the two arguments you provide, the most dangerous one is the
encapsulation violation. Of the two evils, chose the less dangerous - in this 
case,
the wrapping.

Sorry if everything is not completely clean, but it is nearly 2.00 AM over 
here. If
clarifications are needed, just let me know.

Original comment by ogregoire on 9 Sep 2009 at 11:58

[GoogleCodeExporter]

This is a duplicate of issue #3.

Also, Josh Bloch already reviewed this design and agrees with the decision.

Original comment by crazybob...@gmail.com on 10 Sep 2009 at 10:54

• Changed state: Duplicate