bitstadium/HockeySDK-Xamarin

Android SDK should not terminate on UnobservedTaskException unless ThrowUnobservedTaskExceptions config is set

Closed this issue · 8 comments

Howdy,

Currently, the Android SDK will terminate the app if an UnobservedTaskException occurs. This is not expected behaviour - from .NET 4.5 onwards teardown for this reason is opt in, so apps typically don't crash for this reason.

I did some testing and found that it's possible to opt in on Xamarin.iOS, but after some time spent trying to do the same on Android I haven't had any luck. You can probably check with the team as to how app.config is supposed to work on Android - if it's supported then I guess the correct behaviour would be to check for opt in at registration time and only wire up the UnobservedTaskException event if you know those exceptions are going to result in an app crash. If not, then I think it makes more sense for the the default behaviour in these cases to be no logging/termination.

Anyone know if this is still happening?

This is for me. On both Android and iOS. Due the way ReactiveUI works this can happen quite often for certain situations.

Hi @rdavisau and others - sorry for the delay. We are looking at ways to improve this behavior in the future - to make sure we do the right thing, we need your input.
One thing that will be hard for the SDK to figure out will be whether an exception would lead to a crash - it can't really figure that out for itself.
We appreciate your time and hope you can provide more samples as to when this issue occurs and how you currently work around it.
Of course, we also encourage participation through PRs on this 😊

Thanks, Mat

I'm actually finding out our apps are crashing for a lot of customers while it shouldn't. This happens all the time in the following scenario:

We're using Refit and Akavache for REST calls and caching. Refit wraps HttpClient SendAsync calls into IObservable sequences so we can use them more easily with Akavache or Rx in general.
The SendAsync calls work with Task's which contains the Exception if there is any, and in the Rx context this is converted to OnError handling in the subscription.

In our situation it seems as if Exceptions occurring this way, and handled through an Observable are seen by Hockey as uncaught exceptions and it brings the whole app down.

I'll try to write a simple example app which demonstrates this problem later this week if possible. Currently I need to pull Hockey from all our apps because it's more hurting than helping, and I have no clue how to work around it consistently.

Hi Matthias and everyone - here's what i've found from investigating. This is a simple reproduction on the issue on iOS. As it is, the app crashes after the unawaited Task.Run call. However, if you comment out the SetupHockeyApp line the app does not crash.

The crash is caused by these lines in the HockeyApp SDKs. Not observing an exception that occurs in a task does not actually result in termination of a .NET4.5+ app, so I don't think it's correct behaviour for HockeyApp to do that by default. The main issue with this is that if any library you depend on happens to internally allow an exception within a Task to go unhandled - e.g. for a 'fire and forget' scenario - it is impossible to catch, and including HockeyApp will result in crashes where there otherwise would be none.

I think a reasonable solution is to make this behaviour configurable, and disabled by default. This means that HockeyApp matches the .NET platform, but allows users to get this behaviour if they want. I've taken a shot at it and will open a PR. I tested it on iOS and toggling the parameter gave me control over whether or not the app was affected by the unobserved task exceptions as expected. I'm not precious about the approach so you can take the PR or just use it as inspiration.

@Qonstrukt, it might be worth you pulling the branch and testing your app against a build from it given you have a good 'real world' scenario to test against!

Thanks @Qonstrukt for your input and thanks @rdavisau for providing the insight and PR - I will have a look at this later today.

Thanks @rdavisau, your version fixes this for us. 👍

We just released 4.1.1 which comes with a fix for this! Thx @rdavisau and @Qonstrukt! =)