Modify an existing program to better manage exceptions

Question

I am working on an existing program in C #, which requests and makes changes to various resources outside the program and the computer on which the program is running. The program runs on most computers, but randomly fails on random computers so often. When this happens, we have no feedback as to why, except that the user informs us of an exception that was selected on the screen, which they might or could not have cleared. Even if they leave it on the screen for us, it does not give us any useful information. Since then, I modified the program to register specific events and collect information in a text file, which I can use to get closer to the program stopping. This, at least, is the beginning, but from reading many threads on stackoverflow.com, as well as other forums that I know that the program should at least try to handle specific exceptions that may occur when querying and changing LDAP, DS, DNS, SQL and so on.

Currently, there are only large try blocks surrounding everything to make code with only one catch block at the end of each.

private void method_name() { try { //many lines of to do code calling many other methods } catch (exception ex) { MessageBox.Show("An Error has occurred in method_name() :" + ex.Message); } 

My first thought was to embed more specific try / catch blocks in larger try blocks, but I continue to run into problems when variables become inaccessible in different contexts. eg

 try { LdapConnection ldapConn = new LdapConnection(); ldapConn.Connect(details of connection); ldapConn.Bind(details of bind statement); LdapSearchQueue queue = ldapConn.Search(search criteria and such); LdapMessage message; } catch (somesortofexception ex) { //do something sensible about it } while ((message = queue.getResponse()) != null) { //do things with message } ldapConn.Disconnect(); 

The problem is that the message and the queue are not available outside the try block. This is just one example of what was done with "// many lines of code that calls many other methods" that I'm trying to work with.

So here are my questions:

Would it be better to try to make the try block larger in the above example to enable the while and ldap disconnect loops, or just leave the big try loop, make a list of things that happen during this, and create a lot of catch blocks at the end to catch specific exceptions? I feel that including smaller try blocks around specific code is a path based on what I read on this site.

Should I use smaller try blocks, as I tried to implement, would it be nice to use a catch block that catches any exception raised in this small piece of code and writes it to the log file, or do I need to try to catch certain exceptions? I have nothing else I can do with these exceptions, except to write them to a file anyway.

Should exceptions be excluded? I really do not want something to click on the user, except for a message in plain English, which said that something went wrong and contact IT. Currently, none of the catch blocks throw anything.

Answer 1

On breaking exceptions, I will always separate the connection code from the request.

So this will be as follows:

 LdapConnection ldapConn = new LdapConnection(); try { ldapConn.Connect(details of connection); ldapConn.Bind(details of bind statement); } catch (somesortofexception ex) { //Log, send error message.. ldapConn = null; } if (ldapConn != null) { try { //Do what you need with your connection } catch (Exception ex) { //Log, Error.... } finally { //Disconnect your ldap here } } 

Ideally, I would put all the connection code and the search code in different methods, so you would have a more pleasant stack trace.

About the error message I would also just use some general messages and log exclusion features in some file ( http://logging.apache.org/log4net/ ) is pretty nice for well formatted log files.

Answer 2

The local catch block that swallows a particular exception is fine as long as you expect this exception and can be handled locally. In this case, you can provide information to the user based only on what the exception contains, or you can move the definitions of the variables above the try block if you want to include their state in the log and / or message.

For exceptions that you do not expect, you should let them soar up to the top of the call stack, where they can be registered by the global handler before exiting the program gracefully. You do not want to potentially nail a corpse in an upright position, swallowing these exceptions.

Assuming this is a WinForms application, setting up global handlers looks something like this:

 public static void Main(string[] args) { // Switch-off the Windows Forms default handler for unhandled exceptions. // NB From .NET 4 upwards, this won't work if the process state is corrupted. Application.SetUnhandledExceptionMode(UnhandledExceptionMode.CatchException); // Setup event handler to intercept an unhandled exception on a UI thread. // NB The exception will still terminate the application. // But you can show a MessageBox and/or log the exception. Application.ThreadException += new ThreadExceptionEventHandler(App_UiThreadException); // Setup event handler to intercept unhandled exception on a non-UI thread. AppDomain.CurrentDomain.UnhandledException += new UnhandledExceptionEventHandler(App_NonUiThreadException); // Run the application (open main form etc). }