log and throw exception [was: Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java]

[Mario Ivankovits <ma...@ops.co.at>]

Hi!
>          } catch (FileNotFoundException fnfe) {
>              log.error(sm.getString("jsse.keystore_load_failed", type, path,
> -                    fnfe.getMessage()));
> +                    fnfe.getMessage()), fnfe);
>              throw fnfe;
>          } catch (IOException ioe) {
>              log.error(sm.getString("jsse.keystore_load_failed", type, path,
> -                    ioe.getMessage()));
> +                    ioe.getMessage()), ioe);
>              throw ioe;      
>   

I'd like to ask if it is really required to log the exception and throw
it too. Code like this will lead to logfile flooding as normally there
is some exception handling outside of the method which then handle the
exception, rethrow it, or purge it - then with logging of the exception.

I think there is no need to log the exception if you rethrow it. If
everyone along the stack rethrowing an exception also logs it, it will
be hard to read the logs, no?

Probably you can change the message so that it comes to something like:

catch (FileNotFoundException fnfe)
{
    throw new FileNotFoundException(sm.getString(.....), fnfe);
}


Ciao,
Mario


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

Re: log and throw exception [was: Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java]

[Mark Thomas <ma...@apache.org>]

Mario Ivankovits wrote:
> Hi!
>>          } catch (FileNotFoundException fnfe) {
>>              log.error(sm.getString("jsse.keystore_load_failed", type, path,
>> -                    fnfe.getMessage()));
>> +                    fnfe.getMessage()), fnfe);
>>              throw fnfe;
>>          } catch (IOException ioe) {
>>              log.error(sm.getString("jsse.keystore_load_failed", type, path,
>> -                    ioe.getMessage()));
>> +                    ioe.getMessage()), ioe);
>>              throw ioe;      
>>
> 
> I'd like to ask if it is really required to log the exception and throw
> it too.

Strictly, no - providing all the places that call this method log an 
exception correctly.

> Code like this will lead to logfile flooding as normally there
> is some exception handling outside of the method which then handle the
> exception, rethrow it, or purge it - then with logging of the exception.

Not a major issue here - this error only occurs on start up.

> I think there is no need to log the exception if you rethrow it. If
> everyone along the stack rethrowing an exception also logs it, it will
> be hard to read the logs, no?

Not really - just blindingly obvious that there was a problem.

> Probably you can change the message so that it comes to something like:
> 
> catch (FileNotFoundException fnfe)
> {
>     throw new FileNotFoundException(sm.getString(.....), fnfe);
> }

Yes but then you are reliant on the code outside this class logging 
correctly. In the cases I have seen it does but since someone cared enough 
to create a bug report for it I would rather err on this side of too much 
logging in this case. Others may disagree - we'll see how they vote.

Mark

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