Grouper Developers Forum

[Tom Barton <>]

Ahh, thanks. I now see what you mean about passing exceptions up to the 
caller, and agree that it's desirable.

Can all developers agree that, when modifying a code section for other 
reasons, to be on the lookout for opportunities to discuss and/or 
suitably wrap specific exceptions? I suppose those opportunities could 
occur in source adapters or in grouper API.

Tom

Chris Hyzer wrote:
> > Although it would be good to have some uniformity for expressed log
> > levels across the integrated i2mi components, I don't know that it's
> > necessary to pass exceptions around among them. Perhaps a review of
> > as-packaged log4j.properties files would be in order instead? That
> > should allow a given integration of i2mi components to create
> > appropriate log files. In fact, perhaps a single log4j.properties file
> > that captures an agreed-upon log level for each component should
> > accompany each distribution.
>
> I agree with you, and I think having granular logging in one file would be 
> great.
>
> However, I think we also need to decide which errors we want to fail on, and 
> which we want to log and ignore.  For instance, we don't code like this:
>
> try {
>   ... code ...
>   return result;
> } catch (RuntimeException e) {
>   LOG.error(e);
>   return null;
> }
>
> This is because if there is a null pointer or something like that, the 
> program is broken, and we want the calling program to get an exception.  
> Unfortunately the java JDBC api was written back when idealistically all 
> exceptions should be checked exceptions.  Lately frameworks are realizing 
> that this is not the best way to do things (Spring, Hibernate v3, C#, etc).  
> So, if the SQL is bad, do we want the grouper processing to continue, or do 
> we want it to fail (where we will be more likely to find the problem in 
> development instead of production), similar to a null pointer.  And I don't 
> think this type of error is recoverable, so calling programs don't have to 
> handle it... if there is bad sql, things need to fail.  As for the UI not 
> having proper error handling, we should make sure if an unchecked exception 
> propagates, it goes to a friendly error page for the user (and gets logged to 
> the error level).  Granted a few SQL exceptions are recoverable (e.g. a 
> StaleObjectStateExce
ption from hibernate which says an object was updated by someone else in the 
meantime).
> In summary, I think the method should look like this:
>
>         } catch (SQLException ex) {
>             //well, logging is optional, since the caller should log...
>             log.error("SQLException occurred: " + ex.getMessage(), ex);
>             throw new RuntimeException(ex);
>         }
>
> Here's an article which is a little dated, but still talks to my points:
>
> http://www.onjava.com/pub/a/onjava/2003/11/19/exceptions.html?page=1
>
> - The first example at the top is the exact code we are discussing 
> (swallowing a SQL exception)
> - "Moreover, prefer unchecked exceptions for all programming errors: unchecked 
> exceptions have the benefit of not forcing the client API to explicitly deal with them. 
> They propagate to where you want to catch them, or they go all the way out and get 
> reported."
> - "Most of the time, client code cannot do anything about SQLExceptions. Do not 
> hesitate to convert them into unchecked exceptions"
> - "Do not suppress or ignore exceptions - When a method from an API throws a 
> checked exception, it is trying to tell you that you should take some counter action. 
> If the checked exception does not make sense to you, do not hesitate to convert it into 
> an unchecked exception and throw it again, but do not ignore it by catching it with {} 
> and then continue as if nothing had happened."
>
> Anyways, this might be too sharp of a departure for how things currently work 
> and the direction grouper is going, and I am happy to go in that direction, 
> but just wanted to send my thoughts.  :)
>
> Either way, it would probably be helpful to add our exception strategy to our 
> development standards/guide...
>
> Thanks!
> Chris