how to: handle exceptions, best practices

Question

need to implement a global error handling, so maybe you can help out with the following example...

i have this code:

public bool IsUserAuthorizedToSignIn(string userEMailAddress, string userPassword)
        {
            // get MD5 hash for use in the LINQ query
            string passwordSaltedHash = this.PasswordSaltedHash(userEMailAddress, userPassword);

            // check for email / password / validity
            using (UserManagementDataContext context = new UserManagementDataContext())
            {
                var users = from u in context.Users
                            where u.UserEMailAdresses.Any(e => e.EMailAddress == userEMailAddress)
                                && u.UserPasswords.Any(p => p.PasswordSaltedHash == passwordSaltedHash)
                                && u.IsActive == true
                            select u;

                // true if user found
                return (users.Count() == 1) ? true : false;
            }
        }

and the md5 as well:

private string PasswordSaltedHash(string userEMailAddress, string userPassword)
        {
            MD5 hasher = MD5.Create();
            byte[] data = hasher.ComputeHash(Encoding.Default.GetBytes(userPassword + userEMailAddress));

            StringBuilder stringBuilder = new StringBuilder();
            for (int i = 0; i < data.Length; i++)
            {
                stringBuilder.Append(data[i].ToString("x2"));
            }

            Trace.WriteLine(String.Empty);
            Trace.WriteLine("hash: " + stringBuilder.ToString());
            return stringBuilder.ToString();
        }

so, how would i go about handling exceptions from these functions? they first one is called from the Default.aspx page. the second one is only called from other functions from the class library.

what is the best practice?

• surround code INSIDE each function with try-catch
• surround the FUNCTION CALL with try-catch
• something else??

what to do if exceptions happen?

in this example: this is a user sign in, so somehow even if everything fails, the user should get some meaningful info - along the lines: sign in ok (just redirect), sign in not ok (wrong user name / password), sign in not possible due to internal problems, sorry (exception happened).

for the first function i am worried if there is a problem with database access. not sure if there is anything that needs to be handled in the second one.

thnx for the info. how would you do it? need specific info on this (easier for me to understand), but also general info on how to handle other tasks/functions.

i looked around the internet but everyone has different things to say, so unsure what to do... will go with either most votes here, or most logicaly explained answer :) thank you.

Answer 1

Your library code or the code that is used by higher-layers in your application must always only throw exceptions and never worry about how to deal with them.

This is important because you may use this library at many places for different purposes.

In your application presentation layer, if you're consuming a library code and you're aware about the possible exceptions then do catch them with try/catch.

Since you're using asp.net I'd recommend to write a common page-base class and work some error handling mechanism in Page_Error event that catches all un-handled exceptions on the page.

Even beyond that you can use Application_Error even in global.asax to catch any un-handled exception in any part of the application, Modules, Handler, services, pages etc.

I'd strongly recommend to not make it a general practice to handle all exception in global Application_Error.

Answer 2

You need to think about the contracts that the methods define. If a method can't fulfill its contract then you need an exception. Exceptions thrown within the method should be translated into exceptions that make sense from the point of view of the contract of the method.

In the first case, I would catch any exceptions that can be thrown by statement execution (e.g. database errors) and translate them into something like an AuthorisationException. Allowing the exception to propagate straight up doesn't make sense from the point of view of the methods contract. For instance it doesn't make sense to allow a database exception to escape from the method when the method is fulfilling the contract of whether the author is authorised.

Using this idea of the method as a contract allows you to make the right decisions. If a method contract is logging into the database, or some database manipulation, then it might well make sense to allow the exceptions to propagate.

Getting specific: Using the first example:

public bool IsUserAuthorizedToSignIn(string userEMailAddress, string userPassword)
    {
        try
        {
            // get MD5 hash for use in the LINQ query
            string passwordSaltedHash = this.PasswordSaltedHash(userEMailAddress, userPassword);

            // check for email / password / validity
            using (UserManagementDataContext context = new UserManagementDataContext())
            {
                var users = from u in context.Users
                        where u.UserEMailAdresses.Any(e => e.EMailAddress == userEMailAddress)
                            && u.UserPasswords.Any(p => p.PasswordSaltedHash == passwordSaltedHash)
                            && u.IsActive == true
                        select u;

                // true if user found
                return (users.Count() == 1) ? true : false;
            }
        }
        catch(ThisException e)
        {
            thrown new AuthorisationException("Problem1 occurred");
        }
        catch(ThatException e)
        {
            thrown new AuthorisationException("Problem2 occurred");
        }
        catch(OtherException e)
        {
            thrown new AuthorisationException("Problem3 occurred");
        }
    }

You might also like to set the inner exception on the AuthorisationException:

        catch(ThisException e)
        {
            thrown new AuthorisationException("Problem1 occurred", e);
        }

Then your client code can do this:

try
{
    if(User.IsUserAuthorizedToSignIn)
    {
        // Let the magic happen
    }
    else
    {
        // No rights
    }
}
catch(AuthorisationException e)
{
    // Let the user know there is something up with the system. 
}

You may decide that you don't want to catch the AuthorisationException at the immediate calling site since you can't notify the user. So you may let it propagate up to a level where you can do something with it.

Hope that helps!

Answer 3

Two golden rules:

1. Throw an exception if a method can't do what its name says it does.
2. Don't catch exceptions unless you know exactly what to do with them.

Remember that an exception indicates that something has gone wrong, and that particular something may not be what you think. (Out of memory, stack overflow, 3rd party service gone down, botched deployment resulting in missing assemblies, mis-configuration, security exceptions etc).

With very few exceptions, the only place you should see Pokemon exception handling is at the topmost level of your code, where the exception should be published somewhere. For example, in the Application_Error method in your global.asax file.

Here are some links for you that you may find helpful:

• If Your Method Can't Do What Its Name Promises It Can, Throw
• The two golden rules of exception handling
• Exception-Driven Development
• ELMAH - Error Logging Modules and Handlers for ASP.NET