Create, test, and raise a copy of public event's in an event handler to safeguard against exceptions in subscribers

Question

I see alot of code that just calls the eventhandler like so:

if( OnyMyEvent != null)
    OnMyEvent(this,"args");

But in this article C# Event Implementation Fundamentals, Best Practices and Conventions By Jeffrey Schaefer: 7) Event Raising Code on CodeProject he describes a way to raise events so that exceptions subscribers doesnt affect the raiser. I'm curious if this is a best practice that should be applied.

Here is a sample (not tested/compiled) so readers get the idea:

public delegate void MessageReceivedEventHandler(object sender, MessageEventArgs e);

class MessageEventArgs : EventArgs
{
    public string Message
    { get; set; }

    public MessageEventArgs( string message )
    { this.Message = message; }
}

class EventTriggeringClass
{
    public event MessageReceivedEventHandler MessageReceived;

    protected virtual void OnMessageReceived( MessageEventArgs e )
    {
        this.RaiseTriggerOnMessageReceived( e );
    }

    private void RaiseOnMessageReceived( MessageEventArgs e )
    {
        MessageReceivedEventHandler handler = this.MessageReceived;
        if ( handler != null )
        {
            Delegate[] eventHandlers = handler.GetInvocationList();
            foreach ( Delegate currentHandler in eventHandlers )
            {
                MessageReceivedEventHandler currentSubscriber = ( currentHandler as MessageReceivedEventHandler );
                try
                {
                    currentSubscriber( this, e );
                }
                catch ( Exception ex )
                {
                    Debug.Assert( ex == null, ex.Message, ex.ToString() );
                }
            }
        }
    }    

    public void Read()
    {
        bool foundMessage = false, hasMoreMessages = true;
        string msg;
        while( hasMoreMessages )
        {
            // this way or..
            if( foundMessage )
                this.OnMessageReceived( new MessageEventArgs( msg ) );
            // the other way
            if( MessageReceived != null )
                MessageReceived(this, new MessageEventArgs( msg ) );
        }
    }
}

Answer 1

This is not a best practice, why on earth would you hide an exception ? If this is an exception from code you don't trust, this code should not be called (event or not) directly from trusted code.

What if the exception is raised only in release and not in debug ? What if the exception is raised only on some computer depending on some mysterious settings ? You will have hard time to find the bug.

catch (Exception) must be avoided. Please, fail fast !

Setting the error within the event arguments is more expressive, the subscriber will know how to notify you an error :

public class MyEventArgs : EventArgs
{
  private List<MyException> errors = new List<MyException>();

  public ICollection<MyException> Errors { get { return errors; } }
}

Answer 2

The way to safeguard against exceptions thrown by subscribers is to handle the exceptions. Excluding log-and-rethrow (which isn't so much handling an exception as letting it make a brief detour on its way), there are two kinds of exception handling: smart and stupid.

Smart exception handling is when you know what kind of exception a given method can throw, you understand the circumstances that can cause that to happen, and you know what the right way to recover from it is.

Stupid exception handling is everything else. If you don't know why a method is throwing an exception, you can't know that it's safe to handle the exception.

In the case of an event handler with six subscribers, if the first subscriber throws an exception, how do you know that it's safe to call the other five? You don't. You got an exception. You're done, until you find out what caused the exception and fix it. The best that you can hope for if you call the other event handlers is that the condition that caused the exception won't cause them to malfunction.

There are circumstances in which this isn't true, but generally speaking, if those circumstances apply, you're in a position to do smart exception handling. For instance, if the things subscribing to the event handler are sending exceptions to external systems, you might be able to say "if one of these things fail, I should still send all the other messages." But you only know that because you know something about the specific problem space that the event and its handlers are collaborating to solve.

But doing this by default? That's the very opposite of a best practice.