You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/05/14 09:17:47 UTC
svn commit: r656124 -
/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Author: markt
Date: Wed May 14 00:17:46 2008
New Revision: 656124
URL: http://svn.apache.org/viewvc?rev=656124&view=rev
Log:
Improve logging messages associated with previous commit in response to Filip's veto
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java?rev=656124&r1=656123&r2=656124&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java Wed May 14 00:17:46 2008
@@ -322,17 +322,17 @@
ks.load(istream, pass.toCharArray());
} 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;
} catch(Exception ex) {
String msg = sm.getString("jsse.keystore_load_failed", type, path,
ex.getMessage());
- log.error(msg);
- throw new IOException(msg);
+ log.error(msg, ex);
+ throw new IOException(msg, ex);
} finally {
if (istream != null) {
try {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Posted by Mark Thomas <ma...@apache.org>.
Filip Hanik - Dev Lists wrote:
> Mark Thomas wrote:
>> Peter Rossbach wrote:
>>> Hi Mark
>>>
>>> Java 5 don't support new IOExceptin(String, Throwable)
>>>
>>> compile:
>>> [javac] Compiling 49 source files to
>>> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/output/classes
>>> [javac]
>>> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java:335:
>>> cannot find symbol
>>> [javac] symbol : constructor
>>> IOException(java.lang.String,java.lang.Exception)
>>> [javac] location: class java.io.IOException
>>> [javac] throw new IOException(msg, ex);
>>> [javac] ^
>>> [javac] Note: Some input files use or override a deprecated API.
>>>
>>> ==
>>> -1 for this fix.
>>
>> I just knew this patch was going to go this way ;)
>>
>> I'll re-do it. Again :)
> for the status file, can you summarize into one diff? makes it easier to
> review
I was going to back the two out and re-do it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Mark Thomas wrote:
> Peter Rossbach wrote:
>> Hi Mark
>>
>> Java 5 don't support new IOExceptin(String, Throwable)
>>
>> compile:
>> [javac] Compiling 49 source files to
>> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/output/classes
>> [javac]
>> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java:335:
>> cannot find symbol
>> [javac] symbol : constructor
>> IOException(java.lang.String,java.lang.Exception)
>> [javac] location: class java.io.IOException
>> [javac] throw new IOException(msg, ex);
>> [javac] ^
>> [javac] Note: Some input files use or override a deprecated API.
>>
>> ==
>> -1 for this fix.
>
> I just knew this patch was going to go this way ;)
>
> I'll re-do it. Again :)
for the status file, can you summarize into one diff? makes it easier to
review
Filip
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Posted by Mark Thomas <ma...@apache.org>.
Peter Rossbach wrote:
> Hi Mark
>
> Java 5 don't support new IOExceptin(String, Throwable)
>
> compile:
> [javac] Compiling 49 source files to
> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/output/classes
> [javac]
> /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java:335:
> cannot find symbol
> [javac] symbol : constructor
> IOException(java.lang.String,java.lang.Exception)
> [javac] location: class java.io.IOException
> [javac] throw new IOException(msg, ex);
> [javac] ^
> [javac] Note: Some input files use or override a deprecated API.
>
> ==
> -1 for this fix.
I just knew this patch was going to go this way ;)
I'll re-do it. Again :)
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Posted by Peter Rossbach <pr...@objektpark.de>.
Hi Mark
Java 5 don't support new IOExceptin(String, Throwable)
compile:
[javac] Compiling 49 source files to /Users/peter/develop/
projects/tomcat/tomcat6currenttrunk/output/classes
[javac] /Users/peter/develop/projects/tomcat/tomcat6currenttrunk/
java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java:335:
cannot find symbol
[javac] symbol : constructor IOException
(java.lang.String,java.lang.Exception)
[javac] location: class java.io.IOException
[javac] throw new IOException(msg, ex);
[javac] ^
[javac] Note: Some input files use or override a deprecated API.
==
-1 for this fix.
Regards
Peter
Am 14.05.2008 um 09:17 schrieb markt@apache.org:
> Author: markt
> Date: Wed May 14 00:17:46 2008
> New Revision: 656124
>
> URL: http://svn.apache.org/viewvc?rev=656124&view=rev
> Log:
> Improve logging messages associated with previous commit in
> response to Filip's veto
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/
> JSSESocketFactory.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/
> JSSESocketFactory.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> tomcat/util/net/jsse/JSSESocketFactory.java?
> rev=656124&r1=656123&r2=656124&view=diff
> ======================================================================
> ========
> --- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/
> JSSESocketFactory.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/
> JSSESocketFactory.java Wed May 14 00:17:46 2008
> @@ -322,17 +322,17 @@
> ks.load(istream, pass.toCharArray());
> } 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;
> } catch(Exception ex) {
> String msg = sm.getString("jsse.keystore_load_failed",
> type, path,
> ex.getMessage());
> - log.error(msg);
> - throw new IOException(msg);
> + log.error(msg, ex);
> + throw new IOException(msg, ex);
> } finally {
> if (istream != null) {
> try {
>
>
>
> ---------------------------------------------------------------------
> 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]
Posted by 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
log and throw exception [was: Re: svn commit: r656124 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java]
Posted by 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