Merge pull request #217 from zadjii-msft/dev/migrie/f/error-context-pr

This allows extension objects to write error messages specifically to the page that owns them. We don't need the `ShowExceptionMessage` anymore, because that just tossed the error at whatever page was open (even if it wasn't the page that had an error).

I also made this a `IPageContext`, rather than `IErrorContext`, so we could pass the page's task scheduler to that object too. That lets us have one base `UpdateProperties` implementation for all extension objects. 

I think this sneakily also adds support for `Page.Title` (separate from `Page.Name`)

Closes #203

src/modules/cmdpal/Exts/SamplePagesExtension/EvilSampleListPage.cs

@@ -2,7 +2,6 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Timers;
 using Microsoft.CmdPal.Extensions;
 using Microsoft.CmdPal.Extensions.Helpers;
 

src/modules/cmdpal/Exts/SamplePagesExtension/EvilSamplesPage.cs

@@ -2,6 +2,8 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.CmdPal.Extensions;
 using Microsoft.CmdPal.Extensions.Helpers;
 
@@ -25,6 +27,11 @@ public partial class EvilSamplesPage : ListPage
            Title = "Page that keeps throwing exceptions",
            Subtitle = "Will throw every 5 seconds once you open it",
        },
+       new ListItem(new ExplodeOnPropChange())
+       {
+           Title = "Throw in the middle of a PropChanged",
+           Subtitle = "Will throw every 5 seconds once you open it",
+       },
        new ListItem(new SelfImmolateCommand())
        {
            Title = "Terminate this extension",
@@ -40,3 +47,46 @@ public partial class EvilSamplesPage : ListPage
 
     public override IListItem[] GetItems() => _commands;
 }
+
+[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1402:File may only contain a single type", Justification = "Sample code")]
+internal sealed partial class ExplodeOnPropChange : ListPage
+{
+    private bool _explode;
+
+    public override string Title
+    {
+        get => _explode ? Commands[9001].Title : base.Title;
+        set => base.Title = value;
+    }
+
+    private IListItem[] Commands => [
+      new ListItem(new NoOpCommand())
+           {
+               Title = "This page will explode in five seconds!",
+               Subtitle = "I'll change my Name, then explode",
+           },
+        ];
+
+    public ExplodeOnPropChange()
+    {
+        Icon = new(string.Empty);
+        Name = "Open";
+    }
+
+    public override IListItem[] GetItems()
+    {
+        _ = Task.Run(() =>
+        {
+            Thread.Sleep(1000);
+            Title = "Ready? 3...";
+            Thread.Sleep(1000);
+            Title = "Ready? 2...";
+            Thread.Sleep(1000);
+            Title = "Ready? 1...";
+            Thread.Sleep(1000);
+            _explode = true;
+            Title = "boom";
+        });
+        return Commands;
+    }
+}

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandContextItemViewModel.cs

@@ -7,7 +7,7 @@ using Microsoft.CmdPal.UI.ViewModels.Models;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class CommandContextItemViewModel(ICommandContextItem contextItem, TaskScheduler scheduler) : CommandItemViewModel(new(contextItem), scheduler)
+public partial class CommandContextItemViewModel(ICommandContextItem contextItem, IPageContext context) : CommandItemViewModel(new(contextItem), context)
 {
     private readonly ExtensionObject<ICommandContextItem> _contextItemModel = new(contextItem);
 

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs

@@ -12,8 +12,6 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
 {
     private readonly ExtensionObject<ICommandItem> _commandItemModel = new(null);
 
-    protected TaskScheduler Scheduler { get; private set; }
-
     // These are properties that are "observable" from the extension object
     // itself, in the sense that they get raised by PropChanged events from the
     // extension. However, we don't want to actually make them
@@ -46,7 +44,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
             var model = new CommandContextItem(command!)
             {
             };
-            CommandContextItemViewModel defaultCommand = new(model, Scheduler)
+            CommandContextItemViewModel defaultCommand = new(model, PageContext)
             {
                 Name = Name,
                 Title = Name,
@@ -62,10 +60,10 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
         }
     }
 
-    public CommandItemViewModel(ExtensionObject<ICommandItem> item, TaskScheduler scheduler)
+    public CommandItemViewModel(ExtensionObject<ICommandItem> item, IPageContext errorContext)
+        : base(errorContext)
     {
         _commandItemModel = item;
-        Scheduler = scheduler;
     }
 
     //// Called from ListViewModel on background thread started in ListPage.xaml.cs
@@ -85,7 +83,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
         MoreCommands = model.MoreCommands
             .Where(contextItem => contextItem is ICommandContextItem)
             .Select(contextItem => (contextItem as ICommandContextItem)!)
-            .Select(contextItem => new CommandContextItemViewModel(contextItem, Scheduler))
+            .Select(contextItem => new CommandContextItemViewModel(contextItem, PageContext))
             .ToList();
 
         // Here, we're already theoretically in the async context, so we can
@@ -106,9 +104,9 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
         {
             FetchProperty(args.PropertyName);
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            // TODO log? throw?
+            PageContext.ShowException(ex);
         }
     }
 
@@ -141,6 +139,4 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel
 
         UpdateProperty(propertyName);
     }
-
-    protected void UpdateProperty(string propertyName) => Task.Factory.StartNew(() => { OnPropertyChanged(propertyName); }, CancellationToken.None, TaskCreationOptions.None, Scheduler);
 }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/DetailsViewModel.cs

@@ -7,7 +7,7 @@ using Microsoft.CmdPal.UI.ViewModels.Models;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class DetailsViewModel(IDetails _details, TaskScheduler Scheduler) : ExtensionObjectViewModel
+public partial class DetailsViewModel(IDetails _details, IPageContext context) : ExtensionObjectViewModel(context)
 {
     private readonly ExtensionObject<IDetails> _detailsModel = new(_details);
 
@@ -40,6 +40,4 @@ public partial class DetailsViewModel(IDetails _details, TaskScheduler Scheduler
         UpdateProperty(nameof(HeroImage));
         UpdateProperty(nameof(HasHeroImage));
     }
-
-    protected void UpdateProperty(string propertyName) => Task.Factory.StartNew(() => { OnPropertyChanged(propertyName); }, CancellationToken.None, TaskCreationOptions.None, Scheduler);
 }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs

@@ -8,6 +8,13 @@ namespace Microsoft.CmdPal.UI.ViewModels;
 
 public abstract partial class ExtensionObjectViewModel : ObservableObject
 {
+    public IPageContext PageContext { get; set; }
+
+    public ExtensionObjectViewModel(IPageContext? context)
+    {
+        PageContext = context ?? (this is IPageContext c ? c : throw new ArgumentException("You need to pass in an IErrorContext"));
+    }
+
     public async virtual Task InitializePropertiesAsync()
     {
         var t = new Task(() =>
@@ -18,7 +25,7 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject
             }
             catch (Exception ex)
             {
-                System.Diagnostics.Debug.WriteLine(ex);
+                PageContext.ShowException(ex);
             }
         });
         t.Start();
@@ -26,4 +33,14 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject
     }
 
     public abstract void InitializeProperties();
+
+    protected void UpdateProperty(string propertyName) =>
+        Task.Factory.StartNew(
+            () =>
+        {
+            OnPropertyChanged(propertyName);
+        },
+            CancellationToken.None,
+            TaskCreationOptions.None,
+            PageContext.Scheduler);
 }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs

@@ -9,8 +9,8 @@ using Microsoft.CmdPal.UI.ViewModels.Models;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
-    : CommandItemViewModel(new(model), scheduler)
+public partial class ListItemViewModel(IListItem model, IPageContext context)
+    : CommandItemViewModel(new(model), context)
 {
     private readonly ExtensionObject<IListItem> _listItemModel = new(model);
 
@@ -41,7 +41,7 @@ public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
 
         Tags = li.Tags?.Select(t =>
         {
-            var vm = new TagViewModel(t, Scheduler);
+            var vm = new TagViewModel(t, PageContext);
             vm.InitializeProperties();
             return vm;
         })
@@ -51,7 +51,7 @@ public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
         var extensionDetails = li.Details;
         if (extensionDetails != null)
         {
-            Details = new(extensionDetails, Scheduler);
+            Details = new(extensionDetails, PageContext);
             Details.InitializeProperties();
             UpdateProperty(nameof(Details));
             UpdateProperty(nameof(HasDetails));
@@ -78,7 +78,7 @@ public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
             case nameof(Tags):
                 Tags = model.Tags?.Select(t =>
                 {
-                    var vm = new TagViewModel(t, Scheduler);
+                    var vm = new TagViewModel(t, PageContext);
                     vm.InitializeProperties();
                     return vm;
                 })
@@ -93,7 +93,7 @@ public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
                 break;
             case nameof(Details):
                 var extensionDetails = model.Details;
-                Details = extensionDetails != null ? new(extensionDetails, Scheduler) : null;
+                Details = extensionDetails != null ? new(extensionDetails, PageContext) : null;
                 UpdateProperty(nameof(Details));
                 UpdateProperty(nameof(HasDetails));
                 break;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs

@@ -86,7 +86,7 @@ public partial class ListViewModel : PageViewModel
             foreach (var item in newItems)
             {
                 // TODO: When we fetch next page of items or refreshed items, we may need to check if we have an existing ViewModel in the cache?
-                ListItemViewModel viewModel = new(item, Scheduler);
+                ListItemViewModel viewModel = new(item, this);
                 viewModel.InitializeProperties();
                 _itemCache.Add(viewModel); // TODO: Figure out when we clear/remove things from cache...
 

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/ShowExceptionMessage.cs

@@ -1,9 +0,0 @@
-// Copyright (c) Microsoft Corporation
-// The Microsoft Corporation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-namespace Microsoft.CmdPal.UI.ViewModels.Messages;
-
-public record ShowExceptionMessage(Exception Exception)
-{
-}

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/PageViewModel.cs

@@ -4,16 +4,14 @@
 
 using CommunityToolkit.Mvvm.ComponentModel;
 using CommunityToolkit.Mvvm.Input;
-using CommunityToolkit.Mvvm.Messaging;
 using Microsoft.CmdPal.Extensions;
-using Microsoft.CmdPal.UI.ViewModels.Messages;
 using Microsoft.CmdPal.UI.ViewModels.Models;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class PageViewModel : ExtensionObjectViewModel
+public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
 {
-    protected TaskScheduler Scheduler { get; private set; }
+    public TaskScheduler Scheduler { get; private set; }
 
     private readonly ExtensionObject<IPage> _pageModel;
 
@@ -35,14 +33,18 @@ public partial class PageViewModel : ExtensionObjectViewModel
     // on the UI thread.
     public string Name { get; private set; } = string.Empty;
 
+    public string Title { get => string.IsNullOrEmpty(field) ? Name : field; private set; } = string.Empty;
+
     public bool IsLoading { get; private set; } = true;
 
     public IconDataType Icon { get; private set; } = new(string.Empty);
 
     public PageViewModel(IPage model, TaskScheduler scheduler)
+        : base(null)
     {
         _pageModel = new(model);
         Scheduler = scheduler;
+        PageContext = this;
     }
 
     //// Run on background thread from ListPage.xaml.cs
@@ -57,8 +59,9 @@ public partial class PageViewModel : ExtensionObjectViewModel
         {
             InitializeProperties();
         }
-        catch (Exception)
+        catch (Exception ex)
         {
+            ShowException(ex);
             return Task.FromResult(false);
         }
 
@@ -76,10 +79,12 @@ public partial class PageViewModel : ExtensionObjectViewModel
 
         Name = page.Name;
         IsLoading = page.IsLoading;
+        Title = page.Title;
         Icon = page.Icon;
 
         // Let the UI know about our initial properties too.
         UpdateProperty(nameof(Name));
+        UpdateProperty(nameof(Title));
         UpdateProperty(nameof(IsLoading));
         UpdateProperty(nameof(Icon));
 
@@ -93,9 +98,9 @@ public partial class PageViewModel : ExtensionObjectViewModel
             var propName = args.PropertyName;
             FetchProperty(propName);
         }
-        catch (Exception)
+        catch (Exception e)
         {
-            // TODO log? throw?
+            ShowException(e);
         }
     }
 
@@ -119,6 +124,10 @@ public partial class PageViewModel : ExtensionObjectViewModel
         {
             case nameof(Name):
                 this.Name = model.Name ?? string.Empty;
+                UpdateProperty(nameof(Title));
+                break;
+            case nameof(Title):
+                this.Title = model.Title ?? string.Empty;
                 break;
             case nameof(IsLoading):
                 this.IsLoading = model.IsLoading;
@@ -131,18 +140,22 @@ public partial class PageViewModel : ExtensionObjectViewModel
         UpdateProperty(propertyName);
     }
 
-    protected void UpdateProperty(string propertyName) => Task.Factory.StartNew(() => { OnPropertyChanged(propertyName); }, CancellationToken.None, TaskCreationOptions.None, Scheduler);
-
-    protected void ShowException(Exception ex)
+    public void ShowException(Exception ex)
     {
         Task.Factory.StartNew(
             () =>
         {
-            ErrorMessage = $"{ex.Message}\n{ex.Source}\n{ex.StackTrace}\n\nThis is due to a bug in the extension's code.";
-            WeakReferenceMessenger.Default.Send<ShowExceptionMessage>(new(ex));
+            ErrorMessage += $"{ex.Message}\n{ex.Source}\n{ex.StackTrace}\n\nThis is due to a bug in the extension's code.";
         },
             CancellationToken.None,
             TaskCreationOptions.None,
             Scheduler);
     }
 }
+
+public interface IPageContext
+{
+    public void ShowException(Exception ex);
+
+    public TaskScheduler Scheduler { get; }
+}

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TagViewModel.cs

@@ -7,7 +7,7 @@ using Microsoft.CmdPal.UI.ViewModels.Models;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class TagViewModel(ITag _tag, TaskScheduler Scheduler) : ExtensionObjectViewModel
+public partial class TagViewModel(ITag _tag, IPageContext context) : ExtensionObjectViewModel(context)
 {
     private readonly ExtensionObject<ITag> _tagModel = new(_tag);
 
@@ -44,6 +44,4 @@ public partial class TagViewModel(ITag _tag, TaskScheduler Scheduler) : Extensio
         UpdateProperty(nameof(Tooltip));
         UpdateProperty(nameof(Icon));
     }
-
-    protected void UpdateProperty(string propertyName) => Task.Factory.StartNew(() => { OnPropertyChanged(propertyName); }, CancellationToken.None, TaskCreationOptions.None, Scheduler);
 }

src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ActionBar.xaml

@@ -72,7 +72,7 @@
             Grid.Column="1"
             VerticalAlignment="Center"
             FontSize="12"
-            Text="{x:Bind CurrentPageViewModel.Name, Mode=OneWay}" />
+            Text="{x:Bind CurrentPageViewModel.Title, Mode=OneWay}" />
         <!--  TO DO: Replace with ItemsRepeater and bind "Primary commands"?  -->
         <StackPanel
             Grid.Column="2"

src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs

@@ -20,8 +20,7 @@ namespace Microsoft.CmdPal.UI;
 public sealed partial class ListPage : Page,
     IRecipient<NavigateNextCommand>,
     IRecipient<NavigatePreviousCommand>,
-    IRecipient<ActivateSelectedListItemMessage>,
-    IRecipient<ShowExceptionMessage>
+    IRecipient<ActivateSelectedListItemMessage>
 {
     private readonly DispatcherQueue _queue = DispatcherQueue.GetForCurrentThread();
 
@@ -33,7 +32,7 @@ public sealed partial class ListPage : Page,
 
     // Using a DependencyProperty as the backing store for ViewModel.  This enables animation, styling, binding, etc...
     public static readonly DependencyProperty ViewModelProperty =
-        DependencyProperty.Register(nameof(ViewModel), typeof(ListViewModel), typeof(ListPage), new PropertyMetadata(null));
+        DependencyProperty.Register(nameof(ViewModel), typeof(ListViewModel), typeof(ListPage), new PropertyMetadata(null, OnViewModelChanged));
 
     public ViewModelLoadedState LoadedState
     {
@@ -107,7 +106,6 @@ public sealed partial class ListPage : Page,
         WeakReferenceMessenger.Default.Register<NavigateNextCommand>(this);
         WeakReferenceMessenger.Default.Register<NavigatePreviousCommand>(this);
         WeakReferenceMessenger.Default.Register<ActivateSelectedListItemMessage>(this);
-        WeakReferenceMessenger.Default.Register<ShowExceptionMessage>(this);
 
         base.OnNavigatedTo(e);
     }
@@ -119,9 +117,9 @@ public sealed partial class ListPage : Page,
         WeakReferenceMessenger.Default.Unregister<NavigateNextCommand>(this);
         WeakReferenceMessenger.Default.Unregister<NavigatePreviousCommand>(this);
         WeakReferenceMessenger.Default.Unregister<ActivateSelectedListItemMessage>(this);
-        WeakReferenceMessenger.Default.Unregister<ShowExceptionMessage>(this);
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "VS is too agressive at pruning methods bound in XAML")]
     private void ListView_ItemClick(object sender, ItemClickEventArgs e)
     {
         if (e.ClickedItem is ListItemViewModel item)
@@ -130,6 +128,15 @@ public sealed partial class ListPage : Page,
         }
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "VS is too agressive at pruning methods bound in XAML")]
+    private void ItemsList_SelectionChanged(object sender, SelectionChangedEventArgs e)
+    {
+        if (ItemsList.SelectedItem is ListItemViewModel item)
+        {
+            ViewModel?.UpdateSelectedItemCommand.Execute(item);
+        }
+    }
+
     public void Receive(NavigateNextCommand message)
     {
         // Note: We may want to just have the notion of a 'SelectedCommand' in our VM
@@ -160,20 +167,32 @@ public sealed partial class ListPage : Page,
         }
     }
 
-    private void ItemsList_SelectionChanged(object sender, SelectionChangedEventArgs e)
+    private static void OnViewModelChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
     {
-        if (ItemsList.SelectedItem is ListItemViewModel item)
+        if (d is ListPage @this)
         {
-            ViewModel?.UpdateSelectedItemCommand.Execute(item);
+            if (e.OldValue is ListViewModel old)
+            {
+                old.PropertyChanged -= @this.ViewModel_PropertyChanged;
+            }
+
+            if (e.NewValue is ListViewModel page)
+            {
+                page.PropertyChanged += @this.ViewModel_PropertyChanged;
+            }
         }
     }
 
-    public void Receive(ShowExceptionMessage message)
+    private void ViewModel_PropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e)
     {
-        _ = _queue.EnqueueAsync(() =>
+        var prop = e.PropertyName;
+        if (prop == nameof(ViewModel.ErrorMessage) && ViewModel != null)
         {
-            LoadedState = ViewModelLoadedState.Error;
-        });
+            if (!string.IsNullOrEmpty(ViewModel.ErrorMessage))
+            {
+                LoadedState = ViewModelLoadedState.Error;
+            }
+        }
     }
 }
 

src/modules/cmdpal/extensionsdk/Microsoft.CmdPal.Extensions.Helpers/Page.cs

@@ -10,7 +10,7 @@ public partial class Page : Command, IPage
     private string _title = string.Empty;
     private OptionalColor _accentColor;
 
-    public bool IsLoading
+    public virtual bool IsLoading
     {
         get => _loading;
         set
@@ -20,17 +20,17 @@ public partial class Page : Command, IPage
         }
     }
 
-    public string Title
+    public virtual string Title
     {
         get => _title;
         set
         {
             _title = value;
-            OnPropertyChanged(nameof(Name));
+            OnPropertyChanged(nameof(Title));
         }
     }
 
-    public OptionalColor AccentColor
+    public virtual OptionalColor AccentColor
     {
         get => _accentColor;
         set