Problem for the rule DoNotSwallowErrorsCatchingNonSpecificExceptionsRule

[Luping]

I have created a new personal exception named DALException. In the data tier of my application, I wrap the exception catched “e” into this personal exception and rethrow it to the higher tier, but I got a violation “DoNotSwallowErrorsCatchingNonSpecificExceptionsRule”. I don’t think this is a real violation. Someone can help me?

try
{
//Do somethings…..
}
catch (Exception e)
{
throw new DALException (
String.Format(CultureInfo.CurrentCulture,Resource.ERR_DAL,transaction.Connection.ToString()),
ConnexionStringName,
DataProviders,
ConnexionString,
e);
}

[Andres G. Aragoneses]

On 29/06/12 10:52, Luping wrote:
> I have created a new personal exception named DALException. In the data

> tier of my application, I wrap the exception catched �e� into this

> personal exception and rethrow it to the higher tier, but I got a

> violation �DoNotSwallowErrorsCatchingNonSpecificExceptionsRule�. I don�t

> think this is a real violation. Someone can help me?
>
> try
> {

> //Do somethings�..

> }
> catch (Exception e)
> {
> throw new DALException (
> String.Format(CultureInfo.CurrentCulture,Resource.ERR_DAL,transaction.Connection.ToString()),
> ConnexionStringName,
> DataProviders,
> ConnexionString,
> e);
> }

It *is* a real violation because you're catching Exception instead of a
derived class of Exception.

Normally apps have an isolated place with a catch-all though, but just
one. And this is the only one which you should add to your Gendarme
ignore-list.

[Bevan Arps]

On 29/06/2012 11:56 p.m., Andres G. Aragoneses wrote:

It *is* a real violation because you're catching Exception instead of a derived class of Exception.

Normally apps have an isolated place with a catch-all though, but just one. And this is the only one which you should add to your Gendarme ignore-list.

Catching Exception (and not a derived class) in order to wrap the original exception with additional information to give context for the original error is a pretty common pattern, at least in the code I've encountered.

The key is that you're not catching the exception with an intent to *handle* it, but simply to provide additional context by wrapping it with addition information.

Sometimes the only diagnostic you get is the exception message heirarchy and the extra context is invaluable.

My 2c,
Bevan.

[Luping]

Basically I agree with you.
But, I want to handle all exceptions in the top tier, that's why in the others tiers, I have created a series of personals exceptions like DALException, ServiceException ...... to wrap and rethrow those originals exceptions.

[Andres G. Aragoneses]

You can still do that, but wrapping specific exceptions you expect. If
you answer to that "but I don't know all the exceptions I can expect", I
would reply "that is reasonable, but then any non-expected exception
should not be wrapped, otherwise you add noise and make the problem more
difficult to fix".

On 02/07/12 14:57, Luping wrote:
> Basically I agree with you.
> But, I want to handle all exceptions in the top tier, that's why in the
> others tiers, I have created a series of personals exceptions like
> DALException, ServiceException ...... to wrap and rethrow those
> originals exceptions.
>
>

> Le samedi 30 juin 2012 05:28:37 UTC+2, Bevan a �crit :

>
> On 29/06/2012 11:56 p.m., Andres G. Aragoneses wrote:
>> It *is* a real violation because you're catching Exception instead
>> of a derived class of Exception.
>>
>> Normally apps have an isolated place with a catch-all though, but
>> just one. And this is the only one which you should add to your
>> Gendarme ignore-list.
>
> Catching Exception (and not a derived class) in order to wrap the
> original exception with additional information to give context for
> the original error is a pretty common pattern, at least in the code
> I've encountered.
>
> The key is that you're not catching the exception with an intent to
> *handle* it, but simply to provide additional context by wrapping it
> with addition information.
>
> Sometimes the only diagnostic you get is the exception message
> heirarchy and the extra context is invaluable.
>
> My 2c,
> Bevan.
>

> --
> You received this message because you are subscribed to the Google
> Groups "Gendarme" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/gendarme/-/DFPiEf4WC_wJ.
> To post to this group, send email to
> gend...@googlegroups.com.
> To unsubscribe from this group, send email to
> gendarme+u...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/gendarme?hl=en.

[mark]

Just to add my thoughts to this...

I'd agree with Bevan, while this may not strictly adhere to the ideal, extra information is always useful.

I have a slight quibble with the name of this rule - ' DoNotSwallowErrorsCatchingNonSpecificExceptionsRule ' - I've always thought of 'swollowing' exceptions as catching them and then doing nothing with them. In this case they are wrapped and the wrapped exception thrown - clearly not 'doing nothing with them'.

The  'NonSpecificExceptions' part I agree with - catching and wrapping unexpected exceptions might lead to confusion. I'm not sure where the line is between catching exceptions to keep an app running, and letting unexpected exceptions do what they should - fail fast, fail hard, so you know there's a problem.

Mark