canton7 / Stylet

A very lightweight but powerful ViewModel-First MVVM framework for WPF for .NET Framework and .NET Core, inspired by Caliburn.Micro.
MIT License
988 stars 143 forks source link

OnInitialActivate() Method seems to swallow exceptions #133

Closed FutureTD closed 4 years ago

FutureTD commented 4 years ago

It seems the OnInitialActivate() Method swallows exceptions instead of throwing or passing it to the Bootstrapper OnUnhandledException Method.

It would be nice if that method did not just swallow exceptions if possible.

canton7 commented 4 years ago

It shouldn't. Have you got a repro, please?

On 8 June 2020 06:02:27 BST, FutureTD notifications@github.com wrote:

It seems the OnInitialActivate() Method swallows exceptions instead of throwing or passing it to the Bootstrapper OnUnhandledException Method.

It would be nice if that method did not just swallow exceptions if possible.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/canton7/Stylet/issues/133

FutureTD commented 4 years ago

Hey, thanks for the response I do not currently have a repo I was just testing It in my private project.

for example:

    public class MyViewModel: Screen
    {
        protected override void OnInitialActivate()
        {
            throw new InvalidOperationException();
        }
    }

When OnInitialActivate() gets ran I expect the application to crash or execute my bootstrap method

        protected override void OnUnhandledException(DispatcherUnhandledExceptionEventArgs e)
        {
            Environment.Exit(0);
        }

It does not at the moment, just seems to swallow the error.

canton7 commented 4 years ago

OK thanks, I'll see if I can repro on my end.

On 8 June 2020 08:20:24 BST, FutureTD notifications@github.com wrote:

Hey, thanks for the response I do not currently have a repo I was just testing It in my private project.

for example:

   public class MyViewModel: Screen
   {
       protected override void OnInitialActivate()
       {
           throw new InvalidOperationException();
       }
   }

When OnInitialActivate() gets ran I expect the application to crash or execute my bootstrap method

protected override void
OnUnhandledException(DispatcherUnhandledExceptionEventArgs e)
       {
           Environment.Exit(0);
       }

It does not at the moment, just seems to swallow the error.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/canton7/Stylet/issues/133#issuecomment-640416528

FutureTD commented 4 years ago

Were you able to fix / verify this issue?

canton7 commented 4 years ago

I wasn't able to reproduce this, I'm afraid.

Steps:

  1. Clone the repo
  2. Open the Samples soluion
  3. In Stylet.Samples.Hello, edit ShellViewModel to have an OnInitialActivate which throws
  4. Start Stylet.Samples.Hello

Please post some steps which I can follow which reproduces this.

FutureTD commented 4 years ago

During further testing I figured out the problem was coming from the built-in conductor I was using: Conductor.Collection.OneActive

I assumed that when you used this conductor with a TabControl and switched tabs, It would set the ActiveItem property to the selected screen which would in turn execute the OnInitialActivate() and OnActivate() methods.

This does not seem to be the case as it seems to activate the first screen in the items list but for the rest those methods do not get called.

It is not that it was swallowing exceptions, It was that the OnInitialActivate() and OnActivate() methods were not getting called at all.

    public class RootViewModel: Conductor<IScreen>.Collection.OneActive
    {  
        protected override void OnInitialActivate()
        {
            Items.AddRange(new IScreen[]
                           {
                               new FirstPageViewModel(),
                               new SecondPageViewModel()
                           });
        }
    }

    public class FirstPageViewModel: Screen { }

    public class SecondPageViewModel: Screen
    {
        protected override void OnInitialActivate()
        {
            throw new InvalidOperationException();
        }
        protected override void OnActivate()
        {
            throw new InvalidOperationException();
        }
    }

and for the Window.xaml you would use

<TabControl Style="{StaticResource StyletConductorTabControl}"/>

The application should crash when you switch to the Second Page unless I am mistaken, however it does not.

canton7 commented 4 years ago

Cool, that's why I asked for a repro in my first message.

I'll look into this, thanks. The lifecycle stuff should be happening to all tabs.

FutureTD commented 4 years ago

Were you able to reproduce the issue?

canton7 commented 4 years ago

Right, this happens when the ActiveItem is set from a binding (in this case, the switching between tabs is crucial), which causes the VM to be activated. It's because bindings swallow exceptions they encounter by default.

I'm not sure what the best course of action is here: on the one hand this is how WPF works; on the other, we're doing rather more logic from a binding than is usual for WPF.

canton7 commented 4 years ago

The other complication is that we don't know when ActiveItem is being set from a binding, or from code. We don't want to catch and rethrow exceptions in the second case.