A TraceListener, which throws an exception on a failed assertion

Question

My problem with DefaultTraceListener (which is the only trace listener, if not overridden in app.config) is that, if AssertUiEnabled is false (i.e. in ASP.NET) it writes a message as OutputDebugString on failed assertions (calls to Trace.Assert(false)), but continues the execution.

So I'm using the following sub-class of TraceListener, which instead throws an exception. I activate it using

TraceListenerWhichThrowsExceptionOnFail.InsertAsFirstTraceListener(Trace.Listeners);

in Application_Init.

Now, in the Application_Error event I can log the exception (as any exception) with the full stack trace, including the call of Trace.Assert(false).

public class TraceListenerWhichThrowsExceptionOnFail : TraceListener
{
    public static void InsertAsFirstTraceListener(TraceListenerCollection traceListeners)
    {
        traceListeners.Insert(0, new TraceListenerWhichThrowsExceptionOnFail());
    }

    public override void Fail(string message, string detailMessage)
    {
        message = string.IsNullOrEmpty(detailMessage) ? message : message + ", Detail message: " + detailMessage;

        throw new ApplicationException("Trace assertion failed" + 
            (string.IsNullOrEmpty(message) ? "" : ": " + message) + ".");
    }

     public override void Write(string message)
     {
        // NOP
     }

     public override void WriteLine(string message)
     {
        // NOP
     }
}

Now my question: Does anyone see a problem with this approach?

Answer 1

I'd say exception in Trace.Assert is bad idea (not looking at your code).

Trace.XXXX methods generally used to trace stuff. It would be very surprising to future readers (including you) to learn that Trace.Assert actually throws exception.

It is even more surprising to see Trace.Assert (or even Fail) throwing exception. Purpose of Assert is to help identify issues in advance, not to kill application.

I'd recommend to come up with your custom method that clearly shows its behavior in the name instead of coming up with unexpected behavior of existing methods.

Answer 2

It depends.

I'm not sure this would be useful in an ASP.NET scenario. During debugging, you want assertions to behave normally. Throwing exceptions would probably be a bit confusing. And in production, you would use release builds, so that the Trace.Assert statements don't do anything anyway.

However, we use a similar technique for unit tests. You really don't want to have your build server hanging showing an "Assertion Failed" message box. Instead, you want your assertions to cause the tests to fail. So we use an App.config file for our unit tests (and only for hose!), that installs a trace listener that is similar to yours.

Answer 3

Throwing an exception in Fail() could lead to: 1. the call stack will be confusing, since the exception is not thrown near where the condition failed.

Generally you should throw exception near where the condition fails.