Using Enterprise Library Exception Handling Application Block in ASP.NET - Code Review

Question

I'm implementing the Enterprise Library Exception Handling Application Block in an ASP.NET application that I am building. I'm planning to handle uncaught application exceptions by placing the following code in my Global.asax.cs:

protected void Application_Error()
    {
        Exception error = Server.GetLastError();
        Exception errorToThrow;

        if (ExceptionPolicy.HandleException(error, "Application Error", out errorToThrow))
        {
            if (errorToThrow != null)
                throw errorToThrow;
        }
        else
            Server.ClearError();
    }

I believe this will work to handle various post-handling actions of the policy (None, NotifyRethrow, ThrowNewException) but I want to know if anyone sees significant problems with this implementation.

Answer 1

Looks fine, but you might want to redirect to an error page instead of rethrow (it's user friendlier):

Exception wrappedEx;
ExceptionHandlerHelper.ExceptionHandler.HandleException(ex, "MyPolicy", out wrappedEx);
Response.Redirect(string.Format("ErrorPage.aspx?Message={0}", Uri.EscapeDataString((wrappedEx ?? ex).Message)));

Answer 2

I see a few problems:

• You will probably want to handle HttpUnhandledException in your error handler. This is what most of the exceptions raised by your page will be.

• I don't see the value of a handle and resume (PostHandlingAction = None) policy where you call Server.ClearError(). Basically your page has thrown an exception and you are doing nothing. Best case scenario, the user sees an empty page. Worst case, you could have a partially rendered page with no indication what happened.

• I also don't see the point of potentially throwing an exception from your error handler. You're either going to end up with the yellow screen of death or your going to force another error page to be called (e.g. a customErrors redirect defined in web.config).

• Your HandleException logic does not account for the NotifyRethrow scenario. (Removed based on comment that there is a customError redirect).

Using vladhorby's ErrorPage and keeping your main logic would give something like this:

protected void Application_Error()
{
    Exception error = Server.GetLastError();

    if (error is HttpUnhandledException)
    {
        error = error.InnerException;
    }

    Exception errorToThrow;

    if (ExceptionPolicy.HandleException(error, "Application Error", out errorToThrow))
    {
        Response.Redirect(string.Format("ErrorPage.aspx?Message={0}", Uri.EscapeDataString((errorToThrow ?? error).Message)));   
    }
    else
    {
        Server.ClearError();
    }
}

A few notes about the above code:

• If ErrorPage.aspx throws an Exception for some reason you could end up in an infinite loop so make sure that ErrorPage.aspx has it's own Page_Error to try to avoid unhandled exceptions propagating to the global error handler.

• It does not take into account objections 2 and 3. :)

I'm not sure about your situation and requirements but have you considered using ELMAH?

Answer 3

For those who are interested, I updated my code based on some problems I was having. Specifically, I wanted users to see an error page but not have their URL change, so I couldn't rely on the web.config custom error handling. I found that this method gives me some more control over error handling.

if (this.Context.IsCustomErrorEnabled)
{
    Exception originalError = Server.GetLastError();
    Exception replacedError;

    if (ExceptionPolicy.HandleException(originalError, "Application Error", out replacedError))
    {
        if (replacedError != null)
            this.Context.Items[ErrorController.ExceptionKey] = replacedError;
        else
            this.Context.Items[ErrorController.ExceptionKey] = originalError;

        Server.ClearError();

        // Perform an MVC "Server.Transfer" to the error page.
        this.Context.RewritePath(DefaultRedirectUrl);

        IHttpHandler handler = new MvcHttpHandler();
        handler.ProcessRequest(this.Context);
    }
}

Some notes about this implementation:

1. "DefaultRedirectUrl" is a property that pulls the default error redirect URL from the web.config file, which seemed like the most logical place to store that information.
2. I am using the MVC "Server.Transfer" methodology from this question on SO.
3. The default redirect is to an action on the ErrorController, which retrieves the error from the HttpContext and processes it appropriately.