Consider the following c# code:
public class ExceptionManager
{
public static void TreatException(Exception ex)
{
if (ShowAndContinue(ex))
// display a user-friendly message on what happened and let the app run.
else
throw ex;
}
// (more code)
}
(Please note that the above is over-simplified so that the focus is on my actual concern)
IMHO I think the way the exception is thrown is wrong because stacktrace information is lost. The preferred way (again, IMHO) to write the throw line would be this:
throw new UntreatableErrorException("Manager could not treat the exception.", ex);
This way the original exception’s stacktrace is preserved.
However, a colleague of mine is arguing that doing so would mean losing the ability to catch the original exception lower in the call stack, because catch works with exception type and hence an exception trap might be set to catch the original exception but not the UntreatableErrorException. Therefore it’s better to throw back the original exception instance, even if it means losing the stacktrace.
Which one of us is right here on what should be the best practice?
You are both right.
You don’t want to lose the stack trace, but you may want to catch the lower level exception.
Don’t wrap the exception if it isn’t adding information.
I would call your ExceptionManager and TreatException a bit of a code smell – they are trying too hard to centralize exception handling.
To re throw without losing the stack trace:
public static bool ContinueAfterException(Exception ex)
{
if (ShowAndContinue(ex))
{
// display a user-friendly message on what happened and let the app run.
return true;
}
return false;
}
Elsewhere:
try
{
// some exception throwing code
}
catch(Exception ex)
{
if(!ContinueAfterException(ex))
throw; // no ex
}
11
What I see here, that you want to catch a subset of possible errors and you rethrow what you did not want to catch. The right solution would be not catching what you don’t want to catch, so you won’t need to rethrow it.
I am not sure what C# allows, because I never worked with that language, but my guess would be that wrapping your treatable exceptions with TreatableException or adding a Treatable interface to them and changing your code to TreatException(Treatable ex) would work in most of the cases.
1