Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot Catch Exceptions with RxApp.DefaultExceptionHandler #8457

Closed
mysteryx93 opened this issue Jul 6, 2022 · 9 comments
Closed

Cannot Catch Exceptions with RxApp.DefaultExceptionHandler #8457

mysteryx93 opened this issue Jul 6, 2022 · 9 comments
Labels

Comments

@mysteryx93
Copy link

I'm unable to to catch exceptions with RxApp.DefaultExceptionHandler.

Since Avalonia manages its own version of ReactiveUI, I'm posting it here.

To Reproduce
I created a new project from template and reproduced the problem here
https://mega.nz/file/fZQT1ZgK#7yyrTGj_kS1NWbKrSs4VkHWKakCPgvJCkzbYORMyYvQ

Expected behavior
Breakpoints in GlobalErrorHandler should get hit. They don't.

Desktop (please complete the following information):

  • OS: Garuda Linux
  • Version: 0.10.15
@maxkatz6
Copy link
Member

maxkatz6 commented Jul 6, 2022

Since Avalonia manages its own version of ReactiveUI, I'm posting it here.

No, we use normal ReactiveUI.
Avalonia.ReactiveUI is just some framework specific implementations and helpers for Avalonia projects.

@maxkatz6
Copy link
Member

maxkatz6 commented Jul 6, 2022

Ok, I checked your project.
There are at least two problems:

  1. Locator.Current.GetService<GlobalErrorHandler>() returns null for me. Splat is quite basic library to be used as a IoC container. From what I remember, it was created for internal usage and extensibility of ReactiveUI framework. (similarly, how we have application locator in Avalonia).
  2. More serious problem is the way, how ReactiveUI uses default exception handler.

In the source code ReactiveCommand line 660 reactive command reads and caches default exception handler. Which means, if handler is set after command was created, new handler will be ignored.
At the same time, you set default exception handler after MainWindow was created. And after command was read for the first time.

@maxkatz6
Copy link
Member

maxkatz6 commented Jul 6, 2022

So, solution for you would be to set default exception handler before any command property is read, i.e., before any control with Command property is added.

@glennawatson
Copy link
Contributor

Thanks @maxkatz6 to also add it's a good idea to do your splat registration and rxapp interactions on your program entry point. Eg Main or a application Init

@mysteryx93
Copy link
Author

mysteryx93 commented Jul 6, 2022

IOC thing is my mistake, forgot to clean that out for the sample. Replaced with this and problem remains.

private static GlobalErrorHandler Instance => _instance ??= new GlobalErrorHandler();
private static GlobalErrorHandler? _instance;

Moving registration to before creating the View works! That's an undocumented tricky one.

BUT then I cannot pass it the Owner to use to show error messages... would have to register the class first, then set the owner after.

@maxkatz6
Copy link
Member

maxkatz6 commented Jul 6, 2022

That's an undocumented tricky one.

Just for case added it to our documentation. I don't know if it was mentioned in more complete ReactiveUI docs.

BUT then I cannot pass it the Owner to use to show error messages

Not sure if I understand... Do you want to pass owner window to the error handler?
You at least can handle ReactiveCommand.ThrownExceptions on a specific command. But yeah, it would require adding quite a bit of code per each command.

Or show unhandled error only on the main window, which isn't ideal for sure.

@glennawatson
Copy link
Contributor

I'll update the rxui docs for the exception handler to make it more obvious when it should be registered

@mysteryx93
Copy link
Author

mysteryx93 commented Jul 6, 2022

Alright it works!

It's just slightly twisted registrations because

  • RxApp.DefaultExceptionHandler must be set before creating the View
  • The ViewModelLocator must be initialized before calling GlobalErrorHandler.Set(vm)

Thus I first create the ViewModel (which initializes the ViewModelLocator), then set GlobalErrorHandler, and finally the View... in that specific order.

I think I'll leave a comment in my code to clarify that for mortal souls.

// We must initialize the ViewModelLocator before setting GlobalErrorHandler.
// We must set GlobalErrorHandler before View is created.
var vm = ViewModelLocator.Main;
var errHandler = GlobalErrorHandler.Set(vm);

if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop)
{
    desktop.MainWindow = new MainView
    {
        DataContext = vm
    };
}

@mysteryx93
Copy link
Author

mysteryx93 commented Jul 6, 2022

Here's a full generic solution. With a nice extra perk: FASTER LOAD TIME! 😄 How? I initialize the View and the ViewModel/ViewModelLocator in parallel.

  1. Set DefaultExceptionHelper unitialized.
  2. Create View and initialize the ViewModelLocator in parallel.
  3. Configure GlobalErrorHandler.
/// <summary>
/// A global error handler that displays a message box when unhandled errors occur.
/// </summary>
public class GlobalErrorHandler : IObserver<Exception>
{
    private IDialogService? _dialogService;

    public static GlobalErrorHandler Instance => _instance ??= new GlobalErrorHandler(); 
    private static GlobalErrorHandler? _instance;
    
    /// <summary>
    /// Sets the global error handler to display all errors.
    /// We set dependencies later because we want to initialize the ViewModelLocator in parallel to the View,
    /// and DefaultExceptionHandler must be set before creating the view.
    /// </summary>
    public static void BeginInit() => RxApp.DefaultExceptionHandler = Instance;
    
    /// <summary>
    /// Sets the dependencies for handling and displaying errors.
    /// </summary>
    /// <param name="dialogService">The service used to display dialogs.</param>
    /// <param name="ownerVm">The ViewModel of the View in which to display error messages.</param>
    public static void EndInit(IDialogService dialogService, INotifyPropertyChanged? ownerVm)
    {
        Instance._dialogService = dialogService;
        Instance.OwnerVm = ownerVm;
    }

    /// <summary>
    /// Gets or sets the ViewModel of the View in which to display error messages.
    /// </summary>
    public INotifyPropertyChanged? OwnerVm { get; set; }
    
    /// <inheritdoc />
    public async void OnNext(Exception error) => ShowError(error);

    /// <inheritdoc />
    public void OnError(Exception error)
    {
    }

    /// <inheritdoc />
    public void OnCompleted()
    {
    }

    private async void ShowError(Exception error)
    {
        if (_dialogService != null && OwnerVm != null)
        {
            await _dialogService.ShowMessageBoxAsync(OwnerVm!, error.ToString(), "Application Error", MessageBoxButton.Ok, MessageBoxImage.Error);
        }
    }
}


/// <summary>
/// Common application class that handles creating the View and ViewModel in parallel.
/// </summary>
/// <typeparam name="T">The data type of the main Window.</typeparam>
public abstract class CommonApplication<T> : Application
    where T : Window
{
    /// <summary>
    /// Returns the <see cref="IClassicDesktopStyleApplicationLifetime" />.
    /// </summary>
    public IClassicDesktopStyleApplicationLifetime? DesktopLifetime => ApplicationLifetime as IClassicDesktopStyleApplicationLifetime;
    
    /// <summary>
    /// Once Avalonia framework is initialized, create the View and ViewModel.
    /// </summary>
    public override async void OnFrameworkInitializationCompleted()
    {
#if DEBUG
        // Required by Avalonia XAML editor to recognize custom XAML namespaces. Until they fix the problem.
        GC.KeepAlive(typeof(SvgImage));
#endif
        // We must initialize the ViewModelLocator before setting GlobalErrorHandler.
        // We must set GlobalErrorHandler before View is created.
        
        // Set DefaultExceptionHelper now but we want to initialize ViewModelLocator later in parallel with View for faster startup.
        GlobalErrorHandler.BeginInit();

        var desktop = DesktopLifetime;
        if (desktop != null)
        {
            // Initialize View and ViewModel/ViewModelLocator in parallel.
            var t1 = Task.Run(InitViewModel); 
            desktop.MainWindow = Activator.CreateInstance<T>();
            await t1.ConfigureAwait(true);
            desktop.MainWindow.DataContext = t1.Result;
        }
        
        GlobalErrorHandler.EndInit(Locator.Current.GetService<IDialogService>()!, desktop?.MainWindow.DataContext as INotifyPropertyChanged);

        base.OnFrameworkInitializationCompleted();
    }

    /// <summary>
    /// Initializes the ViewModel.
    /// </summary>
    /// <returns>The new ViewModel.</returns>
    protected virtual INotifyPropertyChanged? InitViewModel() => null;
}

public class App : CommonApplication<MainView>
{
    public override void Initialize() => AvaloniaXamlLoader.Load(this);

    protected override INotifyPropertyChanged? InitViewModel() => ViewModelLocator.Main;
}

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

No branches or pull requests

3 participants