CODEWITHSUNDEEP
c#.neteventsc#-4.0
Brett Ryan
Brett Ryan

Reputation: 28255

Advice on handling exceptions in multi-threaded events

I have a class that fires events delegating to DispatcherObject targets which is marshalling fine, though I have the problem where if a handler throws an exception in it's event code, what should I do? I don't want to prevent other listeners from handling the event. I'm looking for advice on how to handle such a problem.

Given an abstract base class with a Saved event, my pattern would be as follows:

public event EventHandler<EventArgs> Saved;
public void Save() {
    try {
        OnSave();
    } catch (Exception) {
        // What should I do here? throwing prevents subsequent handlers,
        // while catching gobbles up the exception. Should this be in OnSave()?
    }
}
protected virtual void OnSave() {
    EventHandler<EventArgs> evt = Saved;
    if (evt != null) {
        var args = EventArgs.Empty;
        foreach (var handler in evt.GetInvocationList()) {
            var target = handler.Target as DispatcherObject;
            if (target == null || target.CheckAccess()) {
                var h = handler as EventHandler<EventArgs>;
                if (h != null) h(this, args);
            } else {
                target.Dispatcher.Invoke(handler, this, args);
            }
        }
    }
}

I've thought about building an exception that holds all exceptions like an ArrayException or something, but this doesn't seem right.

Advice on what to do here would be very much appreciated.

UPDATE: I thank both Daniel and Henrik for your answers, If I could mark both as answered I would, I've decided to go with handling the event as I really don't want it going completely unnoticed, my final solution is as follows (for others looking for the solution).

public event EventHandler<EventArgs> Saved;
public void Save() {
    OnSave();
}
protected virtual void OnSave() {
    EventHandler<EventArgs> evt = Saved;
    if (evt != null) {
        var args = EventArgs.Empty;
        var handlers = evt.GetInvocationList();
        var exceptions = new Queue<Exception>(handlers.Length);
        foreach (var handler in handlers) {
            try {
                var target = handler.Target as DispatcherObject;
                if (target == null || target.CheckAccess()) {
                    var h = handler as EventHandler<EventArgs>;
                    if (h != null) h(this, args);
                } else {
                    target.Dispatcher.Invoke(handler, this, args);
                }
            } catch (Exception ex) {
                exceptions.Enqueue(ex);
            }
        }
        if (exceptions.Count == 1) {
            var ex = exceptions.Peek();
            throw new Exception(ex.Message, ex);
        }
        if (exceptions.Count > 0) {
            throw new AggregateException(exceptions);
        }
    }
}

Upvotes: 2

Views: 215

Answers (2)

Henrik
Henrik

Reputation: 23324

No need to create your own ArrayException. You can just use System.AggregateException for wrapping multiple exceptions.

Upvotes: 2

Daniel Mošmondor
Daniel Mošmondor

Reputation: 19956

From your code, it seems that you should wrap the invoking in the loop into the try/catch, and gobble the exception there.

I don't see how the OnSave caller could benefit from having even the single exception from all the event handlers. Since you have multiple event handles, and from you architecture sketch it seems that every one does something completely different (since each must be called), exception space is so heterogeneous so the caller won't be able to deal with it anyway.

So the question could be: how do I deal with multiple event exceptions on the higher level?

Can you try to correct and re-run OnSave? Will you have to omit calls that succeeded? And so on...

Upvotes: 1

Related Questions