Prevent fail-fast (crashing the AppDomain) from happening
gluck opened this issue · 2 comments
With Rx (and Tasks in general), it's easy to write code which will be executed in various (threading) contexts.
Previously "fail-fast" was the norm in .Net (it still largely is because of legacy), and ThreadPool respects that (un-catched exceptions in scheduled actions will kill your app).
But since .Net 4.5, TaskPool has a different behavior (which is good, I can elaborate the reasons if needed).
This request is to prevent any fail-fast from happening in Rx and fix scenarios where:
Observable.Return(Unit.Default).ObserveOn(TaskPool).Subscribe(_ => throw new Exception())
wouldn't kill the app, but
Observable.Return(Unit.Default).ObserveOn(ThreadPool).Subscribe(_ => throw new Exception())
will.
Given the schedulers are async, the exception can't be thrown to caller, and it needs to be provided to some event for troubleshooting (only thing worse than fail-fast is exception hiding), which happens with TaskPool through TaskScheduler.UnobservedTaskException
Currently with Rx 2.2.5 we can achieve that by customizing the PlatformEnlightmentProvider, but goal is to get rid of it.
For reference our unit tests which are passing with the modified provider:
https://gist.github.com/gluck/5676596e6d321089b9841bd16debd390
The problem I see is that neither ObserveOn nor Subscribe would now how the underlying execution scheme deals with such thrown exceptions.
I suspect the current behavior is due to some older design decisions. @bartdesmet could you weigh in?
The Catch operator on schedulers is meant to address this, though it requires users to manually use it on scheduler parameter. I guess the question here is more around a global behavior. That's where the enlightenment provider came in, and hence it shouldn't be removed haphazardly.
I guess we could consider global exception handlers on the existing schedulers that lack an underlying facility that does provide exception handling (e.g. WPF dispatchers). The one issue with that is when multiple unhandled exception event handlers are attached to such events and it becomes hard to reason over the order. Also, the event should likely have args that allow an exception to be set as "handled". A design for this is understood and been done before, so I wouldn't be opposed to it for advanced users to leverage.