Swallowing exceptions from a long-running event aggregator observable

Question

I am using a simple Subject<object> to implement the Event Aggregator pattern in a web application like so:

public class EventAggregator
{
    private readonly ISubject<object, object> _subject = Subject.Synchronize(new Subject<object>());
    public IObservable<T> GetEvent<T>()
    {
        return _subject.AsObservable().OfType<T>();
    }

    public void Publish<TEvent>(TEvent sampleEvent)
    {
        _subject.OnNext(sampleEvent);
    }
}

When an operator or subscriber throws an exception, I want to log it then ignore it and keep publishing events - essentially I want the stream to "keep going" in the case of some unexpected behaviour, as the Event Aggregator has a singleton lifetime in the context of the application.

In this question the answer given is to create a deferred observable and use Retry(), but I do not want a cold observable here.

The solution I came up with is to use Catch and a try-catch in the subscriber method, which I wrapped up into an extension:

public static IDisposable SubscribeSwallowExceptions<T>(this IObservable<T> source, Action<T> subscription, Action<Exception> log)
{
    source = source.Catch((Exception e) => { log(e); return source; });

    return source.Subscribe(x =>
    {
        try { subscription(x); }
        catch (Exception e) { log(e); }
    });
}

I know that "catch-all" exception handling is generally frowned upon, but in this case I am not sure what other options I have, given that I want the subscription to remain even when an exception is thrown. I don't know what types of exceptions might occur, as I do not yet know what work will be done when processing the stream.

Is this an acceptable way to deal with potential exceptions, and can you foresee any issues that might trip me up with this approach?

Lengthy related discussion with Rx team's feedback, in case you're interested: http://social.msdn.microsoft.com/Forums/en-US/ee5dba5d-eea9-4d85-8f58-c2e1c71ef33a/about-the-contracts-in-rx-exceptions-and-blocking?forum=rx — Dave Sexton, Sep 04 '14 at 14:18

score 1 · Answer 1 · answered Mar 17 '14 at 15:57

This is a problematic solution. Once a subscriber has thrown an exception, you must really assume they are dead in the water at that point (there's little else you can sensibly do) and not call them any further. With your approach, a subscriber that has "gone bad" will keep being sent events and could potentially throw exceptions ad infinitum.

Also, you can't really rely on subscribers using your extension method. I would rewrite your GetEvent<T> method as follows (obviously replace my Console.WriteLine with some logging). This approach will dispose the subscription due just to the bad subscriber, and keep everyone else running.

public IObservable<T> GetEvent<T>()
{       
    return Observable.Create<T>(o =>
    {
        var source = _subject.OfType<T>();
        var m = new SingleAssignmentDisposable();
        m.Disposable = source.Subscribe(
            x => {
                try {
                    o.OnNext(x);                        
                }
                catch(Exception e) {
                    Console.WriteLine(e);
                    m.Dispose();
                }
            },
            e => {
                try {
                    o.OnError(e);                       
                }
                catch(Exception ex) {
                    Console.WriteLine(ex);
                }
                finally {
                    m.Dispose();
                }
            },
            () => {
                try {
                    o.OnCompleted();                        
                }
                catch(Exception e) {
                    Console.WriteLine(e);
                }
                finally {
                    m.Dispose();
                }
            }               
        );

        return m;
    });
}

Agreed. It is not idiomatic Rx to have subscribers throw in their OnNext handlers. The same problem exists for standard .NET event and the Prism EventAggregator pattern. — Lee Campbell, May 22 '14 at 21:53
+1 Though I think it's bad practice to swallow exceptions like this as well. It sometimes feels right to try to keep the app running under any error condition, but rarely is it correct. In your example, the assumption is that no observers' exception should be considered fatal, which is ultimately very dangerous for user data (as I'm sure you know -- just wanted to be very clear about how this is an anti-pattern) — Dave Sexton, Sep 04 '14 at 14:14
Thanks Dave, I totally agree, and worth stating. In fact I made a similar point recently here: http://stackoverflow.com/questions/25344320/how-to-handle-exceptions-thrown-by-observers-onnext-in-rxjava/25348225#25348225 — James World, Sep 04 '14 at 19:30

Swallowing exceptions from a long-running event aggregator observable

1 Answers1