I have a question for you that stems from my partner doing things a different way than I do.
Is it better to do this :
try
{
...
}
catch (Exception ex)
{
...
throw;
}
or this:
try
{
...
}
catch (Exception ex)
{
...
throw ex;
}
Do they do the same thing? Is one better than the other?
This question is related to
c#
.net
exception-handling
It depends. In a debug build, I want to see the original stack trace with as little effort as possible. In that case, "throw;" fits the bill. In a release build, however, (a) I want to log the error with the original stack trace included, and once that's done, (b) refashion the error handling to make more sense to the user. Here "Throw Exception" makes sense. It's true that rethrowing the error discards the original stack trace, but a non-developer gets nothing out of seeing stack trace information so it's okay to rethrow the error.
void TrySuspectMethod()
{
try
{
SuspectMethod();
}
#if DEBUG
catch
{
//Don't log error, let developer see
//original stack trace easily
throw;
#else
catch (Exception ex)
{
//Log error for developers and then
//throw a error with a user-oriented message
throw new Exception(String.Format
("Dear user, sorry but: {0}", ex.Message));
#endif
}
}
The way the question is worded, pitting "Throw:" vs. "Throw ex;" makes it a bit of a red-herring. The real choice is between "Throw;" and "Throw Exception," where "Throw ex;" is an unlikely special case of "Throw Exception."
If you throw an exception without a variable (the second example) the StackTrace will include the original method that threw the exception.
In the first example the StackTrace will be changed to reflect the current method.
Example:
static string ReadAFile(string fileName) {
string result = string.Empty;
try {
result = File.ReadAllLines(fileName);
} catch(Exception ex) {
throw ex; // This will show ReadAFile in the StackTrace
throw; // This will show ReadAllLines in the StackTrace
}
The first preserves the original stack trace of the exception, the second one replaces it with the current location.
Therefore the first is BY FAR the better.
The first is better. If you try to debug the second and look at the call stack you won't see where the original exception came from. There are tricks to keep the call-stack intact (try search, it's been answered before) if you really need to rethrow.
I found that if the exception is thrown in the same method that it is caught, the stack trace is not preserved, for what it's worth.
void testExceptionHandling()
{
try
{
throw new ArithmeticException("illegal expression");
}
catch (Exception ex)
{
throw;
}
finally
{
System.Diagnostics.Debug.WriteLine("finally called.");
}
}
It depends. In a debug build, I want to see the original stack trace with as little effort as possible. In that case, "throw;" fits the bill. In a release build, however, (a) I want to log the error with the original stack trace included, and once that's done, (b) refashion the error handling to make more sense to the user. Here "Throw Exception" makes sense. It's true that rethrowing the error discards the original stack trace, but a non-developer gets nothing out of seeing stack trace information so it's okay to rethrow the error.
void TrySuspectMethod()
{
try
{
SuspectMethod();
}
#if DEBUG
catch
{
//Don't log error, let developer see
//original stack trace easily
throw;
#else
catch (Exception ex)
{
//Log error for developers and then
//throw a error with a user-oriented message
throw new Exception(String.Format
("Dear user, sorry but: {0}", ex.Message));
#endif
}
}
The way the question is worded, pitting "Throw:" vs. "Throw ex;" makes it a bit of a red-herring. The real choice is between "Throw;" and "Throw Exception," where "Throw ex;" is an unlikely special case of "Throw Exception."
The first preserves the original stack trace of the exception, the second one replaces it with the current location.
Therefore the first is BY FAR the better.
I know this is an old question, but I'm going to answer it because I have to disagree with all the answers here.
Now, I'll agree that most of the time you either want to do a plain throw, to preserve as much information as possible about what went wrong, or you want to throw a new exception that may contain that as an inner-exception, or not, depending on how likely you are to want to know about the inner events that caused it.
There is an exception though. There are several cases where a method will call into another method and a condition that causes an exception in the inner call should be considered the same exception on the outer call.
One example is a specialised collection implemented by using another collection. Let's say it's a DistinctList<T> that wraps a List<T> but refuses duplicate items.
If someone called ICollection<T>.CopyTo on your collection class, it might just be a straight call to CopyTo on the inner collection (if say, all the custom logic only applied to adding to the collection, or setting it up). Now, the conditions in which that call would throw are exactly the same conditions in which your collection should throw to match the documentation of ICollection<T>.CopyTo.
Now, you could just not catch the execption at all, and let it pass through. Here though the user gets an exception from List<T> when they were calling something on a DistinctList<T>. Not the end of the world, but you may want to hide those implementation details.
Or you could do your own checking:
public CopyTo(T[] array, int arrayIndex)
{
if(array == null)
throw new ArgumentNullException("array");
if(arrayIndex < 0)
throw new ArgumentOutOfRangeException("arrayIndex", "Array Index must be zero or greater.");
if(Count > array.Length + arrayIndex)
throw new ArgumentException("Not enough room in array to copy elements starting at index given.");
_innerList.CopyTo(array, arrayIndex);
}
That's not the worse code because it's boilerplate and we can probably just copy it from some other implementation of CopyTo where it wasn't a simple pass-through and we had to implement it ourselves. Still, it's needlessly repeating the exact same checks that are going to be done in _innerList.CopyTo(array, arrayIndex), so the only thing it has added to our code is 6 lines where there could be a bug.
We could check and wrap:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentNullException ane)
{
throw new ArgumentNullException("array", ane);
}
catch(ArgumentOutOfRangeException aore)
{
throw new ArgumentOutOfRangeException("Array Index must be zero or greater.", aore);
}
catch(ArgumentException ae)
{
throw new ArgumentException("Not enough room in array to copy elements starting at index given.", ae);
}
}
In terms of new code added that could potentially be buggy, this is even worse. And we don't gain a thing from the inner exceptions. If we pass a null array to this method and receive an ArgumentNullException, we're not going to learn anything by examining the inner exception and learning that a call to _innerList.CopyTo was passed a null array and threw an ArgumentNullException.
Here, we can do everything we want with:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentException ae)
{
throw ae;
}
}
Every one of the exceptions that we expect to have to throw if the user calls it with incorrect arguments, will correctly be thrown by that re-throw. If there's a bug in the logic used here, it's in one of two lines - either we were wrong in deciding this was a case where this approach works, or we were wrong in having ArgumentException as the exception type looked for. It's the only two bugs that the catch block can possibly have.
Now. I still agree that most of the time you either want a plain throw; or you want to construct your own exception to more directly match the problem from the perspective of the method in question. There are cases like the above where re-throwing like this makes more sense, and there are plenty of other cases. E.g. to take a very different example, if an ATOM file reader implemented with a FileStream and an XmlTextReader receives a file error or invalid XML, then it will perhaps want to throw exactly the same exception it received from those classes, but it should look to the caller that it is AtomFileReader that is throwing a FileNotFoundException or XmlException, so they might be candidates for similarly re-throwing.
Edit:
We can also combine the two:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentException ae)
{
throw ae;
}
catch(Exception ex)
{
//we weren't expecting this, there must be a bug in our code that put
//us into an invalid state, and subsequently let this exception happen.
LogException(ex);
throw;
}
}
My preferences is to use
try
{
}
catch (Exception ex)
{
...
throw new Exception ("Put more context here", ex)
}
This preserves the original error, but allows you to put more context, such as an object ID, a connection string, stuff like that. Often my exception reporting tool will have 5 chained exceptions to report, each reporting more detail.
It depends. In a debug build, I want to see the original stack trace with as little effort as possible. In that case, "throw;" fits the bill. In a release build, however, (a) I want to log the error with the original stack trace included, and once that's done, (b) refashion the error handling to make more sense to the user. Here "Throw Exception" makes sense. It's true that rethrowing the error discards the original stack trace, but a non-developer gets nothing out of seeing stack trace information so it's okay to rethrow the error.
void TrySuspectMethod()
{
try
{
SuspectMethod();
}
#if DEBUG
catch
{
//Don't log error, let developer see
//original stack trace easily
throw;
#else
catch (Exception ex)
{
//Log error for developers and then
//throw a error with a user-oriented message
throw new Exception(String.Format
("Dear user, sorry but: {0}", ex.Message));
#endif
}
}
The way the question is worded, pitting "Throw:" vs. "Throw ex;" makes it a bit of a red-herring. The real choice is between "Throw;" and "Throw Exception," where "Throw ex;" is an unlikely special case of "Throw Exception."
I know this is an old question, but I'm going to answer it because I have to disagree with all the answers here.
Now, I'll agree that most of the time you either want to do a plain throw, to preserve as much information as possible about what went wrong, or you want to throw a new exception that may contain that as an inner-exception, or not, depending on how likely you are to want to know about the inner events that caused it.
There is an exception though. There are several cases where a method will call into another method and a condition that causes an exception in the inner call should be considered the same exception on the outer call.
One example is a specialised collection implemented by using another collection. Let's say it's a DistinctList<T> that wraps a List<T> but refuses duplicate items.
If someone called ICollection<T>.CopyTo on your collection class, it might just be a straight call to CopyTo on the inner collection (if say, all the custom logic only applied to adding to the collection, or setting it up). Now, the conditions in which that call would throw are exactly the same conditions in which your collection should throw to match the documentation of ICollection<T>.CopyTo.
Now, you could just not catch the execption at all, and let it pass through. Here though the user gets an exception from List<T> when they were calling something on a DistinctList<T>. Not the end of the world, but you may want to hide those implementation details.
Or you could do your own checking:
public CopyTo(T[] array, int arrayIndex)
{
if(array == null)
throw new ArgumentNullException("array");
if(arrayIndex < 0)
throw new ArgumentOutOfRangeException("arrayIndex", "Array Index must be zero or greater.");
if(Count > array.Length + arrayIndex)
throw new ArgumentException("Not enough room in array to copy elements starting at index given.");
_innerList.CopyTo(array, arrayIndex);
}
That's not the worse code because it's boilerplate and we can probably just copy it from some other implementation of CopyTo where it wasn't a simple pass-through and we had to implement it ourselves. Still, it's needlessly repeating the exact same checks that are going to be done in _innerList.CopyTo(array, arrayIndex), so the only thing it has added to our code is 6 lines where there could be a bug.
We could check and wrap:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentNullException ane)
{
throw new ArgumentNullException("array", ane);
}
catch(ArgumentOutOfRangeException aore)
{
throw new ArgumentOutOfRangeException("Array Index must be zero or greater.", aore);
}
catch(ArgumentException ae)
{
throw new ArgumentException("Not enough room in array to copy elements starting at index given.", ae);
}
}
In terms of new code added that could potentially be buggy, this is even worse. And we don't gain a thing from the inner exceptions. If we pass a null array to this method and receive an ArgumentNullException, we're not going to learn anything by examining the inner exception and learning that a call to _innerList.CopyTo was passed a null array and threw an ArgumentNullException.
Here, we can do everything we want with:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentException ae)
{
throw ae;
}
}
Every one of the exceptions that we expect to have to throw if the user calls it with incorrect arguments, will correctly be thrown by that re-throw. If there's a bug in the logic used here, it's in one of two lines - either we were wrong in deciding this was a case where this approach works, or we were wrong in having ArgumentException as the exception type looked for. It's the only two bugs that the catch block can possibly have.
Now. I still agree that most of the time you either want a plain throw; or you want to construct your own exception to more directly match the problem from the perspective of the method in question. There are cases like the above where re-throwing like this makes more sense, and there are plenty of other cases. E.g. to take a very different example, if an ATOM file reader implemented with a FileStream and an XmlTextReader receives a file error or invalid XML, then it will perhaps want to throw exactly the same exception it received from those classes, but it should look to the caller that it is AtomFileReader that is throwing a FileNotFoundException or XmlException, so they might be candidates for similarly re-throwing.
Edit:
We can also combine the two:
public CopyTo(T[] array, int arrayIndex)
{
try
{
_innerList.CopyTo(array, arrayIndex);
}
catch(ArgumentException ae)
{
throw ae;
}
catch(Exception ex)
{
//we weren't expecting this, there must be a bug in our code that put
//us into an invalid state, and subsequently let this exception happen.
LogException(ex);
throw;
}
}
You should always use "throw;" to rethrow the exceptions in .NET,
Refer this, http://weblogs.asp.net/bhouse/archive/2004/11/30/272297.aspx
Basically MSIL (CIL) has two instructions - "throw" and "rethrow" and C#'s "throw ex;" gets compiled into MSIL's "throw" and C#'s "throw;" - into MSIL "rethrow"! Basically I can see the reason why "throw ex" overrides the stack trace.
The first preserves the original stack trace of the exception, the second one replaces it with the current location.
Therefore the first is BY FAR the better.
If you throw an exception without a variable (the second example) the StackTrace will include the original method that threw the exception.
In the first example the StackTrace will be changed to reflect the current method.
Example:
static string ReadAFile(string fileName) {
string result = string.Empty;
try {
result = File.ReadAllLines(fileName);
} catch(Exception ex) {
throw ex; // This will show ReadAFile in the StackTrace
throw; // This will show ReadAllLines in the StackTrace
}
It depends. In a debug build, I want to see the original stack trace with as little effort as possible. In that case, "throw;" fits the bill. In a release build, however, (a) I want to log the error with the original stack trace included, and once that's done, (b) refashion the error handling to make more sense to the user. Here "Throw Exception" makes sense. It's true that rethrowing the error discards the original stack trace, but a non-developer gets nothing out of seeing stack trace information so it's okay to rethrow the error.
void TrySuspectMethod()
{
try
{
SuspectMethod();
}
#if DEBUG
catch
{
//Don't log error, let developer see
//original stack trace easily
throw;
#else
catch (Exception ex)
{
//Log error for developers and then
//throw a error with a user-oriented message
throw new Exception(String.Format
("Dear user, sorry but: {0}", ex.Message));
#endif
}
}
The way the question is worded, pitting "Throw:" vs. "Throw ex;" makes it a bit of a red-herring. The real choice is between "Throw;" and "Throw Exception," where "Throw ex;" is an unlikely special case of "Throw Exception."
The first preserves the original stack trace of the exception, the second one replaces it with the current location.
Therefore the first is BY FAR the better.
My preferences is to use
try
{
}
catch (Exception ex)
{
...
throw new Exception ("Put more context here", ex)
}
This preserves the original error, but allows you to put more context, such as an object ID, a connection string, stuff like that. Often my exception reporting tool will have 5 chained exceptions to report, each reporting more detail.
The first is better. If you try to debug the second and look at the call stack you won't see where the original exception came from. There are tricks to keep the call-stack intact (try search, it's been answered before) if you really need to rethrow.
If you throw an exception without a variable (the second example) the StackTrace will include the original method that threw the exception.
In the first example the StackTrace will be changed to reflect the current method.
Example:
static string ReadAFile(string fileName) {
string result = string.Empty;
try {
result = File.ReadAllLines(fileName);
} catch(Exception ex) {
throw ex; // This will show ReadAFile in the StackTrace
throw; // This will show ReadAllLines in the StackTrace
}
The first is better. If you try to debug the second and look at the call stack you won't see where the original exception came from. There are tricks to keep the call-stack intact (try search, it's been answered before) if you really need to rethrow.
If you throw an exception without a variable (the second example) the StackTrace will include the original method that threw the exception.
In the first example the StackTrace will be changed to reflect the current method.
Example:
static string ReadAFile(string fileName) {
string result = string.Empty;
try {
result = File.ReadAllLines(fileName);
} catch(Exception ex) {
throw ex; // This will show ReadAFile in the StackTrace
throw; // This will show ReadAllLines in the StackTrace
}
You should always use "throw;" to rethrow the exceptions in .NET,
Refer this, http://weblogs.asp.net/bhouse/archive/2004/11/30/272297.aspx
Basically MSIL (CIL) has two instructions - "throw" and "rethrow" and C#'s "throw ex;" gets compiled into MSIL's "throw" and C#'s "throw;" - into MSIL "rethrow"! Basically I can see the reason why "throw ex" overrides the stack trace.
Source: Stackoverflow.com