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

What should I do with the global exception? #5290

Closed
dashenxian opened this issue Jan 14, 2021 · 18 comments
Closed

What should I do with the global exception? #5290

dashenxian opened this issue Jan 14, 2021 · 18 comments
Labels

Comments

@dashenxian
Copy link

dashenxian commented Jan 14, 2021

Whether there is a mechanism like wfp's DispatcherUnhandledException, my logic is that Project A refers to Project B, project B throws an exception, but not fatally just a hint. I want the program to continue running instead of exiting the program.

//when exception was throw
private void App_DispatcherUnhandledException(object sender, System.Windows.Threading.DispatcherUnhandledExceptionEventArgs e)
{
    if (e.Exception is UserFriendlyException)
    {
        e.Handled = true;
        Log.Logger.Error(e.Exception.Message);
        MessageBox(e.Exception.Message);
        //Other logic...
    }
    //Other logic...
}

Any suggestions or best practices?

I still don't find useful information through search engines, which is the only relevant introduction I've found so far, but I don't quite understand how to use this
https://gitter.im/AvaloniaUI/Avalonia?at=5d833ca2a7a5cc473319c6ed
https://www.reactiveui.net/docs/handbook/default-exception-handler/

@worldbeater
Copy link
Contributor

RxApp.DefaultExceptionHandler allows you to catch any exceptions thrown from a ReactiveCommand, ObservableAsPropertyHelper, and other stuff. It is recommended that in an Avalonia.ReactiveUI app you wrap the code that might throw exceptions into reactive commands (ReactiveUI ICommand implementation), see https://www.reactiveui.net/docs/handbook/commands/ This way your exceptions will tick through your RxApp.DefaultExceptionHandler and won't cause the crash of your app.

// Here we subscribe to ReactiveUI default exception handler to avoid app
// termination in case if we do something wrong in our view models. See:
// https://www.reactiveui.net/docs/handbook/default-exception-handler/
//
// In case if you are using another MV* framework, please refer to its 
// documentation explaining global exception handling.
RxApp.DefaultExceptionHandler = Observer.Create<Exception>(Console.WriteLine);

@pr8x pr8x added the question label Jan 19, 2021
@Gao996
Copy link

Gao996 commented Sep 3, 2021

The same problem is not solved

@olgaMuravjova
Copy link

Anything new on this issue?

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 2, 2021

@olgaMuravjova since DispatcherUnhandledException or any other global unhandled exception handling, which prevents app from crashing, (and most possibly being trapped in invalid state) is considered as a bad practice, we don't really plan to support it.

If you want to handle global errors, you can catch them in Program.Main method, log and close the app peacefully (restart or open report-error app if needed). Any other expected errors should be handled explicitly in place where they are expected.

Alternatively, as it was mentioned above, if you are using ReactiveUI with your app, this library actually provides DefaultExceptionHandler for exceptions inside of observables. Though in most cases it means that Observable pipeline now in error state and won't work anymore.

@maxkatz6 maxkatz6 closed this as completed May 4, 2022
@timunie
Copy link
Contributor

timunie commented May 11, 2022

We now have docs for this 🚀 : https://docs.avaloniaui.net/docs/getting-started/unhandledexceptions

@mysteryx93
Copy link

mysteryx93 commented Jun 10, 2022

I tried starting the app with

try
{
    BuildAvaloniaApp<TApp>()
        .StartWithClassicDesktopLifetime(args);
}
catch (Exception ex)
{
    MessageBox.Avalonia.MessageBoxManager.GetMessageBoxStandardWindow("Application Error", ex.ToString(), ButtonEnum.Ok,
        Icon.Error);
}

Also calling GlobalErrorHandler.Set on startup

public class GlobalErrorHandler : IObserver<Exception>
{
    private readonly IDialogService _dialogService;

    public static void Set(Window owner)
    {
        var errHandler = Locator.Current.GetService<GlobalErrorHandler>()!;
        errHandler.Owner = owner;
        RxApp.DefaultExceptionHandler = errHandler;
    }
    
    public GlobalErrorHandler(IDialogService dialogService)
    {
        _dialogService = dialogService;
    }

    public Window? Owner { get; set; }
    
    public async void OnNext(Exception error)
    {
        if (Debugger.IsAttached) Debugger.Break();

        var _ = await _dialogService.ShowMessageBoxAsync(Owner, error.ToString(), "Application Error", MessageBoxButton.Ok, MessageBoxImage.Error);
    }

    public void OnError(Exception error)
    {
        if (Debugger.IsAttached) Debugger.Break();

    }

    public void OnCompleted()
    {
        if (Debugger.IsAttached) Debugger.Break();

    }
}

Clicking a button that calls this command.

public ICommand AddFiles => _addFiles ??= ReactiveCommand.Create(AddFilesImpl);
private ICommand? _addFiles;
private async Task AddFilesImpl()
{
    throw new InvalidOperationException();
}

Neither of the 2 methods catches the error. What am I missing?

Using Avalonia v0.10.15
OS: Linux (Garuda)

@timunie
Copy link
Contributor

timunie commented Jun 15, 2022

I think you cannot show an Avalonia Window after the App is corrupt. If you know a method can throw, catch the exception locally.

In the global try catch you can write to a log file. If you need to open a new window, I think you should start a separate app.

Hope this helps

Happy coding
Tim

@mysteryx93
Copy link

Except that here the global try catch isn't catching anything.

@timunie
Copy link
Contributor

timunie commented Jun 15, 2022

Oh I don't know about ReactiveUI async Commands. I use MVVM CommunityToolkit with source generators. In there it works for me.

@maxkatz6
Copy link
Member

maxkatz6 commented Jun 15, 2022

ReactiveUI can be tricky sometimes.
For commands there are other options to catch exceptions, though not globally. See https://www.reactiveui.net/docs/guidelines/framework/asynchronous-commands
Even though their documentation says it 'default' one will be used if you haven't subscribed per each command https://www.reactiveui.net/docs/handbook/commands/#handling-exceptions

You might find more answers in their repository as well.

@maxkatz6
Copy link
Member

@mysteryx93 well, I haven't noticed one sure issue in your code.

public ICommand AddFiles => _addFiles ??= ReactiveCommand.CreateFromTask(AddFilesImpl); // Instead of .Create()
private ICommand? _addFiles;
private async Task AddFilesImpl()
{
    throw new InvalidOperationException();
}

@jinek
Copy link
Contributor

jinek commented Jul 1, 2022

@olgaMuravjova since DispatcherUnhandledException or any other global unhandled exception handling, which prevents app from crashing,

The point is not just to prevent crashing ( which can be done on another level, like at Program.cs), but rather to write appropriate handling.

(and most possibly being trapped in invalid state)

This is wrong statement, we can not know the probability of wrong state for every particular exception in general

is considered as a bad practice, we don't really plan to support it.

Exceptions, how they are built in .NET are supposed to be handled exactly this way, they can bubble up thru several levels up the stack to the appropriate handler. Nobody said before it's bad practice. Here, i gave example of the usage: #8418 (comment)

If you want to handle global errors, you can catch them in Program.Main method, log and close the app peacefully (restart or open report-error app if needed).

Exceptions in .NET are part of execution flow control, not necessary the way to close the application.

Any other expected errors should be handled explicitly in place where they are expected.

This method would introduce code duplication which never does make sense.

Alternatively, as it was mentioned above, if you are using ReactiveUI with your app, this library actually provides DefaultExceptionHandler for exceptions inside of observables. Though in most cases it means that Observable pipeline now in error state and won't work anymore.

As mentioned, this can not be used because Observable pipeline does not properly handle exceptions

@mysteryx93
Copy link

mysteryx93 commented Jul 6, 2022

I'm still unable to catch the exception.

I call GlobalErrorHandler.Set in OnFrameworkInitializationCompleted.

Also tried with this.

AppDomain.CurrentDomain.UnhandledException += (x, e) =>
{
    var a = "";
};

Set breakpoints, run my infamous command, and.... nothing.

public ICommand AddFolder => _addFolder ??= ReactiveCommand.CreateFromTask(AddFolderImpl);
private ICommand? _addFolder;
private async Task AddFolderImpl()
{
    throw new InvalidOperationException("You failed.");
}

Which is a bummer, because it really fails on MacOS and I got no way to diagnose.

The only way I'm succeeding at handling the exception is by creating an extension method and call .HandleExceptions when creating the command. But this creates memory leak issues by having references to local controls from a static class.

public static class ExtensionMethods
{
    public static ReactiveCommand<TParam, TResult> HandleExceptions<TParam, TResult>(this ReactiveCommand<TParam, TResult> command)
    {
        command.ThrownExceptions.Subscribe(Exception_Raised);
        return command;
    }

    private static void Exception_Raised(Exception e)
    {
        if (Debugger.IsAttached) Debugger.Break();
    }
}

@mysteryx93
Copy link

Reproduced the issue in a blank project template; since this ticket is closed, created a new issue about it.
#8457

@alanthinker
Copy link

@olgaMuravjova since DispatcherUnhandledException or any other global unhandled exception handling, which prevents app from crashing, (and most possibly being trapped in invalid state) is considered as a bad practice, we don't really plan to support it.

If you want to handle global errors, you can catch them in Program.Main method, log and close the app peacefully (restart or open report-error app if needed). Any other expected errors should be handled explicitly in place where they are expected.

Alternatively, as it was mentioned above, if you are using ReactiveUI with your app, this library actually provides DefaultExceptionHandler for exceptions inside of observables. Though in most cases it means that Observable pipeline now in error state and won't work anymore.

In Android project, I can not find a Main function.

@alanthinker
Copy link

alanthinker commented Nov 17, 2023

@maxkatz6 @jinek @dashenxian

I modified a set of my own code to handle global exceptions, and there are 2 ways to tweak it.
If neither is set, the behaves will be the same as the original version

way1

        DispatcherOperation.MySafeCallBack1 = DispatcherOperation_MySafeCallBack1;
        DispatcherOperation.MySafeCallBack2 = DispatcherOperation_MySafeCallBack2;
        
private static void DispatcherOperation_MySafeCallBack1(Action action)
{
    try
    {
        action();
    }
    catch (Exception ex)
    {
        _log.Error("DispatcherOperation_MySafeCallBack1 Ex:\n" + ex.Message, ex);
    }
}

private static void DispatcherOperation_MySafeCallBack2(SendOrPostCallback action, object? args)
{
    try
    {
        action(args);
    }
    catch (Exception ex)
    {
        _log.Error("DispatcherOperation_MySafeCallBack2 Ex:\n" + ex.Message, ex);
    }
}

way2

    DispatcherOperation.MyExceptionHandler = DispatcherOperation_MyExceptionHandler;

private static void DispatcherOperation_MyExceptionHandler(Exception ex)
{
    _log.Error("DispatcherOperation_MyExceptionHandler Ex:\n" + ex.Message, ex);
}

But I don't know which one makes the most sense

this is the core code I Modified.
src\Avalonia.Base\Threading\DispatcherOperation.cs

    protected override void InvokeCore()
    {
        try
        {
            if (MySafeCallBack2 != null)
            {
                MySafeCallBack2.Invoke((SendOrPostCallback)Callback!, _arg);
            }
            else
            {
                ((SendOrPostCallback)Callback!)(_arg);
            }

            lock (Dispatcher.InstanceLock)
            {
                Status = DispatcherOperationStatus.Completed;
                if (TaskSource is TaskCompletionSource<object?> tcs)
                    tcs.SetResult(null);
            }
        }
        catch (Exception e)
        {
            lock (Dispatcher.InstanceLock)
            {
                Status = DispatcherOperationStatus.Completed;
                if (TaskSource is TaskCompletionSource<object?> tcs)
                    tcs.SetException(e);
            }

            if (MyExceptionHandler != null)
            {
                MyExceptionHandler(e);
            }
            else
            {
                if (ThrowOnUiThread)
                    throw;
            }
        }
    }

@bruno-garcia
Copy link

... and I got no way to diagnose.

Exception should end up on AppDomain.UnhandledException, right? So a tool like Sentry should help with useful context to what went wrong (stack trace, line numbers, breakdown by OS, logs/breadcrumbs etc).

I got to this issue because we (Sentry) want to know if there are additional hooks we need to catch any type of error coming from Avalonia, or if we need a dedicated integration Sentry.Avalonia:

@StefanKoell
Copy link
Contributor

@bruno-garcia
There has been another discussion and some PRs already addressing this:
#14349

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