Closed slajalin closed 5 years ago
Hi Janet,
Thanks for such a detailed bug report.
Will take a look over the next week or so and see if I can fix it.
@slajalin I will look into your repro steps and logs. In the mean time I would take a look at https://github.com/reactiveui/ReactiveUI/issues/1857, as it seems similar why you're getting an error.
And I'll make a comment on consistent behavior across platforms.
Last week I had an Android bug come back to me because a loading indicator was taking over the UI thread. I didn't add an ObserveOn(RxApp.MainThreadScheduler)
to my observable pipeline. On iOS I got no such experience, because iOS handles the Rx code differently. For Android I had to explicitly dictate the thread to fix the problem.
My point is, your Xamarin C# code compiles down to the native platform, and we may not be able to achieve consistency because we aren't guaranteed it from the platforms. If it's achievable we'll work to get there.
Thanks for your thorough issue report.
@glennawatson @RLittlesII thanks for the fast replies and looking into this.
I'll take a look at #1857 and see if there's any additional steps we might have missed. The difference between iOS and Android is understandable and we tried implementing the DefaultExceptionHandler in Android to natively throw the exception up and force a crash. However, we lost call stack information and it couldn't track down the original error.
Thanks for the help!
The reason why @rlittlesii pointed to #1857 is your line
public ReactiveCommand<Unit, Unit> CrashCommand => ReactiveCommand.Create(() => { var someArray = new int[2]; var shouldBreak = arr[3]; });
Should be:
public ReactiveCommand<Unit, Unit> CrashCommand { get; } => ReactiveCommand.Create(() => { var someArray = new int[2]; var shouldBreak = arr[3]; });
Your first approach would recreate a command each time.
The creator of #1857 made the same bug
Ahh, good catch. I went and updated our constructor so that it's created there only once. However, I was still seeing the same behavior on Android with a breakpoint on the error, but no UnhandledErrorException.
Tried revisiting the DefaultExceptionHandler and overriding it in the Android app:
RxApp.DefaultExceptionHandler = Observer.Create<Exception>((ex) => {
var exceptionString = "An object implementing IHandleObservableErrors (often a ReactiveCommand or ObservableAsPropertyHelper) has errored, thereby breaking its observable pipeline. To prevent this, ensure the pipeline does not error, or Subscribe to the ThrownExceptions property of the object in question to handle the erroneous case.";
new Handler(Looper.MyLooper()).Post(() => throw new UnhandledErrorException(exceptionString, ex));
});
Now I'm seeing the exception thrown natively to crash the app and its captured in our reporter, but ideally, this would be the default behavior for Android?
Android is honestly a special snowflake that doesn't play nice with Xamarin Forms at times. Cross platform doesn't mean consistency. It just means it "works" on both natively.
@cabauman may be able to shed a bit more light on the Android behavior as he works with that platform more than I do.
I'll still reproduce this to see if there is anything we can do to provide more consistency.
Seems to be tied to the Schedule method of AndroidUIScheduler.cs. If I comment out the following part:
if (_looperId > 0 && _looperId == Java.Lang.Thread.CurrentThread().Id)
{
return action(this, state);
}
then it throws and crashes as expected.
So if its already on the main thread, it invokes the action directly and somehow gets swallowed. But handler.Post
always gives the desired behavior.
We are building some benchmarks at the moment (and support for running them) to make sure any fix we produce doesn't cause regressions. Also some tests if we can.
Do you want to request a feature or report a bug?
I'd like to report a bug.
What is the current behavior? We currently have a MVVM solution setup where we bind the same method in our ViewModels to both Xamarin.iOS and Xamarin.Android projects using the
BindCommand
method. While trying to set up crash reporting, we noticed that the exception handling is different between the two platforms.Given our Reactive Command is the following:
public ReactiveCommand<Unit, Unit> CrashCommand => ReactiveCommand.Create(() => { var someArray = new int[2]; var shouldBreak = arr[3]; });
in iOS debug, we hit a breakpoint and the application outputs the following stacktrace as well quits the app:
in Android debug, we still hit a breakpoint at the callstack
but the error gets swallowed and a UnhandledErrorException is never thrown.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem
BindCommand
method.this.BindCommand(ViewModel, vm => vm.CrashCommand, v => v.CrashButton);
What is the expected behavior? Both platforms should expect the same behavior in terms of handling unexpected exceptions and should throw UnhandledErrorExceptions by default to crash the app.
What is the motivation / use case for changing the behavior?
Which versions of ReactiveUI, and which platform / OS are affected by this issue? Did this work in previous versions of ReativeUI? Please also test with the latest stable and development snapshot
I am able to reproduce the issue using ReactiveUI 9.2.2 & 8.6.3 nugets. iOS simulator: iPhone XS Max - iOS 12.0 , we are targeting iOS 10.0 Android simulator: Nexus 5X API 26, we a minimum supporting Android 5.1 (API 22) & targeting API 28
Other information (e.g. stacktraces, related issues, suggestions how to fix)
Full iOS stacktrace
Unhandled Exception: ReactiveUI.UnhandledErrorException: An object implementing IHandleObservableErrors (often a ReactiveCommand or ObservableAsPropertyHelper) has errored, thereby breaking its observable pipeline. To prevent this, ensure the pipeline does not error, or Subscribe to the ThrownExceptions property of the object in question to handle the erroneous case. ---> System.IndexOutOfRangeException: Index was outside the bounds of the array. at MyApp.Shared.ViewModels.MyViewModel+<>c.Full Android callstack
ReactiveUI.RxApp.<>c.<.cctor>b__11_1(System.IndexOutOfRangeException ex) in System.Reactive.AnonymousSafeObserver