3

I have an issue with the current project I am working on. The developers have added multiple catch blocks for a try block. Each catch is logging the error and then re-throwing it. Something like this:

catch (OdbcException ex)
{
    ex.Data.Add("CodeSource", MethodBase.GetCurrentMethod().Name);
    ex.Data.Add("My Value", myValue);
    Common.Logger.LogError(ex, "DataAccess");
    throw;
}
catch (Exception ex)
{
    ex.Data.Add("CodeSource", MethodBase.GetCurrentMethod().Name);
    ex.Data.Add("My Value", myValue);
    Common.Logger.LogError(ex, "DataAccess");
    throw;
}

On top of that, the calling class may also have catch blocks doing the samething. So the log file is a mess. I have seen up to 5+ of the same message logged.

Is there anyway I can log the error just once without having to change the code everywhere? The sad thing is the project is near the end so they don't want extra time spent on doing it right. ><

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Kenshin
  • 59
  • 3
  • 13

4 Answers4

1

The best practice is to not catch exceptions at every level. If you're only logging, then you only want to catch and log the exception "at the top".

What does "at the top" mean? That depends on the technologies you're using. A way to think of it is to catch the exception "at the last possible moment". Catch it at that point in your code where, if you don't catch it there, it won't be caught at all.

This may be in an event handler, or in an async callback, or at the top level of a web service.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
  • I completely agree with you. But the sad story is I am working with people that don't understand even the basics of software design let alone this. Also the project doesn't want to take the time. – Kenshin Jan 04 '13 at 15:23
  • I've been-there-done-that. It wastes an enormous amount of time during troubleshooting, since you'll have to ignore most of the log entries, but first have to find out which ones to ignore. – John Saunders Jan 04 '13 at 16:53
0

if you only want to log it when it happen, when it get thrown then maybe you just need to modify Common.Logger.LogError to add something like ex.Data.Add("ExceptionLogged", true) then you just need to loop through the innerexception to find out if one of them got this value, if it does, skip the saving

Fredou
  • 19,848
  • 10
  • 58
  • 113
0

You can log any exception using Global.asax's Application_Error event (but note these caveats):

protected void Application_Error(object sender, EventArgs e)
{
    LogExceptionDetails(Server.GetLastError());
    HttpContext.Current.Server.Transfer("~/Error.aspx");;
}

And if you implement this as an HttpModule then your exception logging can plugged into other applications or removed by simply altering the web.config file. See that link for code details.

Alternatively ELMAH is a really useful library for exception logging. After logging you can re-direct to a custom error page (see link for code details):

<customErrors mode="RemoteOnly" defaultRedirect="~/ErrorPages/CustomError.aspx">
    <error statusCode="404" redirect="~/ErrorPages/404.aspx" />
</customErrors>
Community
  • 1
  • 1
HTTP 410
  • 17,300
  • 12
  • 76
  • 127
0

The only thing I can think of is, if you control the Common.Logger class, to add some sort of logic in there to only log the first occurrence of an exception. You could try to track the exceptions using some thread storage or, since the tag mentions asp.net, you could use HttpContext.Items. E.g.:

public static class Logger
{
    public static void LogError(Exception e, string category)
    {            
        int hashcode = e.GetHashCode();
        if (!HttpContext.Current.Items.Contains(hashcode))
        {
            HttpContext.Current.Items.Add(hashcode, null); 
            System.Diagnostics.Trace.TraceError(e.ToString());
        }
    }
}

Perhaps that can help?

Randy Levy
  • 22,566
  • 4
  • 68
  • 94
  • Should HttpContext.Current.Items.Add(hashcode, null); be HttpContext.Current.Items.Add(hashcode, true);? Won't the if statement be true everytime? Also is the hash code the same if the exception is re-thrown? We are not throwning the exception variable but just doing throw; – Kenshin Jan 04 '13 at 15:16
  • @JasonHardesty, good point. :) I think Contains is probably a better fit. – Randy Levy Jan 04 '13 at 16:12