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

ConductorBaseWithActiveItem swallows exceptions in ActivateItemAsync #863

Open
nkreipke opened this issue Jun 22, 2023 · 4 comments
Open

Comments

@nkreipke
Copy link

Exceptions thrown in ActivateItemAsync, including all other methods called from the default implementation (TryDeactivateAsync, TryActivateAsync, NotifyOfPropertyChange, OnActivationProcessed) are silently discarded.

Repro:

public class MainViewModel : Conductor<Model>.Collection.OneActive
{
    public MainViewModel()
    {
        Items.AddRange(new []
        {
            new Model(),
            new Model(),
            new Model(),
        });
    }

    public override Task ActivateItemAsync(Model item, CancellationToken cancellationToken = new CancellationToken())
    {
        throw new Exception("please notice me");
    }
}

When selecting an item in a WPF ListBox, nothing happens. This is especially bad if there is additional code after the exception, which is simply not being executed without any indication.

I've found a similar issue #745 which is probably related.

The reason is that the setter of ActiveItem calls ActivateItemAsync without awaiting the task. This is a regression from 3.x. I realize awaiting in a setter is not possible and this might be a design issue:

set => ActivateItemAsync(value, CancellationToken.None);

@darxis
Copy link
Contributor

darxis commented Oct 16, 2024

I have the same issue on Caliburn.Micro v4.0.212.

@nkreipke Have you managed to find a workaround to catch unhandled exceptions thrown in OnActivateAsync, OnDeactivateAsync, OnInitializeAsync and others?

@nkreipke
Copy link
Author

@nkreipke Have you managed to find a workaround to catch unhandled exceptions thrown in OnActivateAsync, OnDeactivateAsync, OnInitializeAsync and others?

We had to reimplement the Conductor and are in the process of ditching Caliburn altogether as it seems to be unmaintained.

@vb2ae
Copy link
Member

vb2ae commented Oct 21, 2024

@nkreipke @darxis sorry been busy have not had as much time to work on this as I would like. What are you looking for an unhandled exception event?

@darxis
Copy link
Contributor

darxis commented Oct 22, 2024

@vb2ae I have an application for internal usage that doesn't require user-friendly exception handling - the app would just crash on Caliburn v3 - it's fine for me.

For example it fetches data from my SQL database within OnActivate - if the SQL server could not be reached the app would crash on Caliburn v3. Now when I upgraded to Caliburn v4 using the OnActivateAsync the exception is just ignored because as @nkreipke figured out, it is thrown within an unobserved task that is not awaited anywhere.

It would be nice to have the possibility to globally catch these exceptions thrown within OnActivateAsync, OnDeactivateAsync etc. instead of having separate try/catch inside every ViewModel OnActivateAsync methods and displaying an error to the user.

Maybe a global exception handler could be provided? Something like the void BootstrapperBase.OnUnhandledException(object sender, DispatcherUnhandledExceptionEventArgs e) method or EventHandler<ResultCompletionEventArgs> Coroutine.Completed event? Like OnActivateAsyncUnhandledException?

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

No branches or pull requests

3 participants