Re: [Prevayler-coders] Exception handling philosophy

[Justin T. S. <ju...@kr...>]

Howdy,

Explanation and examples together with responses...

Jacob Kjome wrote:

> > Don't expect checked and unchecked Exceptions to have different
> > semantic importance
>
> If this is what we assert, does it ever make sense to use checked exceptions
> since they are less flexible by their nature? Would we merely throw
> RuntimeExceptions and Errors and document the possibility in Javadoc @throws
> tags (at least for cases with higher probability) rather than including them
> in the throws clause of a methods?

Right, that's my inclination.

However, this particular principle refers more to client/3rd-party
code: Since there are so many different interpretations out there
about how checked and unchecked exceptions should be used, we
shouldn't rely on any particular interpretation when deciding what to
do with exceptions thrown by external code.

Example violation: The food taster assumes that a checked exception is
intentional, and allows the transaction through to the king; and
assumes that an unchecked exception is unintentional, and disallows
the transaction. In this case I guess it's okay, because the food
taster needs some kind of rule to follow, and it's documented (I
think), and it's not part of the "core" functionality, and I don't
like the food taster much anyway. :)

> > Never drop or wrap an Error
>
> Makes sense, and pretty easy to implement since you don't have to catch these
> in the first place. They are unexpected, unchecked, and don't extend
> java.lang.Exception. Just let'em fly.

In general, yes. There are one or two places that it makes sense to
catch an Error and do something, and this principle is a reminder to
*always* rethrow the original Error. Besides playing nice with
externally-generated Errors, following this principle throughout the
Prevayler code ensures that Prevayler itself can generate Errors to
indicate fatal conditions that will always be propagated to client
code.

One place where it makes sense to catch an Error is during transaction
execution: If an Error occurs, it is very likely to have been
nondeterministic (OutOfMemoryError, for example) and so the prevalent
system is probably in an inconsistent state. Any further operations
should be aborted.

> > Never throw a new checked Exception
>
> So, Prevayler won't have it's own checked exceptions. This is fine. But
> should we wrap checked exceptions coming from other API's in RuntimeExceptions
> to avoid any checked exceptions leaking into Prevayler's throws clauses? This
> might make sense? Then again, it could also be decided on a case-by-case
> basis as well. We might decide that, for instance, IOException is the only
> checked exception we allow to leak into Prevayler's API and wrap the rest in
> RuntimeExceptions. I'm not saying we should do that, specifically. I'm only
> using it as an example of a case-by-case basic approach.

Right, there are probably a couple of places it might be okay for
IOException to be declared. But the primary places that external code
is called are Transactions and Serializers. If a Query or
TransactionWithQuery throws a checked exception, it's just propagated
to the caller; we simply declare that Prevayler.execute() throws
Exception.

If a Serializer throws any kind of exception, it's another kind of
thing; same if a file operation throws an IOException: In either case,
persistence is no longer "transparent". That's what I call an
"abstraction violation", which would actually warrant an Error being
generated. I want client code to be able to assume that if an
Exception (checked or unchecked) is thrown from Prevayler.execute(),
it's because the Query or Transaction actually threw it.

> > Throw a PrevalenceError for any abstraction-violating occurrence in
> > operation
>
> Agree

The preceding example is the primary one that comes to mind for
"abstraction-violating". I think there are others, but that's what's
coming to mind right now. In that case, Prevayler would generate an
Error of some kind, saying in essence "oops, something went wrong with
journaling". The underlying exception would be set as the "cause" of
that Error, but the Error has its own distinct meaning -- it's not
just a "wrapper" for the exception to avoid declaring the exception.

> > Throw a RuntimeException for any configuration problems
>
> Agree

Basically, what I had in mind is to throw a standard RuntimeException
(things like IllegalArgumentException, NullPointerException, etc.) for
any problems detected *before* Prevayler has started up, while any
problems detected *after* Prevayler has started up likely warrant a
distinct subclass of PrevalenceError.

> > Never allow the system to continue operating in an inconsistent state

The case of an Error thrown from a Transaction, described above, is
one example. Another example is anything that should be "impossible",
such as transaction sequence numbers (a.k.a. system versions) out of
sequence. If any inconsistency is detected, I would want to throw a
PrevalenceError immediately.

> > Never hang
>
> Definitely agree

Example: PersistentJournal has a private method called hang(). Evil. :)

The proper thing to do is throw a PrevalenceError and set a flag or
something to prevent any further operations. Hanging the calling
thread is just nasty, as it doesn't allow any way to recover. (I
implemented this somewhere in my post-2.3 development. Pre-2.3, I at
least changed hang() so that it wouldn't hog the cpu as it did before.
It used to be while(true){yield}, but as of 2.3 it's
while(true){sleep}.)

Cheers,
Justin