Exception logging and sending email

Question

Is the following method of exception handling correct? I am finding it difficult to log the error and finally send the log file through email. Where do I write code to log the error and send email?

A major problem I get is that when the exception is generated in SomeClass1 it is logged two times - the second time from SomeClass2. Is it a good idea to create a single custom exception type (SomeException in the example) and wrap the System.Exception whenever it occurs?

Also I am confused about how and when to show error message to the end user when we have a chain of try-catches calling one another.

class SomeClass1
{
    public static DataExtract(string sourcepath)
    {
        try
        {
            OleDbConnection oledb = new OleDbConnection();
            oledb.ConnectionString = "someconnectionstring";
            CompanyOLEDB.Open();
        }
        catch (Exception e)
        {
            throw new CustomException(e);
        }
    }
}

class SomeClass2
{
    private void SomeMethod()
    {
        try
        {
            // some code
            // some code

            SomeClass1.DataExtract()
        }
        catch (Exception e)
        {
            throw new CustomException(e);
        }
    }
}

public class CustomException : Exception
{
    protected CustomException() { }

    public CustomException(Exception e)
    {
        Log(e);
    }

    public CustomException(ExceptionType type)
    {
        this.Data.Add("Type", type);
        this.Data.Add("Message", "No message specified");
    }

    public CustomException(ExceptionType type, string message)
    {
        this.Data.Add("Type", type);
        this.Data.Add("Message", message);
    }

    public static void Log(Exception e)
    {
        System.IO.File.WriteAllText(Logfile.txt", e.ToString());
    }

    public static void Sendmail()
    {
        ExceptionMail.Sendmail();
    }
}

No. See http://msdn.microsoft.com/en-us/library/ms229014.aspx — John Saunders, Mar 16 '12 at 07:10
possible duplicate of [Best Practice for Exception Handling in a Windows Forms Application?](http://stackoverflow.com/questions/183589/best-practice-for-exception-handling-in-a-windows-forms-application) — John Saunders, Mar 16 '12 at 07:11

score 5 · Accepted Answer · answered Mar 16 '12 at 07:14

5

Is the following method of exception handling correct?

No. There are several issues. Here is the two most important ones.

1. You should not catch exceptions that you can't handle

Quite important. All exception blocks that just rethrow or log exceptions clutter the code without adding any value. Only catch all exceptions in the topmost layer (in the webservice, the MVC controller, in a background thread etc) to prevent the application from crashing. (however, it's sometimes better to let the application crash).

An exception have been handled if the method can return the expected value.

2. Always include the original exception

You are hiding information that is important to be able to prevent the information in the future when you only copy partial information from the original exception.

If you must catch/throw other you should do it like this:

public class CustomException : Exception
{
    public CustomException(string msg, Exception inner) : base(msg, inner){}
}

// and in a method:

public void DoSomething()
{
    try
    {
        SomeCoolOp();
    }
    catch (Exception err)
    {
        throw new CustomException("Tried to allocate some cool stuff", err);
    }
}

Changes compared to your code:

We include the original exception in the way that Microsoft recommends
We write a operation specific message, i.e. describing what we tried to do when the exception happened (instead of using the original message)

More information

I've written several blog posts about exception handling.

You can start by reading: http://blog.gauffin.org/2010/11/do-not-catch-that-exception/

And than read the rest: http://blog.gauffin.org/tag/exceptions/

answered Mar 16 '12 at 07:14

jgauffin

99,844
45
235
372

If I am using tier architecture like MVP pattern does topmost layer mean presentation.Are you suggesting that exceptions are better catch in this layer. – arjun Mar 16 '12 at 07:24
1

Yes. Only catch exceptions that you can handle inside the library. Catch every other exception inside the presentation layer. You can use `Application.ThreadException` to catch all unhandled exception (and then email them) – jgauffin Mar 16 '12 at 07:30
when we throw exception like -- throw new CustomException("Tried to allocate some cool stuff", err)--, it halt the normal execution of program. Does this mean we are not able to handle the exception. – arjun Mar 16 '12 at 08:46
It's not clear to me what you are asking. All exceptions prevents the application from continuing on with the normal application execution. That's the purpose of exceptions. However, you can prevent the application from crashing by catching all exceptions in the topmost layer. – jgauffin Mar 16 '12 at 08:50
what about logging error?when error are thrown from lower to top layer should i log error at lower layer or top layer? – arjun Mar 16 '12 at 08:56
Top layer since you should not catch exceptions in the lower layers that you can't handle. – jgauffin Mar 16 '12 at 08:57
Can you please provide link to sample project about exception handling in enterprise level architecture( like the one with mvp or mvc) or perhaps some good books in c# – arjun Mar 16 '12 at 09:11
1

Read my articles. Feel free to leave comments in them if there is something that is unclear. There is no difference between enterprise level exception handling and exception handling in a tiny application. – jgauffin Mar 16 '12 at 09:44

score 2 · Answer 2 · answered Mar 16 '12 at 07:12

2

perhaps consider using something like ELMAH much like Log4Net it can be configured such that it logs errors to files, db tables, or email.

answered Mar 16 '12 at 07:12

Al W

7,539
1
19
40

Exception logging and sending email

2 Answers2