RE: Useful error messages

["Noel J. Bergman" <no...@devtech.com>]

> So I did some digging .....
> .. and in the James source for RepositoryManager I noticed the following:

We inherited that code from Cornerstone when we were told that in order to
fix bugs in the Cornerstone code, we needed to take that into our module so
that PeterD could remove it from Avalon.  Unfortunately, I don't think that
anyone on our side has really looked at it much.

  (a) we would be happy to give it back to you.
  (b) we would be happy for some suggestion(s).

> catch( final Exception e )
> {
> final String message = "Cannot find or init repository: " +
e.getMessage();
> getLogger().warn( message, e );
> throw new ServiceException("", message, e );
> }

Probably someone wanted to get the exception into that component's log, and
also to notify the caller.

> in AvalonMailStore (which is actually the "JamesMailStore") we have
> the following fragment

> catch (Exception e) {
>   if (getLogger().isWarnEnabled()) {
>     getLogger().warn( "Exception while creating repository:" +
e.getMessage(), e );
>   }
>   e.printStackTrace();
>   throw new ServiceException("","Cannot find or init repository", e);
> }

> As a rule I NEVER log an exception AND throw it.  The only time
> you need to log an exception is when you don't intent to throw it.

The printStackTrace ought to be canned, along with (almost) all uses of
RuntimeException.  We recently cleaned up a bunch of the latter.

The logging pattern is still an issue.  Logging to the component's log
and/or throwing the exception.  The former is reporting, the latter is
functional.  The latter *must* be done, so we are back to the issue of
wanting the exception to be logged in the right log.

> Finally, onto some Avalon concerns.

> (a) configuration exceptions would be more useful if they contained
>     the offending fragment
> (b) better specs are needed about component/container seperation
>     of responsibilities (e.g. who handled error reporting)

There is still the issue of locality, although this would not be apparent if
you only had one log file.

	--- Noel


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@avalon.apache.org
For additional commands, e-mail: dev-help@avalon.apache.org

Re: Useful error messages

[Stephen McConnell <mc...@apache.org>]

Noel J. Bergman wrote:

>>So I did some digging .....
>>.. and in the James source for RepositoryManager I noticed the following:
>>    
>>
>
>We inherited that code from Cornerstone when we were told that in order to
>fix bugs in the Cornerstone code, we needed to take that into our module so
>that PeterD could remove it from Avalon.  Unfortunately, I don't think that
>anyone on our side has really looked at it much.
>
>  (a) we would be happy to give it back to you.
>  (b) we would be happy for some suggestion(s).
>  
>

I have not idea what giving this back would imply - what I am more than 
happy to do is to work though the james code and cleanup sources of 
multiple logging directives.

>  
>
>>catch( final Exception e )
>>{
>>final String message = "Cannot find or init repository: " +
>>e.getMessage();
>>    
>>
>>getLogger().warn( message, e );
>>throw new ServiceException("", message, e );
>>}
>>    
>>
>
>Probably someone wanted to get the exception into that component's log, and
>also to notify the caller.
>  
>

That's my guess as well.

>  
>
>>in AvalonMailStore (which is actually the "JamesMailStore") we have
>>the following fragment
>>    
>>
>
>  
>
>>catch (Exception e) {
>>  if (getLogger().isWarnEnabled()) {
>>    getLogger().warn( "Exception while creating repository:" +
>>    
>>
>e.getMessage(), e );
>  
>
>>  }
>>  e.printStackTrace();
>>  throw new ServiceException("","Cannot find or init repository", e);
>>}
>>    
>>
>
>  
>
>>As a rule I NEVER log an exception AND throw it.  The only time
>>you need to log an exception is when you don't intent to throw it.
>>    
>>
>
>The printStackTrace ought to be canned, along with (almost) all uses of
>RuntimeException.  We recently cleaned up a bunch of the latter.
>

When I spot this sort of thing I could go ahead and make the change - 
but to be honest I've been hesitating because I'm not totally into the 
James code base and there is always this little angel on my sholder 
saying "maybe your missing something". Well, I think I'll tell the angel 
to take a little holiday and I'll start committing some updates.

>The logging pattern is still an issue.  Logging to the component's log
>and/or throwing the exception.  The former is reporting, the latter is
>functional.  The latter *must* be done, so we are back to the issue of
>wanting the exception to be logged in the right log.
>  
>

This is something that I don't have a clear answer on (which bugs me).  
We have a situation where a decision in the code is taken relative to an 
assumption that the devoper is making about the actions of the 
assembler.  This is one of these situations where there is a qualifiable 
conflict of interest and the problem is that you cannot mediate because 
the parties are never there at the same time.  I'm getting a headache 
already.

>  
>
>>Finally, onto some Avalon concerns.
>>    
>>
>
>  
>
>>(a) configuration exceptions would be more useful if they contained
>>    the offending fragment
>>(b) better specs are needed about component/container seperation
>>    of responsibilities (e.g. who handled error reporting)
>>    
>>
>
>There is still the issue of locality, although this would not be apparent if
>you only had one log file.
>  
>

This is not (or at least should not be) a container concern - it is (or 
should be) an assembler concern (i.e. an assembler should control the 
log life configuration which in this case includes the assignment of 
logging channels to logging targets).  I.e. the container has (or should 
have) only one logging target - the logging system manager.  A log 
system manager (under the control of the a systems manager or assembler) 
would be responsible for any breakout of different targets. 

General comment - having implemented a container, one of the real PITA^3 
subjects is handling the bootstrap logging.

Cheers, Steve.


>	--- Noel
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>For additional commands, e-mail: server-dev-help@james.apache.org
>
>
>  
>

-- 

Stephen J. McConnell
mailto:mcconnell@apache.org

Sent via James running under Merlin as an NT service.
http://avalon.apache.org/sandbox/merlin




---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org