views:

2339

answers:

6

Using .NET 3.5, ASP.NET, Enterprise Library 4.1 Exception Handling and Logging blocks, I wrote a custom exception handler to display a standard error page, as follows:

[ConfigurationElementType(typeof(CustomHandlerData))]
public class PageExceptionHandler : IExceptionHandler {

    public PageExceptionHandler(NameValueCollection ignore) {
    }

    public Exception HandleException(Exception ex, Guid handlingInstanceID) {

        HttpResponse response = HttpContext.Current.Response;
        response.Clear();
        response.ContentEncoding = Encoding.UTF8;
        response.ContentType = "text/html";
        response.Write(BuildErrorPage(ex, handlingInstanceID));
        response.Flush();
        //response.End(); // SOMETIMES DOES NOT WORK

        return ex;
    }
}

This is called from an exception handling policy like this:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <configSections>
    <section name="exceptionHandling" type="Microsoft.Practices.EnterpriseLibrary.ExceptionHandling.Configuration.ExceptionHandlingSettings, Microsoft.Practices.EnterpriseLibrary.ExceptionHandling, Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" />
  </configSections>
  <exceptionHandling>
    <exceptionPolicies>
      <add name="Top Level">
        <exceptionTypes>
          <add type="System.Exception, mscorlib, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"
            postHandlingAction="None" name="Exception">
            <exceptionHandlers>
              <add logCategory="General" eventId="0" severity="Error" title="Application Error"
                formatterType="Microsoft.Practices.EnterpriseLibrary.ExceptionHandling.TextExceptionFormatter, Microsoft.Practices.EnterpriseLibrary.ExceptionHandling, Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35"
                priority="0" useDefaultLogger="false" type="Microsoft.Practices.EnterpriseLibrary.ExceptionHandling.Logging.LoggingExceptionHandler, Microsoft.Practices.EnterpriseLibrary.ExceptionHandling.Logging, Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35"
                name="Log Full Details" />
              <add type="PageExceptionHandler, Test, Culture=neutral, PublicKeyToken=null"
                name="Display Error Page" />
            </exceptionHandlers>
          </add>
        </exceptionTypes>
      </add>
    </exceptionPolicies>
  </exceptionHandling>
</configuration>

It all works fine, except that the error page was being incorporated into whatever markup was displayed at the time the error occurred. I thought that adding response.End() into the exception handler would solve this problem. It did, but I then observed that, sometimes, ASP.NET throws a ThreadAbortException while trying to end the response stream. This is the only exception type that cannot be caught and ignored. Grrr! Can anyone tell me:

  1. Under what conditions ASP.NET would throw a ThreadAbortException when trying to end the response stream?
  2. Is there a safer alternative to response.End() that I could use?
  3. If not, is there a way of detecting whether the response stream is "end-able" before calling response.End()?

EDIT - More Context

The design goal for this code is to make it as simple as possible to add EHAB-style error handling to a web app. I have a custom IHttpModule that must be added to the web.config file. In the module's Application_Error event it calls ExceptionPolicy.HandleException. The exception handling policy must be configured to call the PageExceptionHandler class described above. This whole procedure involves only a small amount of XML config, no extra files, and no extra lines of code in the consuming web app.

In fact, the code as it stands seems to work fine. However, in some situations it is desirable to have an explicit catch block in the web app code that calls the exception handling policy directly. This sort of scenario is what does not work properly. I would like a single solution that works under all possible methods of calling it. I do get the impression that it would be a lot simpler if the EHAB was not involved, but unfortunately it is used throughout all our code, and provides so many other benefits that I would prefer to keep it in.

EDIT - Testing Results

I created a test harness to exercise the code in six different ways:

  1. Writing to the current page's HttpResponse directly.
  2. Constructing an exception object and calling ExceptionPolicy.HandleException(ex, "Top Level") directly.
  3. Throwing an exception object, catching it, and calling ExceptionPolicy.HandleException(ex, "Top Level") in the catch block.
  4. Throwing an exception object and letting the Page_Error event call ExceptionPolicy.HandleException(ex, "Top Level").
  5. Throwing an exception object and letting the Application_Error event call ExceptionPolicy.HandleException(ex, "Top Level").
  6. Throwing an exception object and letting my custom IHttpModule class call ExceptionPolicy.HandleException(ex, "Top Level").

Test results for each method with no code to terminate the response after writing the error page markup:

  • Methods 1, 2, 3 - Error page markup combined with the test page markup.
  • Methods 4, 5, 6 - Error page markup replaced the test page markup (desired result).

Test results for each method when HttpContext.Current.ApplicationInstance.CompleteRequest was called:

  • Methods 1, 2, 3 - Error page markup combined with the test page markup.
  • Methods 4, 5, 6 - Error page markup replaced the test page markup (desired result).

Test results for each method when HttpContext.Current.Response.End was called:

  • Methods 1, 5, 6 - Error page markup replaced the test page markup (desired result).
  • Methods 2, 3, 4 - Error page markup replaced the test page markup, but the EHAB throws a ExceptionHandlingException twice.

Test results for each method when HttpContext.Current.Response.End was called, but wrapped in a try ... catch block:

  • Methods 5, 6 - Error page markup replaced the test page markup (desired result).
  • Methods 1, 3, 4 - Error page markup replaced the test page markup, but ASP.NET throws a ThreadAbortException which is caught and absorbed by the catch block.
  • Method 2 - Error page markup replaced the test page markup, but I get a ThreadAbortException and also two ExceptionHandlingExceptions.

It's a ridiculous set of behaviours. :-(

A: 

Have a look at this Microsoft explanation. It says it applies to ASP.Net 1.1, but I think it still applies in 2.0. Basically you should call the HttpContext.Current.ApplicationInstance.CompleteRequest method instead.

Also, I'm not sure why you can't catch the ThreadAbortException - why can't you do something like

try {
    // code
} catch (ThreadAbortException)
{
    //do nothing
}
Graham Clark
I've just tried HttpContext.Current.ApplicationInstance.CompleteRequest, and it has no effect whatsoever (i.e. the error page markup is still mixed in with the original page). I've also tried catching the exception, and it seems to be ignoring my catch block. The only thing that works so far is calling Response.End in the calling code catch block after calling ExceptionPolicy.HandleException. That's not very practical since I can't change every catch block in the app.
Christian Hayter
After a bit more debugging I can say:* Response.End definitely throws a ThreadAbortException and it is caught, but when I try to view its properties in the debugger I get "Unable to evaluate expression because the code is optimized or a native frame is on top of the call stack" for every property.* The ExceptionPolicy.HandleException call then throws an ExceptionHandlingException, but all its properties are unavailable for the same reason.
Christian Hayter
I googled that error and all the search results basically say "use CompleteRequest instead". That's not very helpful since it doesn't work. :-)
Christian Hayter
A: 

My approach to handling ASP.net exceptions was always to catch it, log it and then redirect to a generic error page with some standard error message (the end user won't care about exceptions or stack traces). You already logged the exception in your policy before your handler so maybe all you have to do is a response redirect to an error page and move the BuildErrorPage logic there. You can pass the handlingInstanceId in the querystring.

Ariel Popovsky
Thanks for the suggestion. I'm looking into it, but due to my workload it may be a while before I know for sure.
Christian Hayter
+1  A: 

Have a look at ELMAH.
There are lots of articles and how-to's on ELMAH, just google or bing for it :-)

Advantages
+ It logs almost everything
+ Email notification
+ Remote viewing of errors

I would suggest to use that and redirect to a default error page, as Ariel mentioned.

Peter Gfader
A: 

Try using Application_Error in the Global.asax to capture any unhandle exceptions thrown by any page in your application.

JNappi
Thanks for the suggestion. I added that method to my test harness (method 5), but it gave identical results in all cases with my original implementation (method 6). Methods 5 and 6 were not the problem - it was methods 1, 2 and 3 that I was primarily concerned about.
Christian Hayter
A: 

This may help: http://msdn.microsoft.com/en-us/library/cyayh29d.aspx

Specifically "Your clean-up code must be in the catch clause or the finally clause, because a ThreadAbortException is rethrown by the system at the end of the finally clause, or at the end of the catch clause if there is no finally clause.

You can prevent the system from rethrowing the exception by calling the Thread..::.ResetAbort method. However, you should do this only if your own code caused the ThreadAbortException. "

Note that Response.End and Response.Redirect may throw a ThreadAbortException under certain conditions.

Marcel Marchon
Thanks for the suggestion. try { response.End(); } catch (ThreadAbortException) { Thread.ResetAbort(); } does indeed successfully absorb all ThreadAbortExceptions. However, I observed that when you absorb the ThreadAbortException, you also "absorb" the response ending effect, so the result is identical in every way to missing out the Response.End call altogether.
Christian Hayter
A: 

After a lot of testing, I have found only two approaches that work without error.

Solution 1

No change to the current architecture at all. The global exception handler already works as desired. If an explicit catch block is needed in code for any reason (e.g. to render validation errors on an input form), then I will tell developers to call Response.End explicitly, e.g.

try {
    // ...
} catch(Exception ex) {
    if(ExceptionPolicy.HandleException(ex, "Top Level")) {
        throw;
    }
    Response.End();
}

Solution 2

Give up trying to overwrite the current response stream and do a genuine redirect to an error page. This works fine with all six test harness methods. The next problem was finding a way of doing this that required the minimum possible changes to the consuming project and abstracted away all the dirty details.

Step 1 - Add a placeholder ErrorPage.ashx file to the project which contains only a reference to a standard web page handler class and no code-behind.

<%@ WebHandler Class="WebApplicationErrorPage" %>

Step 2 - Redirect to this page in the PageExceptionHandler class, using a session key to pass all required data.

public Exception HandleException(Exception ex, Guid handlingInstanceID) {

    HttpResponse response = HttpContext.Current.Response;
    HttpSessionState session = HttpContext.Current.Session;

    session["ErrorPageText"] = BuildErrorPage(ex, handlingInstanceID);
    response.Redirect(errorPageUrl, false);

    return ex;
}

Step 3 - Make the WebApplicationErrorPage class a really dumb HTTP handler that just writes to the response stream.

public class WebApplicationErrorPage : IHttpHandler, IReadOnlySessionState {

    public bool IsReusable {
        get {
            return true;
        }
    }

    public void ProcessRequest(HttpContext context) {

        response.Clear();
        response.ContentEncoding = Encoding.UTF8;
        response.ContentType = "text/html";
        response.Write((string) context.Session["ErrorPageText"]);
        response.Flush();
    }
}

Conclusion

I think I'll stick with solution 1 because it doesn't involve chopping and changing the existing architecture (implementing solution 2 for real will be a lot more complex than the code snippets above suggest). I'll file away solution 2 for possible use elsewhere.

Christian Hayter