Using events rather than exceptions to implement error handling

Question

I'm working on some code that uses a pattern in its business and data tiers that uses events to signal errors e.g.

resource = AllocateLotsOfMemory();
if (SomeCondition())
{    
    OnOddError(new OddErrorEventArgs(resource.StatusProperty));
    resource.FreeLotsOfMemory();    
    return;
}

This looked superficially rather odd, especially as the code that calls this needs to hook into the events (there are four or five different ones!).

The developer tells me that this way they can refer to the properties of the allocated resource in the error handling code, and that responsibility for cleaning up after the error is kept by this tier.

Which makes some kind of sense.

The alternative might be something like

resource = AllocateLotsOfMemory();
if (SomeCondition())
{   
    BigObject temporary = resource.StatusProperty;
    resource.FreeLotsOfMemory();
    throw new OddException(temporary);
}

My questions are:

1. As this "BigObject" is freed when the exception object is released, do we need this pattern?

2. Has anyone else experience of this pattern? If so, what pitfalls did you find? What advantages are there?

Thanks!

Answer 1

If you think in terms of "Errors" and "Warnings", I've had lots of luck when reserving events for the "Warning" category and Exceptions for the "Errors" category.

The rationale here is that events are optional. No one is holding a gun to your head forcing you to handle them. That's probably okay for warnings, but when you have genuine errors you want to make sure they are taken a little more seriously. Exceptions must be handled, or they'll bubble up and create a nasty message for the user.

With regards to your Big Object question: you definitely don't be passing big objects around, but that doesn't mean you can't pass references to big objects around. There's a lot of power in the ability to do that.

As an addendum, there's nothing stopping from from raising an event in addition to the exception, but again: if you have a genuine error you want something to force the client developer to handle it.

Answer 2

1) is it needed? no pattern is absolutely necessary

2) Windows Workflow Foundation does this with all the results from the Workflow Instances running inside the hosted runtime. Just remember that exceptions can happen when trying to raise that event, and you might want to do your cleanup code on a Dispose or a finally block depending on the situation to ensure it runs.

Answer 3

It seems odd to me too. There are a few advantages - such as allowing multiple "handlers" but the semantics are significantly different to normal error handling. In particular, the fact that it doesn't automatically get propagated up the stack concerns me - unless the error handlers themselves throw an exception, the logic is going to keep going as if everything was still okay when it should probably be aborting the current operation.

Another way of thinking about this: suppose the method is meant to return a value, but you've detected an error early. What value do you return? Exceptions communicate the fact that there is no appropriate value to return...

Answer 4

This looks really odd to me, firstly IDisposable is your friend, use it.

If you are dealing with errors and exceptional situations you should be using exceptions, not events, as its much simpler to grasp, debug and code.

So it should be

using(var resource = AllocateLotsOfMemory())
{
   if(something_bad_happened) 
   {
     throw new SomeThingBadException();
   }
}

Answer 5

We have a base Error object and ErrorEvent that we use with the command pattern in our framework to handle non-critical errors (e.g. validation errors). Like exceptions, people can listen for the base ErrorEvent or a more specific ErrorEvent.

Also there's a significant difference between your two snippets.

if resource.FreeLotsOfMemory() clears out the StatusProperty value rather than just setting it to null, your temporary variable will be holding an invalid object when OddException is created and thrown.

The rule of thumb is that Exceptions should only be thrown in non-recoverable situations. I really wish C# supported a Throws clause that's the only thing I really miss from Java.

Answer 6

To be honest, events signaling errors strikes me as scary.

There's a disagreement between camps around returning status codes and throwing exceptions. To simplify (greatly) : The status code camp says that throwing exceptions places detecting and handling the error too far from the code causing the error. The exception throwing cap says that users forget to check status codes and exceptions enforce error handling.

Errors as events seems like the worst of both approaches. The error cleanup is completely separate from the code causing the error, and notification of error is completely voluntary. Ouch.

To me, if the method did not fulfill it's implicit or explicit contract (it didn't do what it was supposed to do), an exception is the apropriate response. Throwing the information you need in the exception seems reasonable in this case.

Answer 7

Take a look at this post by Udi Dahan. Its an elegant approach for dispatching domain events. The previous poster is correct in saying that you should not be using an event mechanism to recover from fatal errors, but it is a very useful pattern for notification in loosely coupled systems:

public class DomainEventStorage<ActionType>
{
    public List<ActionType> Actions
    {
        get
        {
            var k = string.Format("Domain.Event.DomainEvent.{0}.{1}",
                                  GetType().Name,
                                  GetType().GetGenericArguments()[0]);
            if (Local.Data[k] == null)
                Local.Data[k] = new List<ActionType>();

            return (List<ActionType>) Local.Data[k];
        }
    }

    public IDisposable Register(ActionType callback)
    {
        Actions.Add(callback);
        return new DomainEventRegistrationRemover(() => Actions.Remove(callback)
            );
    }
}

public class DomainEvent<T1> : IDomainEvent where T1 : class
{
    private readonly DomainEventStorage<Action<T1>> _impl = new DomainEventStorage<Action<T1>>();

    internal List<Action<T1>> Actions { get { return _impl.Actions; } }

    public IDisposable Register(Action<T1> callback)
    {
        return _impl.Register(callback);
    }

    public void Raise(T1 args)
    {
        foreach (var action in Actions)
        {
            action.Invoke(args);
        }
    }
}

And to consume:

var fail = false;
using(var ev = DomainErrors.SomethingHappened.Register(c => fail = true) 
{
   //Do something with your domain here
}

Answer 8

Another major problem with this approach are concurrency concerns.

With traditional error handling, locks will be released as control moves up the call stack to the error handler in a controlled manner. In this scheme, all locks will still be held when the event is invoked. Any blocking that occurs within the error handler (and you might expect some if there's logging) would be a potential source of deadlocks.

Answer 9

The first snippet should probably be

resource = AllocateLotsOfMemory();
if (SomeCondition())
{
    try
    {
        OnOddError(new OddErrorEventArgs(resource.StatusProperty));
        return;
    }
    finally
    {
        resource.FreeLotsOfMemory();
    }
}

otherwise you won't free your resources when the event handler throws an exception.

As Mike Brown said, the second snippet also has a problem if resource.FreeLotsOfMemory() messes with the contents of resource.StatusProperty instead of setting it to null.