Should I catch exceptions only to log them?

Question

Should I catch exceptions for logging purposes?

public foo(..)
{
   try
   {
     ...
   } catch (Exception ex) {
     Logger.Error(ex);
     throw;
   }
}

If I have this in place in each of my layers (DataAccess, Business and WebService) it means the exception is logged several times.

Does it make sense to do so if my layers are in separate projects and only the public interfaces have try/catch in them? Why? Why not? Is there a different approach I could use?

Answer 1

You need to develop a strategy for handling exceptions. I don't recommend the catch and rethrow. In addition to the superfluous log entries it makes the code harder to read. Consider writing to the log in the constructor for the exception. This reserves the try/catch for exceptions that you want to recover from; making the code easier to read. To deal with unexpected or unrecoverable exceptions, you may want a try/catch near the outermost layer of the program to log diagnostic information.

BTW, if this is C++ your catch block is creating a copy of the exception object which can be a potential source of additional problems. Try catching a reference to the exception type:

  catch (const Exception& ex) { ... }

Answer 2

It depends on the Exception: if this actually should not happen, I definitely would log it. On the other way: if you expect this Exception you should think about the design of the application.

Either way: you should at least try to specify the Exception you want to rethrow, catch or log.

public foo(..)
{
   try
   {
     ...
   }
   catch (NullReferenceException ex) {
     DoSmth(e);
   }
   catch (ArgumentExcetion ex) {
     DoSmth(e);
   }
   catch (Exception ex) {
     DoSmth(e);
   }
}

Answer 3

If that's the only thing it does, i think is better to remove the try/catch's from those classes and let the exception be raised to the class that is responsible on handling them. That way you get only one log per exception giving you more clear logs and even you can log the stacktrace so you wont miss from where the exception was originated.

Answer 4

Sometimes you need to log data which is not available where the exception is handled. In that case, it is appropriate to log just to get that information out.

For example (Java pseudocode):

public void methodWithDynamicallyGeneratedSQL() throws SQLException {
    String sql = ...; // Generate some SQL
    try {
        ... // Try running the query
    }
    catch (SQLException ex) {
        // Don't bother to log the stack trace, that will
        // be printed when the exception is handled for real
        logger.error(ex.toString()+"For SQL: '"+sql+"'");
        throw ex;  // Handle the exception long after the SQL is gone
    }
}

This is similar to retroactive logging (my terminology), where you buffer a log of events but don't write them unless there's a trigger event, such as an exception being thrown.

Answer 5

you may want to lookup standard exception handling styles, but my understanding is this: handle exceptions at the level where you can add extra detail to the exception, or at the level where you will present the exception to the user.

in your example you are doing nothing but catching the exception, logging it, and throwing it again.. why not just catch it at the highest level with one try/catch instead of inside every method if all you are doing is logging it?

i would only handle it at that tier if you were going to add some useful information to the exception before throwing it again - wrap the exception in a new exception you create that has useful information beyond the low level exception text which usually means little to anyone without some context..

Answer 6

The general rule of thumb is that you only catch an exception if you can actually do something about it. So at the Business or Data layer, you would only catch the exception in situation's like this:

            try
            {
                this.Persist(trans);
            }
            catch(Exception ex)
            {
                trans.Rollback();
                throw ex;
            }

My Business/Data Layer attempts to save the data - if an exception is generated, any transactions are rolled back and the exception is sent to the UI layer.

At the UI layer, you can implement a common exception handler:

Application.ThreadException += new ThreadExceptionEventHandler(Application_ThreadException);

Which then handles all exceptions. It might log the exception and then display a user friendly response:

    static void Application_ThreadException(object sender, ThreadExceptionEventArgs e)
    {
        LogException(e.Exception);
    }
    static void LogException(Exception ex)
    {
        YYYExceptionHandling.HandleException(ex,
            YYYExceptionHandling.ExceptionPolicyType.YYY_Policy,
            YYYExceptionHandling.ExceptionPriority.Medium,
            "An error has occurred, please contact Administrator");
    } 

In the actual UI code, you can catch individual exception's if you are going to do something different - such as display a different friendly message or modify the screen, etc.

Also, just as a reminder, always try to handle errors - for example divide by 0 - rather than throw an exception.