views:

691

answers:

5

I recently had to develop an additional module for an existing service developed by a colleague. He had placed a try/catch block in the main working function for catching all unhadled exceptions that bubbled up to this level, logging them together with stack trace info etc:

try
{
    // do main work
}
catch(Exception ex)
{
    // log exception info
}

While this makes the program very stable (as in 'unlikely to crash'), I hate it because when I am testing my code, I do not see the exceptions caused by it. Of course I can look at the exception log and see if there are new entries, but I very much prefer the direct feedback of getting the exception the moment it is thrown (with the cursor on the right line in the code, please).

I removed this top level try/catch at least while i was still coding and testing. But now my task is finished and I have to decide wether to put it back in for the release, or not. I think that I should do it, because it makes the service more stable, and the whole point of it is that it runs in the background without needing any supervision. On the other hand I have read that one should only call specific exceptions (as in IoException), not generically Exception.

What is your advice on this issue?

By the way, the project is written in C#, but I am also interested in answers for non .NET languages.

+7  A: 

Put it back.

The exception should be only relevant while testing. Otherwise it doesn't make sense pop it to the user.

Logging is fine.

You may additionally use the DEBUG symbol defined by Visual Studio for debug builds as a flag.

    ...
    } catch( Exception e ) { 
#if DEBUG
          throw;
#else
          log as usual 
#endif
    }

So next time a modification is needed the debug flag should be set to true and the exception will pop up.

OscarRyz
Hm, I like the debug flag!
Treb
Shouldn't he use 'throw' not 'throw e'? Otherwise, the stack trace is cleared (or something -- not too sure).
strager
@strager: Yup! The above example will mask the original exception. Just use 'throw'.
Theo Lenndorff
OscarRyz
@Reyes, Yes. Thanks. =]
strager
Why a variable named 'debug' and not a #ifdef DEBUG directive? The later is automatically defined for debug builds in Visual Studio.
Sergio Acosta
I mean, using #ifdef DEBUG you don't have to alter any code (like setting your debug variable to true) to turn off the exception throwing, just compile the Release build.
Sergio Acosta
@Oscar: ok. I made the change.
Sergio Acosta
@Sergio. Thanks. I guess that's more C sharpy. ;)
OscarRyz
+1  A: 

See also http://stackoverflow.com/questions/576532/is-dying-is-awesome-preferred

Brian
Thanks for pointing me to it, that discussion was very helpful
Treb
+1  A: 

Ideally you want to handle an exception as close to where it occured as possible but that doesn't mean a global exception handler is a bad idea. Especially for a service which must remain running at all costs. I would continue what you have been doing. Disable it while debugging but leave it in place for production.

Keep in mind it should be used as a safety net. Still try to catch all exceptions before they elevate that far.

codeelegance
+1  A: 

In any Java application, you just about always want to define an exception handler for uncaught exceptions with something like this:

Thread.setDefaultUncaughtExceptionHandler( ... );

where the object that will catch these uncaught exceptions will at least log the failure so you have a chance to know about it. Otherwise, there is no guarantee that you'll even be notified that a Thread took an Exception -- not unless it's your main Thread.

In addition to doing this, most of my threads have a try/catch where I'll catch RunnableException (but not Error) and log it ... some Threads will die when this happens, others will log and ignore, others will display a complaint to the user and let the user decide, depending on the needs of the Application. This try/catch is at the very root of the Thread, in the Runnable.run() method or equivalent. Since the try/catch is at the root of the Thread, there's no need to sometimes disable this catch.

When I write in C#, I code similarly. But this all depends on the need of the application. Is the Exception one that will corrupt data? Well then, don't catch and ignore it. ALWAYS log it, but then Let the application die. Most Exceptions are not of this sort, however.

Eddie
+1  A: 

The urge to catch all exceptions and make the program 'stable' is very strong and the idea seems very enticing indeed to everyone. The problem as you point out is that this is just a ruse and the program may very well be buggy and worse, w/o indications of failures. No one monitors the logs regularly.
My advice would be to try and convince the other developer to do extensive testing and then deploy it in production w/o the outer catch.

Mohit Chakraborty
I understand where you are coming from, but there is a great class of Exceptions where your program may run in a degraded state, but still run, where you really want to catch-and-log Exceptions. Stability does matter. You just have to know where you CANNOT do this.
Eddie
@Eddie - True, this area is always very much application and user-expectation dependent. We all would like to make robust performant applications that never fail but then reality calls :).Nothing beats extensive upfront testing, code coverage and betas w/ real customers to get confidence.
Mohit Chakraborty