Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

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

Closed
rdavisau opened this issue May 15, 2016 · 8 comments
Assignees

Comments

@rdavisau
Copy link
Contributor

rdavisau commented May 15, 2016

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.

@MisterJimson
Copy link

Anyone know if this is still happening?

@Qonstrukt
Copy link

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

@matthiaswenz matthiaswenz self-assigned this Oct 24, 2016
@matthiaswenz
Copy link
Contributor

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

@Qonstrukt
Copy link

Qonstrukt commented Oct 24, 2016

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.

@rdavisau
Copy link
Contributor Author

rdavisau commented Oct 24, 2016

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!

@matthiaswenz
Copy link
Contributor

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

@Qonstrukt
Copy link

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

@ElektrojungeAtWork
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants