search

dotnet / reactive

The Reactive Extensions for .NET
http://reactivex.io
MIT License
6.73k stars 751 forks source link

Fix ToObservable operator can throw unhandled exception #2172

Open fedeAlterio opened 1 month ago

fedeAlterio commented 1 month ago

Fixes #1677 The issue was that exceptions thrown by GetAsyncEnumerator() were not being propagated to observer.OnError(), resulting in unhandled exceptions. This has been addressed by wrapping the GetAsyncEnumerator() call in a try-catch block

The same issue appears also on DisposeAsync(). This is more complex, because we cannot rely on the observer anymore, because it is already completed at that point. Currently the exceptioncauses the process to crash directly. In my opinion this is bad since there is nothing that warns the caller about that, and also it does not offer any solution.

Observable.FromAsync already "solves" this problem, so I used the same approach here, creating an overload where is possible to specify if suppress the exception or not. If not, the exception would be routed to TaskScheduler.UnobservedTaskException, rather that be rethrown on the ThreadPool / Synchronization Context directly (causing with 99% probability the process to crash)

fedeAlterio commented 1 month ago

@dotnet-policy-service agree

fedeAlterio commented 3 weeks ago

Hi @idg10,

Apologies for the ping! Could you take a look at the fix when you have a chance? This bug is quite severe as it's causing the process to completely shut down. We’ve implemented internally a custom alternative as a .ToObservableSafe(), but it’s challenging to enforce this version consistently due to the presence of .ToObservable within System.Linq that seems really authoritative. Thank you very much for your help!