Who should log an error/exception

Question

I am trying to figure out the best practices when loggin exceptions.

So far, I am logging every time I catch an exception. But when a lower lever class catches an exception (say, from the database layer), and wraps it in our own application exception - should I also log the original exception there, or should I just let the upper lever class log all the details?
And what about locations where my lower level class decides to throw an exception because of bad input parameters? Should it log the exception there also, or, once again, just let the catching code log it?

Answer 1

log where you catch, If you're wrapping then you should. If the lower wrapper doesn't then you have a reason (for debugability) to do so. However don't swallow the exception unless you know its benign or you can handle it.

I'd suggest

try{
.
.
.
} catch(Exception ex){
 ... log ....
 throw;
}

if you need to log and pass the exception on.

Answer 2

Mainly you should avoid logging it in both a lower-level catch and a higher-level catch, as this bloats the log with redundant information (not to mention takes up additional IO resources to write to the log).

If you are looking for general best practice information on exception handling, this link is handy.

Answer 3

You can get away with logging only once at the very top level of your app, as long as your logging code (a) logs the stack trace of an exception, and (b) logs the entire chain of inner exceptions as well.

The Microsoft Exception Handling Application Block takes care of both of those things for you. I guess other logging frameworks would do the same.

Answer 4

In my winform's applications i created some Observer for logging. Observer has subscribers, which can write log somewhere, or process it. It's look:

    public static class LoggingObserver
    {
        /// <summary>
        /// Last getted log message
        /// </summary>
        public static string LastLog;

        /// <summary>
        /// Last getted exception
        /// </summary>
        public static Exception LastException;

        /// <summary>
        /// List of log's processors
        /// </summary>
        public static List<BaseLogging> loggings = new List<BaseLogging>();

        /// <summary>
        /// Get Exception and send for log's processors
        /// </summary>
        /// <param name="ex">Exception with message</param>
        public static void AddLogs(Exception ex)
        {
            LastException = ex;
            LastLog = string.Empty;
            foreach (BaseLogging logs in loggings)
            {
                logs.AddLogs(ex);
            }
        }

        /// <summary>
        /// Get message log for log's processors
        /// </summary>
        /// <param name="str">Message log</param>
        public static void AddLogs(string str)
        {
            LastException = null;
            LastLog = str;
            foreach (BaseLogging logs in loggings)
            {
                logs.AddLogs(str);
            }
        }

        /// <summary>
        /// Close all processors
        /// </summary>
        public static void Close()
        {
            foreach (BaseLogging logs in loggings)
            {
                logs.Close();
            }
        }
    }

Subscriber's abstract class:

public abstract class BaseLogging
    {
        /// <summary>
        /// Culture (using for date) 
        /// </summary>
        public CultureInfo culture;

        /// <summary>
        /// Constructor
        /// </summary>
        /// <param name="culture">Culture</param>
        public BaseLogging(CultureInfo culture)
        {
            this.culture = culture;
        }

        /// <summary>
        /// Add log in log system
        /// </summary>
        /// <param name="str">message of log</param>
        public virtual void AddLogs(string str)
        {
            DateTime dt = DateTime.Now;

            string dts = Convert.ToString(dt, culture.DateTimeFormat);

            WriteLine(String.Format("{0} : {1}", dts, str));
        }

        /// <summary>
        /// Add log in log system
        /// </summary>
        /// <param name="ex">Exception</param>
        public virtual void AddLogs(Exception ex)
        {
            DateTime dt = DateTime.Now;

            string dts = Convert.ToString(dt, culture.DateTimeFormat);
            WriteException(ex);
        }

        /// <summary>
        /// Write string on log system processor 
        /// </summary>
        /// <param name="str">logs message</param>
        protected abstract void WriteLine(string str);

        /// <summary>
        /// Write string on log system processor 
        /// </summary>
        /// <param name="ex">Exception</param>
        protected abstract void WriteException(Exception ex);

        /// <summary>
        /// Close log system (file, stream, etc...)
        /// </summary>
        public abstract void Close();
    }

And implementation for logging to file:

/// <summary>
    /// Logger processor, which write log to some stream
    /// </summary>
    public class LoggingStream : BaseLogging
    {
        private Stream stream;

        /// <summary>
        /// Constructor.
        /// </summary>
        /// <param name="stream">Initialized stream</param>
        /// <param name="culture">Culture of log system</param>
        public LoggingStream (Stream stream, CultureInfo culture)
            : base(culture)
        {
            this.stream = stream;
        }

        /// <summary>
        /// Write message log to stream
        /// </summary>
        /// <param name="str">Message log</param>
        protected override void WriteLine(string str)
        {
            try
            {
                byte[] bytes;

                bytes = Encoding.ASCII.GetBytes(str + "\n");
                stream.Write(bytes, 0, bytes.Length);
                stream.Flush();
            }
            catch { }
        }

        /// <summary>
        /// Write Exception to stream
        /// </summary>
        /// <param name="ex">Log's Exception</param>
        protected override void WriteException(Exception ex)
        {
            DateTime dt = DateTime.Now;

            string dts = Convert.ToString(dt, culture.DateTimeFormat);
            string message = String.Format("{0} : Exception : {1}", dts, ex.Message);
            if (ex.InnerException != null)
            {
                message = "Error : " + AddInnerEx(ex.InnerException, message);
            }
            WriteLine(message);
        }
        /// <summary>
        /// Closing stream
        /// </summary>
        public override void Close()
        {
            stream.Close();
        }

        private string AddInnerEx(Exception exception, string message)
        {
            message += "\nInner Exception : " + exception.Message;
            if (exception.InnerException != null)
            {
                message = AddInnerEx(exception.InnerException, message);
            }
            return message;
        }
    }

Using:

//initialization 
FileStream FS = new FileStream(LogFilePath, FileMode.Create);
LoggingObserver.loggings.Add(new LoggingStream(FS, Thread.CurrentThread.CurrentCulture));
//write exception 
catch (Exception ex) {
LoggingObserver.AddLog(new Exception ("Exception message", ex));
}
//write log 
LoggingObserver.AddLog("Just a log");