Grouper Developers Forum

[Chris Hyzer <>]

> 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 
StaleObjectStateException 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