Is it good practice to catch exception, log and throw the exception again?

Question

For the purpose of logging, I would like to catch an exception. But since I would also need to throw the exception, I would then throw the exception again. For example, I would do this in C# as below:

try
{
    //exception-prone code block here
}
catch(Exception ex)
{
   Logger.Log(ex);
   throw new Exception(ex);
}

Now I am curious is this the best way for the purpose of logging? Suggestions please.

Answer 1

There is no best way of logging. It always depends on what you need.

But if you want to reduce code and just log the error, you could create your own event handler and attach it to a specific event.

Take the for example the AppDomain.UnhandledException event:

Demo code from MSDN:

public class Example 
{
   [SecurityPermission(SecurityAction.Demand, Flags=SecurityPermissionFlag.ControlAppDomain)]
   public static void Main()
   {
      AppDomain currentDomain = AppDomain.CurrentDomain;
      currentDomain.UnhandledException += new UnhandledExceptionEventHandler(MyHandler);

      try {
         throw new Exception("1");
      } catch (Exception e) {
         Console.WriteLine("Catch clause caught : {0} \n", e.Message);
      }

          throw new Exception("2");
       }

       static void MyHandler(object sender, UnhandledExceptionEventArgs args) 
       {
          Exception e = (Exception) args.ExceptionObject;
          Console.WriteLine("MyHandler caught : " + e.Message);
          Console.WriteLine("Runtime terminating: {0}", args.IsTerminating);
       }
    }
}

You could put your loggin in the MyHandler method and be done with it. You wouldn't need to abuse a catch block without really dealing with the exception.

In Asp.Net you could override the Application_Error method in the global.asax:

protected void Application_Error()
{
    Exception ex = Server.GetLastError();
    NLog.Logger logger = NLog.LogManager.GetCurrentClassLogger();
    logger.Fatal(ex);
}

Based on your logging routine, anything could be done now. In my above case, every fatal exception, gets send via email to an incident management system.

In my oppinion the keypoint is, that a try / catch is supposed to handle exceptions. Logging an error in a catch block is like having a no statements in a catch block at all, you are just swallowing the exception, without handling it. In the end it's just redundant code.

Answer 2

This answer is more about best Practice for Error Handling than best practice for logging exceptions

Exception handling is an expensive process that deliberately breaks the correct flow of your program, so should not be used as your default checking mechanism, that should always be validation checks.for example if you want to create a file, check if the file exists before creating it, this then allows the normal flow of the program to handle expected situations.

exception handling should only be used as the name suggests in exceptional circumstances. either when the exception is something not expected or its impossible for you to decide how to handle the exception with out further information. for example in the above the MS framework gives you a function to check if a file exists, but if the file creation is called and the file is already there what should it do? Append to the Existing file? delete it and create a new one?? try again with a different filename? Microsoft can't know what is the correct way to handle this so have to raise an exception to let you know that you did something you shouldn't have. as such this is the only time you should raise an exception in your code.

if you're a handling an exception then that is exactly what you should do, handle it you should never need to throw an exception inside an exception handler unless you are developing class libraries in which case is isn't appropriate for a library to communicate with the user, in which case the exceptions should be left unhandled or rethrown with additional information. if you can't handle an exception at the point you are in the code then your exception handler is in the wrong place.

an exception handler should do 1 of 3 things

1. perform a predefine action that corrects an error ie wait 5 seconds and try again
2. ask the user which predefine action is the correct way to solve the error ie Retry or Cancel
3. report the error to the user and return the program to a stable state ie Could not save file at this time because blah blah, try again later

any one of these action can include logging of the exception, but all of them should be final

try
{
    //exception-prone code block here
}
catch(KnowException1 ex)
{
   Logger.Log(ex); //logging may be optional as its a known handled situation
   //perform predefined action
}
catch(KnowException2 ex)
{
   Logger.Log(ex); //logging may be optional as its a known handled situation
   // fire message box asking users how to proceed
   //perform selected predefined action
}
catch(UnknowException ex)
{
   Logger.Log(ex); //logging is vital
   // fire message box something unexpected happened
   //cancel action and return to normal operations
}

this is why its always advisable to raise an appropriately typed exception