You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "Alan D. Cabrera" <li...@toolazydogs.com> on 2010/01/19 15:41:39 UTC
Not sure why...
try {
doSomeStuff();
} catch (RuntimeException e) {
throw e;
} catch (Error e) {
throw e;
} catch (Exception e) {
throw new RuntimeIoException("Failed to create a
pollset.", e);
} finally {
cleanUp();
}
What's the point in catching and re-throwing RuntimeException and Error?
Regards,
Alan
Re: Re : Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
In practice this is true. But in this instance we do nothing.
Regards,
Alan
On Jan 19, 2010, at 9:16 AM, Edouard De Oliveira wrote:
> The point is to react differently in case of different exceptions
> but exceptions must be sorted from specific exceptions to generic ones
> it can also be done by catching exception and using instanceof
> statements
>
> my 2 cents :)
>
> Cordialement, Regards,
> -Edouard De Oliveira-
> Blog: http://tedorgwp.free.fr
> WebSite: http://tedorg.free.fr/en/main.php
>
>
>
> ----- Message d'origine ----
> De : Alan D. Cabrera <li...@toolazydogs.com>
> À : dev@mina.apache.org
> Envoyé le : Mar 19 Janvier 2010, 16 h 12 min 41 s
> Objet : Re: Not sure why...
>
> org.apache.mina.transport.socket.apr.AprIoProcessor
>
>
> Regards,
> Alan
>
> On Jan 19, 2010, at 6:44 AM, Ashish wrote:
>
>> Where did u pulled this out from ?
>>
>> On Tue, Jan 19, 2010 at 8:11 PM, Alan D. Cabrera <list@toolazydogs.com
>> > wrote:
>>> try {
>>> doSomeStuff();
>>> } catch (RuntimeException e) {
>>> throw e;
>>> } catch (Error e) {
>>> throw e;
>>> } catch (Exception e) {
>>> throw new RuntimeIoException("Failed to create a
>>> pollset.", e);
>>> } finally {
>>> cleanUp();
>>> }
>>>
>>> What's the point in catching and re-throwing RuntimeException and
>>> Error?
>>>
>>>
>>> Regards,
>>> Alan
>>>
>>>
>>>
>>
>>
>>
>> --thanks
>> ashish
>>
>> Blog: http://www.ashishpaliwal.com/blog
>> My Photo Galleries: http://www.pbase.com/ashishpaliwal
>
>
>
>
Re : Not sure why...
Posted by Edouard De Oliveira <do...@yahoo.fr>.
The point is to react differently in case of different exceptions
but exceptions must be sorted from specific exceptions to generic ones
it can also be done by catching exception and using instanceof statements
my 2 cents :)
Cordialement, Regards,
-Edouard De Oliveira-
Blog: http://tedorgwp.free.fr
WebSite: http://tedorg.free.fr/en/main.php
----- Message d'origine ----
De : Alan D. Cabrera <li...@toolazydogs.com>
À : dev@mina.apache.org
Envoyé le : Mar 19 Janvier 2010, 16 h 12 min 41 s
Objet : Re: Not sure why...
org.apache.mina.transport.socket.apr.AprIoProcessor
Regards,
Alan
On Jan 19, 2010, at 6:44 AM, Ashish wrote:
> Where did u pulled this out from ?
>
> On Tue, Jan 19, 2010 at 8:11 PM, Alan D. Cabrera <li...@toolazydogs.com> wrote:
>> try {
>> doSomeStuff();
>> } catch (RuntimeException e) {
>> throw e;
>> } catch (Error e) {
>> throw e;
>> } catch (Exception e) {
>> throw new RuntimeIoException("Failed to create a pollset.", e);
>> } finally {
>> cleanUp();
>> }
>>
>> What's the point in catching and re-throwing RuntimeException and Error?
>>
>>
>> Regards,
>> Alan
>>
>>
>>
>
>
>
> --thanks
> ashish
>
> Blog: http://www.ashishpaliwal.com/blog
> My Photo Galleries: http://www.pbase.com/ashishpaliwal
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 19, 2010, at 8:58 PM, Ashish wrote:
>>> We can simply catch it and do whatever we want with it.
>>
>> In this case, I think the constructor should be throwing an
>> IOException.
>> Anyone mind if I remove the places where we removed the checked
>> exceptions?
>>
>
> I am afraid, we have already freezed the API, so not sure if we
> should do this.
Good point. 3.0 it is.
Regards,
Alan
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 20, 2010, at 1:41 AM, Emmanuel Lcharny wrote:
> Ashish a écrit :
>>>> We can simply catch it and do whatever we want with it.
>>>>
>>> In this case, I think the constructor should be throwing an
>>> IOException.
>>> Anyone mind if I remove the places where we removed the checked
>>> exceptions?
>>>
>>>
>>
>> I am afraid, we have already freezed the API, so not sure if we
>> should do this.
>>
> But you can remove the crap inside the method, and throw one single
> runtime exception with a decent message. The bst would be to throw a
> RuntimeIoException, as this constructor does not throws anything.
Yes, since we don't want to change the API we should still throw a
RuntimeException.
> Btw, I'd like to know your opinion about constructor throwing a
> checked exception : do you consider this as a bad smell?
I think constructors throwing a checked exception is fine.
Regards,
Alan
Re: Not sure why...
Posted by Emmanuel Lcharny <el...@gmail.com>.
Ashish a écrit :
>>> We can simply catch it and do whatever we want with it.
>>>
>> In this case, I think the constructor should be throwing an IOException.
>> Anyone mind if I remove the places where we removed the checked exceptions?
>>
>>
>
> I am afraid, we have already freezed the API, so not sure if we should do this.
>
But you can remove the crap inside the method, and throw one single
runtime exception with a decent message. The bst would be to throw a
RuntimeIoException, as this constructor does not throws anything.
Btw, I'd like to know your opinion about constructor throwing a checked
exception : do you consider this as a bad smell?
Re: Not sure why...
Posted by Ashish <pa...@gmail.com>.
>> We can simply catch it and do whatever we want with it.
>
> In this case, I think the constructor should be throwing an IOException.
> Anyone mind if I remove the places where we removed the checked exceptions?
>
I am afraid, we have already freezed the API, so not sure if we should do this.
> Regards,
> Alan
>
>
>
--
thanks
ashish
Blog: http://www.ashishpaliwal.com/blog
My Photo Galleries: http://www.pbase.com/ashishpaliwal
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 19, 2010, at 3:15 PM, Emmanuel Lcharny wrote:
> Alan D. Cabrera a écrit :
>>
>> On Jan 19, 2010, at 7:31 AM, Ashish wrote:
>>
>>> Well I think only Exception could be the only case needed. However,
>>> catching generic Exception means any error, a specific Exception
>>> caught is better.
>>
>> The other odd thing is that it catches the checked exception and
>> wraps it in a runtime exception. This strikes me as a poor practice.
> To say the least ...
>
> Added on http://svn.apache.org/viewvc/mina/trunk/transport-apr/src/main/java/org/apache/mina/transport/socket/apr/AprIoProcessor.java?r1=593014&r2=596587
>
> Why am I not surprised ? ...
>
> Here, tomcat-APR just throw a java.lang.Exception instance, not a
> Runtime or an Error :
>
> http://svn.apache.org/viewvc/tomcat/native/tags/TOMCAT_NATIVE_1_1_19/native/src/error.c?revision=896054&view=markup
>
> /*
> 39 * Convenience function to help throw an java.lang.Exception.
> 40 */
> 41 void tcn_ThrowException(JNIEnv *env, const char *msg)
> 42 {
> 43 jclass javaExceptionClass;
> 44 45 javaExceptionClass = (*env)->FindClass(env, "java/
> lang/Exception");
> 46 if (javaExceptionClass == NULL) {
> 47 fprintf(stderr, "Cannot find java/lang/Exception class
> \n");
> 48 return;
> 49 }
> 50 (*env)->ThrowNew(env, javaExceptionClass, msg);
> 51 (*env)->DeleteLocalRef(env, javaExceptionClass);
> 52 53 }
>
> We can simply catch it and do whatever we want with it.
In this case, I think the constructor should be throwing an
IOException. Anyone mind if I remove the places where we removed the
checked exceptions?
Regards,
Alan
Re: Not sure why...
Posted by Emmanuel Lcharny <el...@gmail.com>.
Alan D. Cabrera a écrit :
>
> On Jan 19, 2010, at 7:31 AM, Ashish wrote:
>
>> Well I think only Exception could be the only case needed. However,
>> catching generic Exception means any error, a specific Exception
>> caught is better.
>
> The other odd thing is that it catches the checked exception and wraps
> it in a runtime exception. This strikes me as a poor practice.
To say the least ...
Added on
http://svn.apache.org/viewvc/mina/trunk/transport-apr/src/main/java/org/apache/mina/transport/socket/apr/AprIoProcessor.java?r1=593014&r2=596587
Why am I not surprised ? ...
Here, tomcat-APR just throw a java.lang.Exception instance, not a
Runtime or an Error :
http://svn.apache.org/viewvc/tomcat/native/tags/TOMCAT_NATIVE_1_1_19/native/src/error.c?revision=896054&view=markup
/*
39 * Convenience function to help throw an java.lang.Exception.
40 */
41 void tcn_ThrowException(JNIEnv *env, const char *msg)
42 {
43 jclass javaExceptionClass;
44
45 javaExceptionClass = (*env)->FindClass(env,
"java/lang/Exception");
46 if (javaExceptionClass == NULL) {
47 fprintf(stderr, "Cannot find java/lang/Exception class\n");
48 return;
49 }
50 (*env)->ThrowNew(env, javaExceptionClass, msg);
51 (*env)->DeleteLocalRef(env, javaExceptionClass);
52
53 }
We can simply catch it and do whatever we want with it.
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 19, 2010, at 6:44 PM, Ashish wrote:
>>
>>> Well I think only Exception could be the only case needed. However,
>>> catching generic Exception means any error, a specific Exception
>>> caught is better.
>>
>> The other odd thing is that it catches the checked exception and
>> wraps it in
>> a runtime exception. This strikes me as a poor practice.
>>
>>> BTW, the catch(Error e) {} stuff has been caught by IntelliJ
>>> Inspections.
>>>
>>> Just one more thing that I noted in the class, there are no log
>>> statements. Dom't we need them?
>>
>> I'm against logging exceptions if we're re-throwing them unless we
>> print out
>> some critical piece of information that cannot be discerned by any
>> other
>> method. I say this from experience where I had to slog through
>> millions of
>> log statements generated by one exception where every well meaning
>> layer
>> printed something out.
>
> Alan, my comment was in general, not specific to Exception clause :-)
> The class doesn't have logging at all :-(
Duh. I misread the post. Sorry. :(
You and I are of the same mind on this. :)
Regards,
Alan
Re: Not sure why...
Posted by Ashish <pa...@gmail.com>.
>
>> Well I think only Exception could be the only case needed. However,
>> catching generic Exception means any error, a specific Exception
>> caught is better.
>
> The other odd thing is that it catches the checked exception and wraps it in
> a runtime exception. This strikes me as a poor practice.
>
>> BTW, the catch(Error e) {} stuff has been caught by IntelliJ Inspections.
>>
>> Just one more thing that I noted in the class, there are no log
>> statements. Dom't we need them?
>
> I'm against logging exceptions if we're re-throwing them unless we print out
> some critical piece of information that cannot be discerned by any other
> method. I say this from experience where I had to slog through millions of
> log statements generated by one exception where every well meaning layer
> printed something out.
Alan, my comment was in general, not specific to Exception clause :-)
The class doesn't have logging at all :-(
>
> Regards,
> Alan
>
>
--
thanks
ashish
Blog: http://www.ashishpaliwal.com/blog
My Photo Galleries: http://www.pbase.com/ashishpaliwal
Re: Not sure why...
Posted by Emmanuel Lcharny <el...@gmail.com>.
Alan D. Cabrera a écrit :
>> IMO, we should log :
>> - where we got the exception if we know that no log has already been
>> generated
>
> I'm against this since you usually have no idea if someone else has
> logged the error.
When you are writing a framework, you are the one responsible for
logging, nobody else will do that for you. This is why I think it's
important to log as close as possible to the place the error was produced.
>> - In some case, it might be interesting to use a specific logger. For
>> instance, if you grab a new connection to a LDAP serve,r and get an
>> exception, using a dedicated logger can help when analysing
>> specifically all the LDAP related issues.
>
> Interesting idea but my experience has been that you need a "larger"
> context to make sense of things. I think that if we did have a
> dedicated logger then we would log to both w/ the chattier logging
> going to the dedicated logger.
Not necessarily. I have written such system where dedicated logs were
produced (mainly JDBC or LDAP or whatever technical system) but not
duplicated in the mail logs, or with less detailed errors.
Anyway, most of the case you'll will be able to know when you added too
many logs by running the code you have added some logs to.
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 19, 2010, at 10:45 AM, Emmanuel Lcharny wrote:
> Alan D. Cabrera a écrit :
>>
>>>
>>> Just one more thing that I noted in the class, there are no log
>>> statements. Dom't we need them?
>>
>> I'm against logging exceptions if we're re-throwing them unless we
>> print out some critical piece of information that cannot be
>> discerned by any other method. I say this from experience where I
>> had to slog through millions of log statements generated by one
>> exception where every well meaning layer printed something out.
> IMO, we should log :
> - where we got the exception if we know that no log has already been
> generated
I'm against this since you usually have no idea if someone else has
logged the error.
> - if and only if the log adds some value to what has already be loaded
Agreed. I think that "unless we print out some critical piece of
information that cannot be discerned by any other method." catches
this case.
> - In some case, it might be interesting to use a specific logger.
> For instance, if you grab a new connection to a LDAP serve,r and get
> an exception, using a dedicated logger can help when analysing
> specifically all the LDAP related issues.
Interesting idea but my experience has been that you need a "larger"
context to make sense of things. I think that if we did have a
dedicated logger then we would log to both w/ the chattier logging
going to the dedicated logger.
Regards,
Alan
Re: Not sure why...
Posted by Emmanuel Lcharny <el...@gmail.com>.
Alan D. Cabrera a écrit :
>
>>
>> Just one more thing that I noted in the class, there are no log
>> statements. Dom't we need them?
>
> I'm against logging exceptions if we're re-throwing them unless we
> print out some critical piece of information that cannot be discerned
> by any other method. I say this from experience where I had to slog
> through millions of log statements generated by one exception where
> every well meaning layer printed something out.
IMO, we should log :
- where we got the exception if we now that no log has already been
generated
- if and only if the log adds some value to what has already be loaded
- In some case, it might be interesting to use a specific logger. For
instance, if you grab a new connection to a LDAP serve,r and get an
exception, using a dedicated logger can help when analysing specifically
all the LDAP related issues.
Logging too much is deadly... Not logging enough is deadly too. Use
common sense, test you code, and check if you are just logging enough
and not too much.
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
On Jan 19, 2010, at 7:31 AM, Ashish wrote:
> Well I think only Exception could be the only case needed. However,
> catching generic Exception means any error, a specific Exception
> caught is better.
The other odd thing is that it catches the checked exception and wraps
it in a runtime exception. This strikes me as a poor practice.
> BTW, the catch(Error e) {} stuff has been caught by IntelliJ
> Inspections.
>
> Just one more thing that I noted in the class, there are no log
> statements. Dom't we need them?
I'm against logging exceptions if we're re-throwing them unless we
print out some critical piece of information that cannot be discerned
by any other method. I say this from experience where I had to slog
through millions of log statements generated by one exception where
every well meaning layer printed something out.
Regards,
Alan
Re: Not sure why...
Posted by Ashish <pa...@gmail.com>.
Well I think only Exception could be the only case needed. However,
catching generic Exception means any error, a specific Exception
caught is better,
BTW, the catch(Error e) {} stuff has been caught by IntelliJ Inspections.
Just one more thing that I noted in the class, there are no log
statements. Dom't we need them?
On Tue, Jan 19, 2010 at 8:42 PM, Alan D. Cabrera <li...@toolazydogs.com> wrote:
> org.apache.mina.transport.socket.apr.AprIoProcessor
>
>
> Regards,
> Alan
>
> On Jan 19, 2010, at 6:44 AM, Ashish wrote:
>
>> Where did u pulled this out from ?
>>
>> On Tue, Jan 19, 2010 at 8:11 PM, Alan D. Cabrera <li...@toolazydogs.com>
>> wrote:
>>>
>>> try {
>>> doSomeStuff();
>>> } catch (RuntimeException e) {
>>> throw e;
>>> } catch (Error e) {
>>> throw e;
>>> } catch (Exception e) {
>>> throw new RuntimeIoException("Failed to create a pollset.", e);
>>> } finally {
>>> cleanUp();
>>> }
>>>
>>> What's the point in catching and re-throwing RuntimeException and Error?
>>>
>>>
>>> Regards,
>>> Alan
>>>
>>>
>>>
>>
>>
>>
>> --
>> thanks
>> ashish
>>
>> Blog: http://www.ashishpaliwal.com/blog
>> My Photo Galleries: http://www.pbase.com/ashishpaliwal
>
>
--
thanks
ashish
Blog: http://www.ashishpaliwal.com/blog
My Photo Galleries: http://www.pbase.com/ashishpaliwal
Re: Not sure why...
Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
org.apache.mina.transport.socket.apr.AprIoProcessor
Regards,
Alan
On Jan 19, 2010, at 6:44 AM, Ashish wrote:
> Where did u pulled this out from ?
>
> On Tue, Jan 19, 2010 at 8:11 PM, Alan D. Cabrera
> <li...@toolazydogs.com> wrote:
>> try {
>> doSomeStuff();
>> } catch (RuntimeException e) {
>> throw e;
>> } catch (Error e) {
>> throw e;
>> } catch (Exception e) {
>> throw new RuntimeIoException("Failed to create a
>> pollset.", e);
>> } finally {
>> cleanUp();
>> }
>>
>> What's the point in catching and re-throwing RuntimeException and
>> Error?
>>
>>
>> Regards,
>> Alan
>>
>>
>>
>
>
>
> --
> thanks
> ashish
>
> Blog: http://www.ashishpaliwal.com/blog
> My Photo Galleries: http://www.pbase.com/ashishpaliwal
Re: Not sure why...
Posted by Emmanuel Lecharny <el...@gmail.com>.
Ashish a écrit :
> Where did u pulled this out from ?
>
NO ! Alan, please, no need to tell ;)
Seriously, it stinks ;)
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: Not sure why...
Posted by Ashish <pa...@gmail.com>.
Where did u pulled this out from ?
On Tue, Jan 19, 2010 at 8:11 PM, Alan D. Cabrera <li...@toolazydogs.com> wrote:
> try {
> doSomeStuff();
> } catch (RuntimeException e) {
> throw e;
> } catch (Error e) {
> throw e;
> } catch (Exception e) {
> throw new RuntimeIoException("Failed to create a pollset.", e);
> } finally {
> cleanUp();
> }
>
> What's the point in catching and re-throwing RuntimeException and Error?
>
>
> Regards,
> Alan
>
>
>
--
thanks
ashish
Blog: http://www.ashishpaliwal.com/blog
My Photo Galleries: http://www.pbase.com/ashishpaliwal