Introduction
Microsoft advices in several documents never to do try { ... } catch (Exception ex) { ... } if you don't rethrow the same exception. This document explains how you can attempt to comply to this .NET Framework design guidelines rule: Do Not Catch Exceptions That You Cannot Handle.
Background
After working with Visual Basic 6 error-handling for several years, .NET exception handling comes as a relief. Although it is a major improvement compared to on error goto statements, it does have some problems. Since there are already plenty of resources out there that explain exception handling, I won't do that in this document. Instead, I'll explain how I attempt to comply to one of the .NET Framework design guidelines rules: Do Not Catch Exceptions That You Cannot Handle.
Or in other words: You should never catch System.Exception or System.SystemException in a catch block because you could inadvertently hide run-time problems like Out Of Memory. Refer to the Improving .NET Application Performance and Scalability document on Patterns & Practices website for more information on this rule. An example of something you should not do:
try {
intNumber = int.Parse(strNumber);
} catch (Exception ex) {
Console.WriteLine("Can't convert the string to " +
"a number: " + ex.Message);
}
I agree that most of the times you should let the exception propagate up the call stack. However, this doesn't always work for me. For example, suppose I have an unattended application that copies files from one directory to another (it's a stupid example, I know, I just want to prove my point). I want this application to continue working regardless of what goes wrong during the file-copy operation because I'm going to send an e-mail in the end informing the administrator of all the failures. However, I don't want the application to continue if it throws an OutOfMemoryException or a BadImageFormatException for example. This is the C# code of my little application:
static void Main(string[] args)
{
string[] strFiles =
System.IO.Directory.GetFiles(@"C:\Temp");
System.Collections.ArrayList objSuccesses =
new System.Collections.ArrayList();
System.Collections.ArrayList objFailures =
new System.Collections.ArrayList();
foreach (string strFile in strFiles)
{
try
{
System.IO.File.Copy(strFile, strFile + ".new");
objSuccesses.Add(strFile);
}
catch (Exception ex)
{
Console.WriteLine("Failed to copy the file: " +
ex.Message);
objFailures.Add(strFile);
}
}
}
If we follow Microsoft's guidelines then we have to catch only those exceptions that could be triggered by the File.Copy method. If you look in the .NET Framework's Class Library, you'll see that this is UnauthorizedAccessException, ArgumentException, ArgumentNullException, PathTooLongException, DirectoryNotFoundException, FileNotFoundException, IOException and NotSupportedException. If you ask me, catching all these exceptions individually is not doable. All I wanted to know is whether or not a file copy failed or not. It would be sufficient to only capture IOException, ArgumentException and NotSupportedException (because all the other exceptions derive from these three exceptions) but to know this I have to read the documentation again. I really don't want to go through that much trouble to simply copy a file.
The guideline assumes that we know all the exceptions that can be thrown by the task executed. Copying a file is a very simple operation that is properly documented by Microsoft. But what if we're using some third-party tool? The stability of our unattended tool depends on the quality of the documentation of this tool. If this third-party forgets to mention one of the exceptions that their tool throws in the documentation, then our unattended application could stop prematurely.
Also, suppose that we know all the exceptions that can be thrown by this tool and suppose that we write our code to handle all these exceptions individually. What happens if we start using a new version of this tool one day? This new version could have the exact same API, but could be throwing more detailed or other exceptions. We therefore need to compare all the exceptions thrown by this tool with the ones that we capture in our code. Again, not doable...
Also, suppose I wanted to use for example the Save method of the System.Drawing.Image class. This method doesn't even list any exceptions; so how can I possibly handle an exception thrown by this method according to the guideline? This method is supposed to save an image to disk; there are dozens of things that can go wrong.
The feature that is missing in C# according to me is the ability to catch all the exceptions except the critical ones. To me Critical exceptions are for example, Out Of Memory, or AppDomain Is Unloading. That's why I wrote a small static method that tests if the exception thrown is a critical one. If it is, the exception is rethrown (the application could also simply stop gracefully). If it isn't a critical exception, it is logged, swallowed and the application continues. This is the code of my IsCritical method:
public static bool IsCritical(Exception ex)
{
if (ex is OutOfMemoryException) return true;
if (ex is AppDomainUnloadedException) return true;
if (ex is BadImageFormatException) return true;
if (ex is CannotUnloadAppDomainException) return true;
if (ex is ExecutionEngineException) return true;
if (ex is InvalidProgramException) return true;
if (ex is System.Threading.ThreadAbortException)
return true;
return false;
}
This is the code of the application while using the IsCritical method:
static void Main(string[] args)
{
string[] strFiles =
System.IO.Directory.GetFiles(@"C:\Temp");
System.Collections.ArrayList objSuccesses =
new System.Collections.ArrayList();
System.Collections.ArrayList objFailures =
new System.Collections.ArrayList();
foreach (string strFile in strFiles)
{
try
{
System.IO.File.Copy(strFile, strFile + ".new");
objSuccesses.Add(strFile);
}
catch (Exception ex)
{
if (IsCritical(ex)) throw;
Console.WriteLine("Failed to copy the file: " +
ex.Message);
objFailures.Add(strFile);
}
}
}
If you think that using this IsCritical method is a good idea, please let me know. If you know a reason why it's a bad idea, please do let me know.
| You must Sign In to use this message board. |
|
|
 |
|
 |
Michael,
For your first scenario the follow may address that.
I am OK with throwing out the argument of needing to explicitly catch every different kind of exception.
That leaves how to generically handle exceptions and more importantly what to do with unexpected exceptions. My personal opinion would still be to test for the exceptions that I don't want to "error out" on and treat everything else as fatal. Your philosophy is to only look for the errors you consider fatal and treat everything else as a non-fatal, documented or not. That's OK as long as you understand you may be masking problems. If that is not an issue or is needed with a given application, then your method works just fine.
If your code is a library that I may use in my own code, personally, I would not appreciate the errors being masked out and I would ask for an option for it not to do that. If on the other hand your code is run by your company and it is used to run other peoples' code in this MaintenanceTask module, then what you are doing may be exactly appropriate. My only comment on that is you should at least print out the stack trace somewhere so it can be reviewed by someone so those other customers can be told that their code failed and where it failed. I understand that the routine you presented above has been simplified and you may already be doing that.
Your design philosophy is just different than mine. I would rather have the job error out on something unexpected and have me evaluate if it should of errored out at that point and make a coding change at that time if it is needed. Call me a "control freak" 
Since my design philosophy has a different focus than yours, what I do in practice when I write a "batch" job in c# is a little different. I will write my code with very few error handlers, usually just one. In my main I will add one generic try-catch statement that makes no distinction as to what the exception is (OK, I have one exception to that, but I will discuss that later). All this generic handler does is trap the exception and print out the error message, the stack trace, and exits with a non-zero return code for our job scheduler to pick up and tell the operators that the job failed. We always deploy a .pdb file with our apps so that the stack traces will tell us which line of code failed. You just need to modify the project properties to generate one for "Release" code, it's done by default for "Debug" code. Now, if my code cares about trapping for a particular exception, then I will code for that particular exception and handle the condition or re-throw the exception.
OK, now for those who have not yet fallen asleep reading this, my one exception to my generic exception handler is for SQL errors. We use MS SQL Server extensively and I will catch those errors in my main also. I do this as a separate handler because in addition to being able to print the error message and stack trace the SQL exception object also will tell me what line of code in the Stored Proc failed, which is also very helpful.
As far as your second scenario, that one is a little trickier. I will assume the focus of this is from a library where you are throwing the errors and someone else has to catch them. I have to deal with this issue myself. We purchased a third party VB6 COM library a while back. In that code the routines just set a return code and I need to check the code in context of what is happening. Now since we have "upgraded" to their .NET version of the same library they are throwing about ten different kinds of exception objects. Personally, I would have preferred a little more of a "happy medium" on this one and to have fewer objects to catch and have a richer object to deal with that would tell me what went wrong.
More specifically in your case, unless you have a good reason to catch specific errors, like ones that are truly expected and non-fatal, I think you should just let errors percolate through. Leave it up to the developer using your library to handle the errors. If you are distributing code to the outside world and you are deliberately "hiding" errors by not re-throwing them just because you don't know what they are, then my guess is when things don't work as expected it is going to be much harder for them and you to track down the real problem during the support call. As far as the error objects you create and throw, yes, don't have too many different types. Make fewer (or one) error object and back it up with enough attributes to fully describe the problem to the caller.
Again, this is just my opinion and I don't know all of the details of your programs. Good Luck, Jim
|
| Sign In·View Thread·PermaLink | 4.00/5 |
|
|
|
 |
|
 |
very interesting articles which makes me think a lot.
yep, if you have a batch processor, you want it to run, no matter what, but if you catch (almost) all exception you could have a big problem masked, mhh. how to handle that ?
but there is a solution exactly for that in the API I just realized!
in the old days you will have a batch processor as a process which would spawn child process for each maintenance task.
but here comes .NET and you have application domain ! your main loop should be in one application domain, and each maintenance task in its own. what about that, hey ? Moreover ApplicationDomain have been created exactly to solve such issues !
|
| Sign In·View Thread·PermaLink | 3.00/5 |
|
|
|
 |
|
 |
I don't think that I fully understand your application-domain-solution. I understand that having your main loop in one application domain and having your maintenance tasks in separate application domains solves some problems: If you maintenance task crashes, than it will only be that application domain that crashes and not the main loop. But how does that solve the exception-dilema? Not all 'critical' exceptions are so bad that they cause your application domain to crash. Some (like 'out of memory') are also very important but will not (to my knowledge, I might be wrong) stop your application domain from running. So how you deal with that?
But, let's assume for now that this application domain-solution solves my maintenance-task problem. I doesn't solve the Application.Login-problem that I described earlier. I personally don't think that the solution Jim (jfos) suggested using pdb-files is feasible for us because we're going to obfuscate our library. I don't think that the pdb-files will still work. Jim also stated in his latest response that I should let exceptions percolate through so that they developers using my library can decide what to do with it. I see three ways of doing this:
Solution 1 ---------- Either my code does this:
public class Application { void Login(string username, string password) { ConnectToDatabase(); VerifyLicenseKey(); ConnectToDotNetRemotingServer(); ... } }
in which case a user might receive ApplicationExceptions that I trew because the license key was invalid for example. But he might also receive ADO.NET exceptions because I had problems with the database or he might get .NET Remoting Exceptions... The disadvantages of this according to me are: 1. Documenting all the exceptions that this method could throw is A LOT of work for me to do. I don't only have to document all exceptions thrown by Application.Login but also every exception that can possibly be thrown by any of the methods that Application.Login calls! 2. Trapping all these exceptions is also a lot of work for a user of my library. Most of the time, as a developer, they don't even want to know why it failed because they can't do much about it. They just want to know if the login was successful or not. I personally feel that I can't expect from my customers to catch 5 or even 10 different exceptions simply to check if a login was successful. 3. The advantage of using a business-layer (like my library is) is that you hide most of the complexity from users using your business-layer. I've decided to use .NET Remoting in this version, but if I run into problems with it later on, I might decide to use Memory Mapped Files in the next version of my library. Technically, this shouldn't have any impact on my users because they only call Application.Login; they don't even know that I use .NET Remoting. But the problem is that I used to let .NET Remoting exceptions bubble all the way up so they had to catch these exceptions! If they would start using my newest version, their exception handler suddenly becomes useless.
Solution 2 ---------- Another thing that I could do is this: public class Application {
void Login(string username, string password) { try { ConnectToDatabase(); VerifyLicenseKey(); ConnectToDotNetRemotingServer(); ... } catch (Exception ex) { throw new MyApplicationException("Unable to login", ex); } } }
This would be the easiest solution for users of my library. They just have to catch MyApplicationException to know if they've logged in or not. But this also has two disadvantages:
1. I hide some of the details of the exception. Using my previous code, a user might receive a 'can't connect to database'-exception which is cool because they immediately know what's wrong. They can also write your code to handle exceptions in a specific way. 2. The biggest danger with this way of working is that it hides important exceptions again. So, I can't do that.
Solution 3 ---------- Another thing I could do is: public class Application {
void Login(string username, string password) { try { ConnectToDatabase(); VerifyLicenseKey(); ConnectToDotNetRemotingServer(); ... } catch (Exception ex) { if IsCritical(ex) throw; throw new MyApplicationException("Unable to login", ex); } } }
This has the advantage that any critical exception simply walks up the stack, but any other exception is wrapped in a MyApplicationException. Users of my library can now safely catch MyApplicationException not having to worry that they would catch an 'out of memory'-exception. But I agree, what do we do if a new critical exception appears?
|
| Sign In·View Thread·PermaLink | 3.00/5 |
|
|
|
 |
|
 |
As a former VB developer, I agree with your analysis. I can say that I see where he is coming from. It is a misguided way of thinking that comes from his VB experience. In VB there is no concept of exception handling. Coming from a VB background one will think that all errors are bad, when in fact when they are not. I mean really if ANY error occurs it must be trapped by a ON ERROR GOTO statement, which jumps to a block of code that handles ALL errors, and it is optional to check what ERR is set to so that you can get an integer value of the last error. This "flat" approach to error handling is what he is attempting to partially recreate when it's a horrible system and bad design from the start.
I used to write code that would let no error pass by, and I thought I was writing "solid" code. That just wasn't true. The fact is, if something "unexpected" happens then you need to let it follow through it's due course, and not try to handle everything just to keep something from crashing. Crashing is meant to allow a program has malfunctioned to cease operating. Interfering in this often causes more harm than good. Exceptions are GOOD things, not BAD things. Embrace them! 
|
| Sign In·View Thread·PermaLink | 3.13/5 |
|
|
|
 |
|
|