microsling: Exceptions

[Felix Meschberger <fm...@gmail.com>]

Hi all,

Many methods in microsling are defined to throw Exception. I think this
is bad practice and should not be done. Rather I would suggest, that all
methods allowed to throw checked exceptions should throw SlingException.

Regards
Felix

Re: microsling: Exceptions

[Thomas Mueller <th...@gmail.com>]

The public API should never throw 'Exception' because it forces the
API user to catch RuntimeExceptions, and he may not want to do that. I
think a good API only throws one kind of Exception, for example
SlingException (maybe one or two subclasses, but the JCR API has way
too many in my view).

The implementation could internally use 'throws Exception' to avoid
'throws a, b, c, d', and convert the Exception at the outermost level
(in all methods that are part of the API). This should be done in any
case for logging (even catch Throwable). RuntimeExceptions and Errors
can be re-thrown as is (use a converter utility to avoid copy &
paste).

Still I wouldn't internally throw Exception, because you want to add
context information to the exception. Example:

copyFile(String fileA, String fileB) throws SlingException {
 try {
   ...
 } catch(IOException e) {
   throw new SlingException("Can not copy " + fileA + " to " + fileB, e);
 }

Important is to include the original exception (this is often forgotten).

Thomas

Re: microsling: Exceptions

[Felix Meschberger <fm...@gmail.com>]

Examples of this you may see in the microsling DefaultSlingServlet and
StreamServlet.....

The DefaultSlingServlet has to catch and threat the ServletException and
IOException separately just to be able to handle the Exception declared
by SlingRequestContext.

Regards
Felix


Am Freitag, den 19.10.2007, 15:37 +0200 schrieb Carsten Ziegeler:
> Felix Meschberger wrote:
> > 
> > Another point is, that whenever a method is declared throws Exception I
> > am force to catch(Exception), which is something a definitely do not
> > like because I might want to let RuntimeException pass through.
> > Otherwise I would also be required to declare throws Exception and may
> > not even be able to describe what exact exception I am expected to
> > throw ...
> > 
> While I usually tend to just throw Exception (as Bertrand does), this
> usually creates nasty code for clients of the api. Imagine you implement
> an interface from another api which has a method that declares to throw
> exception A and B. Now in the implementation of the method you call the
> ms api (ms = microsling :) ) which throws exception. So you have to test
> if the ms api call throws exception A or B and pass this through or if
> it is any other exception, you have to wrap it.
> If the ms api just throws sling exception you catch it, wrap it and
> that's it.
> 
> Carsten
> 

Re: microsling: Exceptions

[Carsten Ziegeler <cz...@apache.org>]

Felix Meschberger wrote:
> 
> Another point is, that whenever a method is declared throws Exception I
> am force to catch(Exception), which is something a definitely do not
> like because I might want to let RuntimeException pass through.
> Otherwise I would also be required to declare throws Exception and may
> not even be able to describe what exact exception I am expected to
> throw ...
> 
While I usually tend to just throw Exception (as Bertrand does), this
usually creates nasty code for clients of the api. Imagine you implement
an interface from another api which has a method that declares to throw
exception A and B. Now in the implementation of the method you call the
ms api (ms = microsling :) ) which throws exception. So you have to test
if the ms api call throws exception A or B and pass this through or if
it is any other exception, you have to wrap it.
If the ms api just throws sling exception you catch it, wrap it and
that's it.

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: microsling: Exceptions

[Bertrand Delacretaz <bd...@apache.org>]

On 10/19/07, Felix Meschberger <fm...@gmail.com> wrote:

>...We define
> an API for Sling (or microsling for that matter) applications, so the
> most natural thing counting these two together is to require
> SlingExceptions to be thrown....

Ok, considering other's comments here (thanks!) I have created
SLING-75 to move to SlingException throughout the API.

> ...So rather, I would suggest to define more specific exceptions if
> required (e.g. if we think Resource resolution failure should be better
> visible, define a ResourceResolutionException extends SlingException)....

I usually try to do this so that exception class names carry
meaningful information. But we shouldn't encumber the API with too
many specific exception classes.

I think (most of) the exception classes which extend SlingException
can safely be defined at the implementation level: they won't be
exposed in the API then, but someone seeing a
ResourceResolutionException in their log will have more info than if
it was a plain SlingException, so I think it's worth it.

-Bertrand

Re: microsling: Exceptions

[Felix Meschberger <fm...@gmail.com>]

Am Freitag, den 19.10.2007, 14:53 +0200 schrieb Bertrand Delacretaz:
> > ...I would suggest, that all
> > methods allowed to throw checked exceptions should throw SlingException....
> 
> The downside is having to rethrow exceptions which the implementation
> gets and which are not SlingExceptions.
> 
> What's the upside, in your opinion?
> 
> (I'm not being ironic, it's really a question that no one answered in
> a satisfactory way for my pragmatic brain until now)

I am ok with the question - I asked it to myself, too. But then, here is
why I propose this:

We define an API for the Sling realm which has its own exception
hierarchy rooted at SlingException (extends ServletException). We define
an API for Sling (or microsling for that matter) applications, so the
most natural thing counting these two together is to require
SlingExceptions to be thrown.

So applications may catch Exceptions within the core domain model of the
API. So I think, that trying to unwind exceptions would involve
knowledge of the implementation (e.g. interpreting RepositoryException
(or worse SqlException) ) upfront, would link the application into the
implementation too much.

So rather, I would suggest to define more specific exceptions if
required (e.g. if we think Resource resolution failure should be better
visible, define a ResourceResolutionException extends SlingException).

Another point is, that whenever a method is declared throws Exception I
am force to catch(Exception), which is something a definitely do not
like because I might want to let RuntimeException pass through.
Otherwise I would also be required to declare throws Exception and may
not even be able to describe what exact exception I am expected to
throw ...

Moreover, requiring specific API domain exceptions also requires the
implementor (not the user) to think about the possible problems that may
occurr.

There may be more reasons, which do not come to my mind currently. But I
know (unfortunately I  do not have the book handy right now) that Josh
Bloch wrote about this issue in his seminal work Effective Java [1].

Regards
Felix

[1]
http://www.amazon.com/Effective-Java-Programming-Language-Guide/dp/0321356683

Re: microsling: Exceptions

[Bertrand Delacretaz <bd...@apache.org>]

On 10/19/07, Felix Meschberger <fm...@gmail.com> wrote:

> ...Many methods in microsling are defined to throw Exception....

That's me ;-)

I know many people find this wrong (do you have more info/references
on why?) but I like to do that in widely-used interfaces, as it allows
the implementation to throw any suitable exception, without having to
run through hoops to rethrow the "correct" one.

Then, at the highest level where this makes sense, you catch
Exceptions and their subclasses in a fine-grained way, as needed. But
that's (IMHO) a high-level concern, not something that should happen
all over the place.

> ...I would suggest, that all
> methods allowed to throw checked exceptions should throw SlingException....

The downside is having to rethrow exceptions which the implementation
gets and which are not SlingExceptions.

What's the upside, in your opinion?

(I'm not being ironic, it's really a question that no one answered in
a satisfactory way for my pragmatic brain until now)

-Bertrand