Every client I have done consulting at, without exception (so far), has done exception handling incorrectly in .NET.
This should not be too surprising given that people in VB6 and C++ rarely did exception handling right either.
In .NET (and Java), there are just a few things you have to remember (compared to VB6/C++) to do exception handling correctly. Yet in ~5 years of looking for these simple rules, I have not yet found them (I discovered them the hard way, by maintaining production code that did exception handling incorrectly).
Here is my attempt at documenting good (and pragmatic) exception handling practices. (Microsoft's attempt is here: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpguide/html/cpconBestPracticesForHandlingExceptions.asp)
These examples are specifically for VB.NET, but they basically apply in the same way to Java and C#.
How not to do it:
1)
On Error Resume Next
What you are doing: You are ignoring *every* error and pretending it never happened and continuing execution.
Why that is bad: Errors happen for a reason. If you aren't even checking to see what error happened, how do you know you can safely continue to execute? Continuing execution after an error occurred can easily lead to file or database corruption among other problems.
2)
Try
... code here ...
Catch
End Try
What you are doing: You are ignoring *every* error and pretending it never happened and continuing execution.
Why that is bad: Errors happen for a reason. If you aren't even checking to see what error happened, how do you know you can safely continue to execute? Continuing execution after an error occurred can easily lead to file or database corruption among other problems.
3)
Try
... code here ...
Catch exception As Exception
End Try
What you are doing: You are ignoring *every* .NET Exception and pretending it never happened and continuing execution.
Why that is bad: Errors happen for a reason. If you aren't even checking to see what error happened, how do you know you can safely continue to execute? Continuing execution after an error occurred can easily lead to file or database corruption among other problems.
4)
Try
... code here ...
Catch exception As Exception
Throw exception
End Try
What you are doing: You are resetting the stack trace for the error.
Why that is bad: A normal stack trace will show the exact line the error originated from. In this scenario, the stack trace will instead point the Throw line as the origin of the error.
--
Unless you are at the absolute top of the stack (i.e. the main method or a global exception handler), you never want to catch exceptions of type "Exception". You should only be catching Exception classes that are below Exception in the hierarchy, and even more strongly, the leaf nodes of the Exception class hierarchy.
How to do exception handling properly:
1)
Really, try/catch is better than On Error GoTo, but this example is more for VB6 programmers who don't have try/catch.
On Error GoTo <ExceptionHandler>
You need to do all of the following in the Exception Handler:
(a) Close any open resources (database connections, etc.)
(b) Check the error/error code
(c) Check the line that the error occured on
(d) Put the error (message and stack trace with line numbers) somewhere so someone can see it (to the screen, to a log file, to a database, etc.)
(e) Take appropriate action (i.e. return an error code from the current function, exit the application, etc.)
2)
Try
... code here ...
Catch exception As SqlException
Log(exception.ToString, selectStatement)
Throw
End Try
Why is this better:
You are catching a specific exception class (SqlException) instead of something higher level like Exception.
You are logging the exception to a file, database, e-mail, etc. so a support person can log a defect for development to look at.
You are logging the database SELECT statement that was in use at the time the exception occurred.
You are rethrowing the exception as is so the stack trace will remain intact.
3)
Try
... code here ...
Catch innerException As SqlException
Log(innerException.ToString)
Throw New CustomException("SQL Statement [" & selectStatement & "]", innerException)
End Try
Why is this better:
You are catching a specific exception class (SqlException) instead of something higher level like Exception.
You are logging the exception to a file, database, e-mail, etc. so a support person can log a defect for development to look at.
You are throwing a custom exception that the calling method may be able to deal with easier/more successfully than a SqlException.
You are improving the information available in the exception.
You are including the innerException so that no information is lost on the new custom throw.
--
How do deal with third party event driven code:
If you are using badly behaved third party event driven code, it will sometimes throw away useful exception information.
For example, if you are handling/overriding events thrown by third party code, and an exception occurs in your code, that exception should go into the third party code and then make it back into your code that originally called the third party code. Instead, the third party code may throw away the exception (because they have bad exception handling as described above).
In that case, before you leave your event handler, you need to catch every exception and log it so it doesn't get completely lost. You should rethrow it, just in case the third party exception handling code is eventually fixed and they propagate the exception properly in the future.
Try
... code here ...
Catch innerException As Exception
LogToFile("innerException [" & innerException.ToString() & "]")
Throw New BizException("Exception in event handler for <third party package> Code", innerException)
End Try
--
A stack trace is a very important debugging tool. If exceptions are not handled properly, stack traces get lost or mangled, making debugging much, much harder.
Here are some code samples with accompanying stack traces:
1)
Here is a fairly simple example that calls a function which causes an exception to be thrown.
This example has no try/catch block in the function, which is normal default behavior. The try/catch block in the Main method is just so we can view the exception before the application closes and isn't necessary/useful for any other purpose.
--
Module Module1
Sub Main()
Try
Dim age As Integer
Dim ageAsString As String = "a1234"
age = ConvertStringAgeToInteger(ageAsString)
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
End Sub
Function ConvertStringAgeToInteger(ByVal ageAsString As String) As Integer
Dim age As Integer
age = CInt(ageAsString)
Return age
End Function
End Module
--
Here is the associated stack trace. It's pretty basic. The DoubleType.Parse method threw a FormatException, which was then wrapped and rethrown in the IntegerType.FromString method by an InvalidCastException. We don't have the line number in the stack trace for the Microsoft.VisualBasic component, but we do have the proper line numbers for our code. Line 16 is the call to CInt. Line 7 is the call to ConvertStringAgeToInteger.
--
System.InvalidCastException: Cast from string "a1234" to type 'Integer' is not valid. ---> System.FormatException: Input string was not in a correct format.
at Microsoft.VisualBasic.CompilerServices.DoubleType.Parse(String Value, NumberFormatInfo NumberFormat)
at Microsoft.VisualBasic.CompilerServices.DoubleType.Parse(String Value)
at Microsoft.VisualBasic.CompilerServices.IntegerType.FromString(String Value)
--- End of inner exception stack trace ---
at Microsoft.VisualBasic.CompilerServices.IntegerType.FromString(String Value)
at ExceptionHandlingSampleCode.Module1.ConvertStringAgeToInteger(String ageAsString) in C:\mmaddox\dev\Prototype\ExceptionHandlingSampleCode\Module1.vb:line 16
at ExceptionHandlingSampleCode.Module1.Main() in C:\mmaddox\dev\Prototype\ExceptionHandlingSampleCode\Module1.vb:line 7
--
2)
We will now modify the above example to add some bad practices and see how the stack trace changes.
--
Module Module1
Sub Main()
Try
Dim age As Integer
Dim ageAsString As String = "a1234"
age = ConvertStringAgeToInteger(ageAsString)
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
End Sub
Function ConvertStringAgeToInteger(ByVal ageAsString As String) As Integer
Dim age As Integer
Try
age = CInt(ageAsString)
Catch
End Try
Return age
End Function
End Module
--
There is no output, we have completely ignored/squelched/destroyed the error and error information. The age variable was never set as expected, but we have no way of knowing that.
3)
This is basically the same as above, but we will no longer catch non-.NET exceptions (that is the difference between "Catch" and "Catch exception As Exception".
--
Module Module1
Sub Main()
Try
Dim age As Integer
Dim ageAsString As String = "a1234"
age = ConvertStringAgeToInteger(ageAsString)
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
End Sub
Function ConvertStringAgeToInteger(ByVal ageAsString As String) As Integer
Dim age As Integer
Try
age = CInt(ageAsString)
Catch exception As Exception
End Try
Return age
End Function
End Module
--
Again, there is no output for the same reasons as above (the exception is a .NET exception, so it is caught and ignored).
4)
Now we will try to rethrow the exception incorrectly.
--
Module Module1
Sub Main()
Try
Dim age As Integer
Dim ageAsString As String = "a1234"
age = ConvertStringAgeToInteger(ageAsString)
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
End Sub
Function ConvertStringAgeToInteger(ByVal ageAsString As String) As Integer
Dim age As Integer
Try
age = CInt(ageAsString)
Catch exception As Exception
Throw exception
End Try
Return age
End Function
End Module
--
Here is the output. The big difference is that instead of line 16 (the call to CInt), the stack trace now shows line 19 (the Throw statement). If the try block was 100 lines long, we would have no idea which line in the Try block caused the exception.
--
System.InvalidCastException: Cast from string "a1234" to type 'Integer' is not valid. ---> System.FormatException: Input string was not in a correct format.
at Microsoft.VisualBasic.CompilerServices.DoubleType.Parse(String Value, NumberFormatInfo NumberFormat)
at Microsoft.VisualBasic.CompilerServices.DoubleType.Parse(String Value)
at Microsoft.VisualBasic.CompilerServices.IntegerType.FromString(String Value)
--- End of inner exception stack trace ---
at ExceptionHandlingSampleCode.Module1.ConvertStringAgeToInteger(String ageAsString) in C:\mmaddox\dev\Prototype\ExceptionHandlingSampleCode\Module1.vb:line 19
at ExceptionHandlingSampleCode.Module1.Main() in C:\mmaddox\dev\Prototype\ExceptionHandlingSampleCode\Module1.vb:line 7
--
5)
So, now let's actually try to fix the code using a value of -1 to specify an invalid input value.
--
Module Module1
Sub Main()
Dim age As Integer
Try
Dim ageAsString As String = "a1234"
age = ConvertStringAgeToInteger(ageAsString)
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
System.Diagnostics.Debug.WriteLine("age is [" & age & "]")
End Sub
' If this function returns -1, an error occured
Function ConvertStringAgeToInteger(ByVal ageAsString As String) As Integer
Dim age As Integer
Try
age = CInt(ageAsString)
Catch exception As InvalidCastException
age = -1
End Try
Return age
End Function
End Module
--
Here is the output. This is workable, but I'd rather see a very useful exception thrown than having to check for an error code (error codes are pretty much obsolete with .NET, unless you are doing COM Interop). That brings us back to the very first sample code example, which I believe is more correct. Don't catch exceptions unless you can actually do something useful with them!
--
age is [-1]
--
--
Here is an example of a case where I have found it useful to catch an exception. In .NET 1.1, Integer.Parse will throw one of these three exceptions if the string is not parseable. It would be quite a pain to check for these three exceptions every time we wanted to convert a string to an integer, but that is the proper way to do it (if you tried to catch a common parent exception, you might accidentally catch an OutOfMemoryException or some other exception that you should let bubble up to call stack).
An interesting note, in .NET 1.1, the Double class is the only one that has a "TryParse" method that will parse and return a boolean instead of an exception. In .NET 2.0, they addressed this shortcoming. The Integer class also has a TryParse method in .NET 2.0.
--
Module Module1
Sub Main()
Try
If (IsValidInteger(Nothing)) Then
System.Diagnostics.Debug.WriteLine("The Integer was valid!")
End If
Catch exception As Exception
System.Diagnostics.Debug.WriteLine(exception.ToString)
End Try
End Sub
Function IsValidInteger(ByVal integerAsString As String) As Boolean
Try
Integer.Parse(integerAsString)
Catch argumentNullException As ArgumentNullException
System.Diagnostics.Debug.WriteLine(argumentNullException.ToString)
Return False
Catch formatException As FormatException
System.Diagnostics.Debug.WriteLine(formatException.ToString)
Return False
Catch overflowException As OverflowException
System.Diagnostics.Debug.WriteLine(overflowException.ToString)
Return False
End Try
Return True
End Function
End Module
--