diff --git a/Docs/architecture/error-reporting.md b/Docs/architecture/error-reporting.md index 252af7d..c74ec24 100644 --- a/Docs/architecture/error-reporting.md +++ b/Docs/architecture/error-reporting.md @@ -46,8 +46,28 @@ read `Errors[0]` directly. discrete one-shot channel. `RecipeCoordinator` bridges each fault to the operation channel via `ReportError(error.Message)` — the message text is owned in Core. The persistent "disconnected" label stays in the status bar. -- **Exception handlers** (`ReportError($"... {ex.Message}")` on `ThrownExceptions`) are not - `Result`-based and stay as-is. +- **Command exceptions** (a `ReactiveCommand` fault on `ThrownExceptions`) are not `Result`-based; + they route through `ReportThrownExceptions` (see below) rather than a hand-written handler. - **One deliberate exception** to the `"; "` idiom: the clipboard *paste* failure lists each error on its own line (`Environment.NewLine`), because a rejected paste can carry many per-step errors that read better stacked. It still surfaces every error (never just `Errors[0]`). + +## Command-exception pipe (report + log) + +A command whose `Execute` throws (file I/O, PLC calls) faults on `ReactiveCommand.ThrownExceptions`. +These faults route through one extension: + +- `IDisposable ReportThrownExceptions(this ReactiveCommand, MessagePanelViewModel panel, ILogger logger, string context)` + (`SemiStep.UI/MessageService/ReactiveCommandReportingExtensions.cs`). Per thrown exception it both + `logger.LogError(ex, "{Context} failed", context)` **and** `panel.ReportError($"{context}: {ex.Message}")`. + The panel keeps the user-facing message it always showed; the log now carries the exception type and + full stack that the message drops. + +The `logger` argument is the caller's own `ILogger`, so the Serilog `{SourceContext}` field names +the originating view model in every logged fault. The extension takes the concrete +`MessagePanelViewModel`, not an `IMessageSink` abstraction — one panel implementation exists, so the +seam stays concrete (matches `ResultReportingExtensions`). + +Modal dialogs are the exception: `GridStyleEditorViewModel.SaveCommand` surfaces its fault on the +editor's own `ErrorMessage` property (and logs), not the shared panel, because a modal owns its error +surface while it is open. diff --git a/Docs/architecture/headless-ui-testing.md b/Docs/architecture/headless-ui-testing.md index a8e4ada..7a23381 100644 --- a/Docs/architecture/headless-ui-testing.md +++ b/Docs/architecture/headless-ui-testing.md @@ -17,8 +17,8 @@ stand-in. See `SemiStep.Tests/UI/MainWindow/MainWindowExitFlowTests.cs` for a wo `RunJobs` remains necessary only to pump work the test never awaits: - fire-and-forget `async void` continuations (e.g. a `Closing` event handler that awaits a dialog); -- `ObserveOn(RxApp.MainThreadScheduler)` deliveries, such as `ThrownExceptions` reports posted to - the message panel. +- `ObserveOn(RxApp.MainThreadScheduler)` deliveries, such as the `MainWindowViewModel` sync-time + `Observable.Interval` tick that reposts `LastSyncTimeText` on the UI scheduler. Call `Dispatcher.UIThread.RunJobs()` after triggering such work and before asserting on its effect. diff --git a/SemiStep/SemiStep.Core/Configuration/GridStyleEditorFacade.cs b/SemiStep/SemiStep.Core/Configuration/GridStyleEditorFacade.cs index 2c24c75..062e734 100644 --- a/SemiStep/SemiStep.Core/Configuration/GridStyleEditorFacade.cs +++ b/SemiStep/SemiStep.Core/Configuration/GridStyleEditorFacade.cs @@ -12,7 +12,7 @@ namespace SemiStep.Core.Configuration; /// row height, spacing, panel height) is the caller's responsibility — the editor view model enforces /// those bounds before invoking . /// -public sealed class GridStyleEditorFacade +public sealed class GridStyleEditorFacade : IGridStyleEditorFacade { private readonly GridStyleWriter _gridStyleWriter = new(); diff --git a/SemiStep/SemiStep.Core/Configuration/IGridStyleEditorFacade.cs b/SemiStep/SemiStep.Core/Configuration/IGridStyleEditorFacade.cs new file mode 100644 index 0000000..6c6abe7 --- /dev/null +++ b/SemiStep/SemiStep.Core/Configuration/IGridStyleEditorFacade.cs @@ -0,0 +1,16 @@ +using FluentResults; + +namespace SemiStep.Core.Configuration; + +/// +/// The style-editor seam consumed by the UI editor view model. Declared as an interface so the +/// view model depends on an abstraction it can mock, rather than the concrete Core facade. +/// +public interface IGridStyleEditorFacade +{ + Task> Load(string configDir); + + Result Validate(GridStyleOptions options); + + Result Save(string configDir, GridStyleOptions options); +} diff --git a/SemiStep/SemiStep.Tests/UI/Clipboard/ClipboardViewModelCanExecuteTests.cs b/SemiStep/SemiStep.Tests/UI/Clipboard/ClipboardViewModelCanExecuteTests.cs index d5d5cbb..30c0dfd 100644 --- a/SemiStep/SemiStep.Tests/UI/Clipboard/ClipboardViewModelCanExecuteTests.cs +++ b/SemiStep/SemiStep.Tests/UI/Clipboard/ClipboardViewModelCanExecuteTests.cs @@ -4,6 +4,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using SemiStep.Core.Recipes.Clipboard; using SemiStep.Core.Recipes.Helpers; using SemiStep.Tests.Core.Helpers; @@ -39,7 +41,8 @@ public async ValueTask InitializeAsync() _surface, clipboardSerializer, importedRecipeValidator, - _fixture.MessagePanel); + _fixture.MessagePanel, + NullLogger.Instance); } public async ValueTask DisposeAsync() @@ -113,7 +116,8 @@ public void Cut_CanExecuteTrue_WhenSelectionExistsBeforeConstruction() _surface, new ClipboardSerializer(_fixture.RecipeMetadataRegistry), new ImportedRecipeValidator(_fixture.RecipeMetadataRegistry), - _fixture.MessagePanel); + _fixture.MessagePanel, + NullLogger.Instance); ((ICommand)clipboard.CutStepCommand).CanExecute(null).Should().BeTrue(); } diff --git a/SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs b/SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs index 5643ce2..b06df92 100644 --- a/SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs +++ b/SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs @@ -140,7 +140,8 @@ public void SeedRecipe(int stepCount) } public MainWindowViewModel CreateMainWindowViewModel( - Func? styleEditorFactory = null) + Func? styleEditorFactory = null, + ILogger? logger = null) { var grid = CreateActiveSurface(); @@ -153,9 +154,13 @@ public MainWindowViewModel CreateMainWindowViewModel( grid, clipboardSerializer, importedRecipeValidator, - MessagePanel); + MessagePanel, + NullLogger.Instance); - var recipeFile = new RecipeFileViewModel(Coordinator, MessagePanel); + var recipeFile = new RecipeFileViewModel( + Coordinator, + MessagePanel, + NullLogger.Instance); var plcMonitor = new PlcMonitorViewModel( Coordinator, @@ -166,7 +171,8 @@ public MainWindowViewModel CreateMainWindowViewModel( () => new GridStyleEditorViewModel( new GridStyleEditorFacade(), @"C:\does-not-exist", - AppConfiguration.GridStyle)); + AppConfiguration.GridStyle, + NullLogger.Instance)); return new MainWindowViewModel( Coordinator, @@ -177,7 +183,7 @@ public MainWindowViewModel CreateMainWindowViewModel( MessagePanel, plcMonitor, gridStyleEditorViewModelFactory, - NullLogger.Instance); + logger ?? NullLogger.Instance); } public void SetSyncEnabled(bool isSyncEnabled) diff --git a/SemiStep/SemiStep.Tests/UI/MainWindow/MainWindowViewModelReportingTests.cs b/SemiStep/SemiStep.Tests/UI/MainWindow/MainWindowViewModelReportingTests.cs new file mode 100644 index 0000000..45f2def --- /dev/null +++ b/SemiStep/SemiStep.Tests/UI/MainWindow/MainWindowViewModelReportingTests.cs @@ -0,0 +1,73 @@ +using System; + +using Avalonia.Headless.XUnit; + +using FluentAssertions; + +using Microsoft.Extensions.Logging; + +using SemiStep.Tests.Helpers; +using SemiStep.Tests.UI.Helpers; +using SemiStep.UI.MainWindow; +using SemiStep.UI.MessageService; + +using Xunit; + +namespace SemiStep.Tests.UI.MainWindow; + +[Trait("Component", "UI")] +[Trait("Area", "MessageReporting")] +[Trait("Category", "Unit")] +public sealed class MainWindowViewModelReportingTests : IAsyncLifetime +{ + private readonly UIFixture _fixture = new(); + private readonly RecordingLogger _logger = new(); + private MainWindowViewModel _viewModel = null!; + + public async ValueTask InitializeAsync() + { + await _fixture.InitializeAsync(); + _viewModel = _fixture.CreateMainWindowViewModel(logger: _logger); + } + + public ValueTask DisposeAsync() + { + _viewModel.Dispose(); + return _fixture.DisposeAsync(); + } + + [AvaloniaTheory] + [InlineData("PLC state update")] + [InlineData("PLC conflict handling")] + [InlineData("Sync time refresh")] + public void OnSubscriptionError_ReportsToPanelAndLogsException(string context) + { + var failure = new InvalidOperationException("boom"); + + _viewModel.OnSubscriptionError(context)(failure); + + _fixture.MessagePanel.Entries.Should().ContainSingle( + entry => entry.Severity == MessageSeverity.Error && entry.Message == $"{context}: boom"); + + var logged = _logger.Entries.Should().ContainSingle().Subject; + logged.Level.Should().Be(LogLevel.Error); + logged.Exception.Should().BeSameAs(failure); + } + + [AvaloniaFact] + public void Guarded_ThrowInOnNextBody_ReportsToPanelAndLogs_WithoutEscaping() + { + var failure = new InvalidOperationException("boom"); + + var act = () => _viewModel.Guarded("Sync time refresh", () => throw failure); + + act.Should().NotThrow("a throw in the onNext body must be contained, not propagated to the pipeline"); + + _fixture.MessagePanel.Entries.Should().ContainSingle( + entry => entry.Severity == MessageSeverity.Error && entry.Message == "Sync time refresh: boom"); + + var logged = _logger.Entries.Should().ContainSingle().Subject; + logged.Level.Should().Be(LogLevel.Error); + logged.Exception.Should().BeSameAs(failure); + } +} diff --git a/SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs b/SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs index fdfbe5d..6f5df01 100644 --- a/SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs +++ b/SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs @@ -7,6 +7,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using ReactiveUI; using SemiStep.Core.Recipes.Clipboard; @@ -47,7 +49,10 @@ public async ValueTask DisposeAsync() [AvaloniaFact] public async Task RecipeFile_Save_ReportsSuccess() { - var recipeFile = new RecipeFileViewModel(_fixture.Coordinator, _fixture.MessagePanel); + var recipeFile = new RecipeFileViewModel( + _fixture.Coordinator, + _fixture.MessagePanel, + NullLogger.Instance); var filePath = Path.Combine(Path.GetTempPath(), $"semistep-save-{Guid.NewGuid():N}.csv"); recipeFile.SaveFileInteraction.RegisterHandler(context => context.SetOutput(filePath)); @@ -69,7 +74,10 @@ public async Task RecipeFile_Save_ReportsSuccess() [AvaloniaFact] public async Task RecipeFile_Load_ReportsSuccess() { - var recipeFile = new RecipeFileViewModel(_fixture.Coordinator, _fixture.MessagePanel); + var recipeFile = new RecipeFileViewModel( + _fixture.Coordinator, + _fixture.MessagePanel, + NullLogger.Instance); var filePath = Path.Combine(Path.GetTempPath(), $"semistep-load-{Guid.NewGuid():N}.csv"); recipeFile.SaveFileInteraction.RegisterHandler(context => context.SetOutput(filePath)); recipeFile.OpenFileInteraction.RegisterHandler(context => context.SetOutput(filePath)); @@ -94,7 +102,10 @@ public async Task RecipeFile_Load_ReportsSuccess() [AvaloniaFact] public async Task RecipeFile_LoadMissingFile_ReportsError() { - var recipeFile = new RecipeFileViewModel(_fixture.Coordinator, _fixture.MessagePanel); + var recipeFile = new RecipeFileViewModel( + _fixture.Coordinator, + _fixture.MessagePanel, + NullLogger.Instance); var missingPath = Path.Combine(Path.GetTempPath(), $"semistep-missing-{Guid.NewGuid():N}.csv"); recipeFile.OpenFileInteraction.RegisterHandler(context => context.SetOutput(missingPath)); @@ -149,7 +160,8 @@ public async Task Clipboard_PasteInvalidContent_ReportsError() _surface, clipboardSerializer, importedRecipeValidator, - _fixture.MessagePanel); + _fixture.MessagePanel, + NullLogger.Instance); var window = new Window(); window.Show(); @@ -187,7 +199,8 @@ public async Task Clipboard_PasteValidContent_LeavesNoOperationError() _surface, clipboardSerializer, importedRecipeValidator, - _fixture.MessagePanel); + _fixture.MessagePanel, + NullLogger.Instance); var window = new Window(); window.Show(); diff --git a/SemiStep/SemiStep.Tests/UI/MessageService/ReactiveCommandReportingExtensionsTests.cs b/SemiStep/SemiStep.Tests/UI/MessageService/ReactiveCommandReportingExtensionsTests.cs new file mode 100644 index 0000000..9160181 --- /dev/null +++ b/SemiStep/SemiStep.Tests/UI/MessageService/ReactiveCommandReportingExtensionsTests.cs @@ -0,0 +1,88 @@ +using System; +using System.Reactive; +using System.Reactive.Linq; + +using Avalonia.Headless.XUnit; + +using FluentAssertions; + +using Microsoft.Extensions.Logging; + +using ReactiveUI; + +using SemiStep.Tests.Helpers; +using SemiStep.UI.MessageService; + +using Xunit; + +namespace SemiStep.Tests.UI.MessageService; + +[Trait("Component", "UI")] +[Trait("Area", "MessageReporting")] +[Trait("Category", "Unit")] +public sealed class ReactiveCommandReportingExtensionsTests +{ + [AvaloniaFact] + public async Task ReportThrownExceptions_ThrowingCommand_ReportsSinglePanelErrorAndLogsException() + { + var panel = new MessagePanelViewModel(); + var logger = new RecordingLogger(); + var failure = new InvalidOperationException("boom"); + var command = ReactiveCommand.Create(_ => throw failure); + + using var subscription = command.ReportThrownExceptions(panel, logger, "Copy failed"); + + try + { + await ExecuteSwallowing(command); + + var entry = panel.Entries.Should().ContainSingle().Subject; + entry.Severity.Should().Be(MessageSeverity.Error); + entry.Message.Should().Be("Copy failed: boom"); + + var logged = logger.Entries.Should().ContainSingle().Subject; + logged.Level.Should().Be(LogLevel.Error); + logged.Exception.Should().BeSameAs(failure); + } + finally + { + command.Dispose(); + panel.Dispose(); + } + } + + [AvaloniaFact] + public async Task ReportThrownExceptions_NonThrowingCommand_ProducesNoPanelEntryAndNoLog() + { + var panel = new MessagePanelViewModel(); + var logger = new RecordingLogger(); + var command = ReactiveCommand.Create(_ => Unit.Default); + + using var subscription = command.ReportThrownExceptions(panel, logger, "Copy failed"); + + try + { + await command.Execute(); + + panel.Entries.Should().BeEmpty(); + logger.Entries.Should().BeEmpty(); + } + finally + { + command.Dispose(); + panel.Dispose(); + } + } + + private static async Task ExecuteSwallowing(ReactiveCommand command) + { + try + { + await command.Execute(); + } + catch (InvalidOperationException) + { + // The command routes the throw to ThrownExceptions; Execute also rethrows to the awaiter. + } + } +} diff --git a/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelCanExecuteTests.cs b/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelCanExecuteTests.cs index 8e982b5..f5748b8 100644 --- a/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelCanExecuteTests.cs +++ b/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelCanExecuteTests.cs @@ -4,6 +4,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using SemiStep.Tests.UI.Helpers; using SemiStep.UI.RecipeFile; @@ -23,7 +25,10 @@ public sealed class RecipeFileViewModelCanExecuteTests : IAsyncLifetime public async ValueTask InitializeAsync() { await _fixture.InitializeAsync(); - _recipeFile = new RecipeFileViewModel(_fixture.Coordinator, _fixture.MessagePanel); + _recipeFile = new RecipeFileViewModel( + _fixture.Coordinator, + _fixture.MessagePanel, + NullLogger.Instance); } public async ValueTask DisposeAsync() diff --git a/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelSaveResultTests.cs b/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelSaveResultTests.cs index a21ddc4..81f900a 100644 --- a/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelSaveResultTests.cs +++ b/SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelSaveResultTests.cs @@ -5,6 +5,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using SemiStep.Tests.Core.Helpers; using SemiStep.Tests.UI.Helpers; @@ -28,7 +30,10 @@ public sealed class RecipeFileViewModelSaveResultTests : IAsyncLifetime public async ValueTask InitializeAsync() { await _fixture.InitializeAsync(); - _recipeFile = new RecipeFileViewModel(_fixture.Coordinator, _fixture.MessagePanel); + _recipeFile = new RecipeFileViewModel( + _fixture.Coordinator, + _fixture.MessagePanel, + NullLogger.Instance); } public async ValueTask DisposeAsync() @@ -166,7 +171,7 @@ public async Task SaveAsRecipe_ThrowingPicker_ReportsSaveAsFailed() var saveAs = async () => await _recipeFile.SaveAsRecipeCommand.Execute(); await saveAs.Should().ThrowAsync(); - // The ThrownExceptions report is posted to the dispatcher via ObserveOn. + // The report reaches the panel via MessagePanel's own UI-thread marshalling; drain any queued work. Dispatcher.UIThread.RunJobs(); var errorEntry = _fixture.MessagePanel.Entries .Should().ContainSingle(e => e.Severity == MessageSeverity.Error).Subject; diff --git a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorViewModelTests.cs b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorViewModelTests.cs index b0c5422..d634d3e 100644 --- a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorViewModelTests.cs +++ b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorViewModelTests.cs @@ -1,9 +1,21 @@ -using Avalonia.Headless.XUnit; +using System; +using System.Reactive; +using System.Reactive.Linq; + +using Avalonia.Headless.XUnit; using Avalonia.Media; using FluentAssertions; +using FluentResults; + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +using ReactiveUI; + using SemiStep.Core.Configuration; +using SemiStep.Tests.Helpers; using SemiStep.UI.Localization; using SemiStep.UI.StyleEditor; @@ -247,8 +259,82 @@ public async Task LoadAsync_MissingConfigDir_SetsErrorMessageAndDoesNotReseed() viewModel.CellFontSize.Should().Be(18); } + [AvaloniaFact] + public async Task SaveCommand_WhenSaveThrows_SurfacesErrorMessageAndLogs_WithoutCrashing() + { + var logger = new RecordingLogger(); + var failure = new InvalidOperationException("disk gone"); + var viewModel = new GridStyleEditorViewModel( + new ThrowingFacade(failure), + ConfigDir, + GridStyleOptions.Default, + logger); + + viewModel.CanSave.Should().BeTrue("the seeded default draft is valid, so SaveCommand can execute"); + + await ExecuteSwallowing(viewModel.SaveCommand); + + viewModel.ErrorMessage.Should().Be("Save failed: disk gone"); + var logged = logger.Entries.Should().ContainSingle().Subject; + logged.Level.Should().Be(LogLevel.Error); + logged.Exception.Should().BeSameAs(failure); + } + + [AvaloniaFact] + public void ReportSaveException_SurfacesOnEditorErrorMessage_AndLogsWithException() + { + var logger = new RecordingLogger(); + var viewModel = new GridStyleEditorViewModel( + new GridStyleEditorFacade(), + ConfigDir, + GridStyleOptions.Default, + logger); + var failure = new InvalidOperationException("boom"); + + viewModel.ReportSaveException(failure); + + viewModel.ErrorMessage.Should().Be("Save failed: boom"); + var logged = logger.Entries.Should().ContainSingle().Subject; + logged.Level.Should().Be(LogLevel.Error); + logged.Exception.Should().BeSameAs(failure); + } + private static GridStyleEditorViewModel CreateViewModel(GridStyleOptions source) { - return new GridStyleEditorViewModel(new GridStyleEditorFacade(), ConfigDir, source); + return new GridStyleEditorViewModel( + new GridStyleEditorFacade(), + ConfigDir, + source, + NullLogger.Instance); + } + + private static async Task ExecuteSwallowing(ReactiveCommand command) + { + try + { + await command.Execute(); + } + catch (InvalidOperationException) + { + // The command routes the throw to ThrownExceptions; Execute also rethrows to the awaiter. + } + } + + private sealed class ThrowingFacade(Exception failure) : IGridStyleEditorFacade + { + public Task> Load(string configDir) + { + return Task.FromResult(Result.Ok(GridStyleOptions.Default)); + } + + public Result Validate(GridStyleOptions options) + { + return Result.Ok(); + } + + public Result Save(string configDir, GridStyleOptions options) + { + throw failure; + } } } diff --git a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowOwnerRoutingTests.cs b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowOwnerRoutingTests.cs index 8da0d70..2bdf4aa 100644 --- a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowOwnerRoutingTests.cs +++ b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowOwnerRoutingTests.cs @@ -5,6 +5,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using SemiStep.Core.Configuration; using SemiStep.Tests.Config.Helpers; @@ -115,7 +117,11 @@ public async Task SaveThenExitNow_DrivesRealGlue_CascadesToOwnerClose() using var tempDir = CopyShippedConfig("MBE"); var facade = new GridStyleEditorFacade(); var loaded = (await facade.Load(tempDir.Path)).Value; - var viewModel = new GridStyleEditorViewModel(facade, tempDir.Path, loaded); + var viewModel = new GridStyleEditorViewModel( + facade, + tempDir.Path, + loaded, + NullLogger.Instance); var editor = new GridStyleEditorWindow { ViewModel = viewModel }; // Modal show establishes the Owner relationship and fires the WhenActivated glue that diff --git a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowTests.cs b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowTests.cs index 89e9758..2fe116d 100644 --- a/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowTests.cs +++ b/SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowTests.cs @@ -5,6 +5,8 @@ using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + using SemiStep.Core.Configuration; using SemiStep.Tests.Config.Helpers; using SemiStep.Tests.Helpers; @@ -28,7 +30,11 @@ public async Task Save_AfterEditingColorAndNumeric_PersistsToYaml_AndReloads() var configDir = tempDir.Path; var loaded = (await facade.Load(configDir)).Value; - var viewModel = new GridStyleEditorViewModel(facade, configDir, loaded); + var viewModel = new GridStyleEditorViewModel( + facade, + configDir, + loaded, + NullLogger.Instance); viewModel.SelectionBackground = Color.Parse("#123456"); viewModel.CellFontSize = loaded.CellFontSize + 1; @@ -56,7 +62,11 @@ public async Task NoEditThenDiscard_LeavesFileByteIdentical() var loaded = (await facade.Load(configDir)).Value; // Construct and discard the VM without invoking Save (mirrors the Cancel path). - _ = new GridStyleEditorViewModel(facade, configDir, loaded); + _ = new GridStyleEditorViewModel( + facade, + configDir, + loaded, + NullLogger.Instance); var after = await File.ReadAllBytesAsync(filePath, TestContext.Current.CancellationToken); after.Should().Equal(before); diff --git a/SemiStep/SemiStep.UI/Clipboard/ClipboardViewModel.cs b/SemiStep/SemiStep.UI/Clipboard/ClipboardViewModel.cs index b41bc2e..e271d4e 100644 --- a/SemiStep/SemiStep.UI/Clipboard/ClipboardViewModel.cs +++ b/SemiStep/SemiStep.UI/Clipboard/ClipboardViewModel.cs @@ -8,6 +8,8 @@ using FluentResults; +using Microsoft.Extensions.Logging; + using ReactiveUI; using SemiStep.Core.Recipes; @@ -27,6 +29,7 @@ public class ClipboardViewModel : ReactiveObject, IDisposable private readonly CompositeDisposable _disposables = new(); private readonly ImportedRecipeValidator _importedRecipeValidator; + private readonly ILogger _logger; private readonly MessagePanelViewModel _messagePanel; private readonly IRecipeGridSurface _recipeGrid; private IClipboard? _clipboard; @@ -36,13 +39,15 @@ public ClipboardViewModel( IRecipeGridSurface recipeGrid, ClipboardSerializer clipboardSerializer, ImportedRecipeValidator importedRecipeValidator, - MessagePanelViewModel messagePanel) + MessagePanelViewModel messagePanel, + ILogger logger) { _coordinator = coordinator; _recipeGrid = recipeGrid; _clipboardSerializer = clipboardSerializer; _importedRecipeValidator = importedRecipeValidator; _messagePanel = messagePanel; + _logger = logger; var canCopyOrCut = _recipeGrid.CanDeleteStep; var canEdit = _coordinator.CanEditRecipe; @@ -53,19 +58,13 @@ public ClipboardViewModel( canEdit.CombineLatest(canCopyOrCut, (left, right) => left && right)); PasteStepCommand = ReactiveCommand.CreateFromTask(PasteStepsAsync, canEdit); - CopyStepCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Copy failed: {ex.Message}")) + CopyStepCommand.ReportThrownExceptions(_messagePanel, _logger, "Copy failed") .DisposeWith(_disposables); - CutStepCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Cut failed: {ex.Message}")) + CutStepCommand.ReportThrownExceptions(_messagePanel, _logger, "Cut failed") .DisposeWith(_disposables); - PasteStepCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Paste failed: {ex.Message}")) + PasteStepCommand.ReportThrownExceptions(_messagePanel, _logger, "Paste failed") .DisposeWith(_disposables); } diff --git a/SemiStep/SemiStep.UI/MainWindow/MainWindowViewModel.cs b/SemiStep/SemiStep.UI/MainWindow/MainWindowViewModel.cs index 0ab3008..ad23a6b 100644 --- a/SemiStep/SemiStep.UI/MainWindow/MainWindowViewModel.cs +++ b/SemiStep/SemiStep.UI/MainWindow/MainWindowViewModel.cs @@ -68,30 +68,32 @@ public MainWindowViewModel( .ToProperty(this, x => x.IsTransposedOrientation) .DisposeWith(_disposables); - ToggleSyncCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => messagePanel.ReportError($"Sync toggle failed: {ex.Message}")) + ToggleSyncCommand.ReportThrownExceptions(MessagePanel, _logger, "Sync toggle failed") .DisposeWith(_disposables); - OpenStyleEditorCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => messagePanel.ReportError($"Style editor failed: {ex.Message}")) + OpenStyleEditorCommand.ReportThrownExceptions(MessagePanel, _logger, "Style editor failed") .DisposeWith(_disposables); _coordinator.Mutated += OnCoordinatorMutated; _disposables.Add(Disposable.Create(() => _coordinator.Mutated -= OnCoordinatorMutated)); _coordinator.PlcStateChanged - .Subscribe(_ => RaiseConnectionStateProperties()) + .Subscribe( + _ => Guarded("PLC state update", RaiseConnectionStateProperties), + OnSubscriptionError("PLC state update")) .DisposeWith(_disposables); _coordinator.PlcRecipeConflictDetected - .Subscribe(conflict => _ = HandleConflictAsync(conflict.Local, conflict.Plc)) + .Subscribe( + conflict => Guarded("PLC conflict handling", () => _ = HandleConflictAsync(conflict.Local, conflict.Plc)), + OnSubscriptionError("PLC conflict handling")) .DisposeWith(_disposables); Observable.Interval(TimeSpan.FromSeconds(1)) .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(_ => this.RaisePropertyChanged(nameof(LastSyncTimeText))) + .Subscribe( + _ => Guarded("Sync time refresh", () => this.RaisePropertyChanged(nameof(LastSyncTimeText))), + OnSubscriptionError("Sync time refresh")) .DisposeWith(_disposables); } @@ -249,6 +251,26 @@ private void OnCoordinatorMutated(MutationSignal signal) RaiseAllStateProperties(); } + internal Action OnSubscriptionError(string context) + { + return ex => ExceptionReporter.ReportAndLog(MessagePanel, _logger, context, ex); + } + + // A throw inside a subscription's onNext body is NOT routed to onError: Rx disposes the + // subscription and rethrows up the pipeline (fatal for a dispatcher-scheduled tick). Wrapping + // the body here contains it on the same report + log path as a source-observable error. + internal void Guarded(string context, Action body) + { + try + { + body(); + } + catch (Exception ex) + { + ExceptionReporter.ReportAndLog(MessagePanel, _logger, context, ex); + } + } + private void RaiseAllStateProperties() { this.RaisePropertyChanged(nameof(WindowTitle)); diff --git a/SemiStep/SemiStep.UI/MessageService/ExceptionReporter.cs b/SemiStep/SemiStep.UI/MessageService/ExceptionReporter.cs new file mode 100644 index 0000000..9c3aad7 --- /dev/null +++ b/SemiStep/SemiStep.UI/MessageService/ExceptionReporter.cs @@ -0,0 +1,14 @@ +using System; + +using Microsoft.Extensions.Logging; + +namespace SemiStep.UI.MessageService; + +internal static class ExceptionReporter +{ + public static void ReportAndLog(MessagePanelViewModel panel, ILogger logger, string context, Exception exception) + { + logger.LogError(exception, "{Context}", context); + panel.ReportError($"{context}: {exception.Message}"); + } +} diff --git a/SemiStep/SemiStep.UI/MessageService/ReactiveCommandReportingExtensions.cs b/SemiStep/SemiStep.UI/MessageService/ReactiveCommandReportingExtensions.cs new file mode 100644 index 0000000..379374f --- /dev/null +++ b/SemiStep/SemiStep.UI/MessageService/ReactiveCommandReportingExtensions.cs @@ -0,0 +1,24 @@ +using System; + +using Microsoft.Extensions.Logging; + +using ReactiveUI; + +namespace SemiStep.UI.MessageService; + +public static class ReactiveCommandReportingExtensions +{ + public static IDisposable ReportThrownExceptions( + this ReactiveCommand command, + MessagePanelViewModel panel, + ILogger logger, + string context) + { + ArgumentNullException.ThrowIfNull(command); + ArgumentNullException.ThrowIfNull(panel); + ArgumentNullException.ThrowIfNull(logger); + ArgumentNullException.ThrowIfNull(context); + + return command.ThrownExceptions.Subscribe(ex => ExceptionReporter.ReportAndLog(panel, logger, context, ex)); + } +} diff --git a/SemiStep/SemiStep.UI/RecipeFile/RecipeFileViewModel.cs b/SemiStep/SemiStep.UI/RecipeFile/RecipeFileViewModel.cs index af393b9..40b8d49 100644 --- a/SemiStep/SemiStep.UI/RecipeFile/RecipeFileViewModel.cs +++ b/SemiStep/SemiStep.UI/RecipeFile/RecipeFileViewModel.cs @@ -3,6 +3,8 @@ using System.Reactive.Disposables.Fluent; using System.Reactive.Linq; +using Microsoft.Extensions.Logging; + using ReactiveUI; using SemiStep.UI.Coordinator; @@ -15,14 +17,17 @@ public class RecipeFileViewModel : ReactiveObject, IDisposable private readonly RecipeCoordinator _coordinator; private readonly CompositeDisposable _disposables = new(); + private readonly ILogger _logger; private readonly MessagePanelViewModel _messagePanel; public RecipeFileViewModel( RecipeCoordinator coordinator, - MessagePanelViewModel messagePanel) + MessagePanelViewModel messagePanel, + ILogger logger) { _coordinator = coordinator; _messagePanel = messagePanel; + _logger = logger; OpenFileInteraction = new Interaction(); SaveFileInteraction = new Interaction(); @@ -34,19 +39,13 @@ public RecipeFileViewModel( LoadRecipeCommand = ReactiveCommand.CreateFromTask(LoadRecipeAsync, canEdit); NewRecipeCommand = ReactiveCommand.Create(NewRecipe, canEdit); - SaveRecipeCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Save failed: {ex.Message}")) + SaveRecipeCommand.ReportThrownExceptions(_messagePanel, _logger, "Save failed") .DisposeWith(_disposables); - SaveAsRecipeCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Save As failed: {ex.Message}")) + SaveAsRecipeCommand.ReportThrownExceptions(_messagePanel, _logger, "Save As failed") .DisposeWith(_disposables); - LoadRecipeCommand.ThrownExceptions - .ObserveOn(RxSchedulers.MainThreadScheduler) - .Subscribe(ex => _messagePanel.ReportError($"Load failed: {ex.Message}")) + LoadRecipeCommand.ReportThrownExceptions(_messagePanel, _logger, "Load failed") .DisposeWith(_disposables); } diff --git a/SemiStep/SemiStep.UI/StyleEditor/GridStyleEditorViewModel.cs b/SemiStep/SemiStep.UI/StyleEditor/GridStyleEditorViewModel.cs index 26b0344..667f70c 100644 --- a/SemiStep/SemiStep.UI/StyleEditor/GridStyleEditorViewModel.cs +++ b/SemiStep/SemiStep.UI/StyleEditor/GridStyleEditorViewModel.cs @@ -1,10 +1,13 @@ -using System.Reactive; +using System; +using System.Reactive; using System.Runtime.CompilerServices; using Avalonia.Media; using FluentResults; +using Microsoft.Extensions.Logging; + using ReactiveUI; using SemiStep.Core.Configuration; @@ -33,30 +36,41 @@ public sealed class GridStyleEditorViewModel : ReactiveObject public const int MinPanelMaxHeight = 20; public const int MaxPanelMaxHeight = 2000; - private readonly GridStyleEditorFacade _gridStyleEditorFacade; + private readonly IGridStyleEditorFacade _gridStyleEditorFacade; private readonly string _configDir; + private readonly ILogger _logger; private GridStyleOptions _source; private bool _seeding; - public GridStyleEditorViewModel(GridStyleEditorFacade gridStyleEditorFacade, StartupOptions startupOptions) - : this(gridStyleEditorFacade, startupOptions.ConfigDir, GridStyleOptions.Default) + public GridStyleEditorViewModel( + IGridStyleEditorFacade gridStyleEditorFacade, + StartupOptions startupOptions, + ILogger logger) + : this(gridStyleEditorFacade, startupOptions.ConfigDir, GridStyleOptions.Default, logger) { } public GridStyleEditorViewModel( - GridStyleEditorFacade gridStyleEditorFacade, + IGridStyleEditorFacade gridStyleEditorFacade, string configDir, - GridStyleOptions source) + GridStyleOptions source, + ILogger logger) { _gridStyleEditorFacade = gridStyleEditorFacade; _configDir = configDir; _source = source; + _logger = logger; SaveCommand = ReactiveCommand.Create( Save, this.WhenAnyValue(viewModel => viewModel.CanSave)); + // Modal editor: a save fault must surface on the editor's own ErrorMessage, not the + // shared message panel hidden behind the dialog. Subscription lifetime tracks this + // transient dialog VM, which is neither IDisposable nor pooled. + SaveCommand.ThrownExceptions.Subscribe(ReportSaveException); + Seed(source); } @@ -288,6 +302,12 @@ private bool Save() return true; } + internal void ReportSaveException(Exception exception) + { + _logger.LogError(exception, "Style editor save failed"); + ErrorMessage = $"Save failed: {exception.Message}"; + } + private void Seed(GridStyleOptions options) { _seeding = true; diff --git a/SemiStep/SemiStep.UI/UiDi.cs b/SemiStep/SemiStep.UI/UiDi.cs index eae906a..9e87c10 100644 --- a/SemiStep/SemiStep.UI/UiDi.cs +++ b/SemiStep/SemiStep.UI/UiDi.cs @@ -23,7 +23,7 @@ public static class UiDi public static IServiceCollection AddUi(this IServiceCollection services) { services.AddSingleton(sp => sp.GetRequiredService().GridStyle); - services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(_ => RxSchedulers.MainThreadScheduler); services.AddSingleton(); services.AddSingleton(); diff --git a/docs/plans/completed/20260727-pr-f1-report-log-extension.md b/docs/plans/completed/20260727-pr-f1-report-log-extension.md new file mode 100644 index 0000000..1de0ff3 --- /dev/null +++ b/docs/plans/completed/20260727-pr-f1-report-log-extension.md @@ -0,0 +1,148 @@ +# PR-F1: Report+Log command-exception extension + +## Overview + +The command-boundary ring of the error pipe (see `20260727-error-reporting-pipe-roadmap.md`). Today +the `ThrownExceptions → ObserveOn(MainThreadScheduler) → panel.ReportError($"…{ex.Message}")` block is +copy-pasted 8× and **logs nothing** — user-visible errors discard the exception type and stack. This +PR extracts one extension that reports **and** logs with the stack, wires every existing site through +it, closes three unguarded `Subscribe` faults and one unsubscribed command that crashes today. + +No `IMessageSink` — consumers keep the concrete `MessagePanelViewModel` (roadmap decision). No +behavior change on the happy path; on failure, the same panel message appears and a stack-carrying +log line is added. + +## Context (grounded) + +- The 8 identical blocks: `ClipboardViewModel.cs:56-69` (3), `RecipeFileViewModel.cs:37-50` (3), + `MainWindowViewModel.cs:71-79` (2). Each is `command.ThrownExceptions.ObserveOn(RxSchedulers.MainThreadScheduler).Subscribe(ex => _messagePanel.ReportError($"…: {ex.Message}"))`. +- The `ObserveOn` is redundant: `MessagePanelViewModel.PostOnUiThread` (`:211-221`) self-marshals. +- Existing extension home: `ResultReportingExtensions` (`SemiStep.UI/MessageService/ResultReportingExtensions.cs:14`) already extends the concrete `MessagePanelViewModel`. +- `ILogger` → class-named logs: the Serilog template prints `{SourceContext}` (`Program.cs`), and the MEL→Serilog bridge maps `ILogger`'s category to it. `MainWindowViewModel` already injects `ILogger` (`:32,47`); `ClipboardViewModel`, `RecipeFileViewModel`, `GridStyleEditorViewModel` do not. +- Three bare `Subscribe` (no `onError`, fatal-path today): `MainWindowViewModel.cs:85` (`PlcStateChanged`), `:89` (`PlcRecipeConflictDetected`), `:92-94` (`Interval` tick). +- Live crash path: `GridStyleEditorViewModel.SaveCommand` (`:56-58`) has no `ThrownExceptions` subscription; `Save()` (`:272`) does file I/O. A throw hits `RxApp.DefaultExceptionHandler` → crash. The editor has its own `ErrorMessage` surface (`:65`). +- DI: all VMs are container-registered (`UiDi.cs:36-44`); adding an `ILogger` ctor param resolves automatically. Direct test construction sites that need the new arg: + - `ClipboardViewModel`: `ClipboardViewModelCanExecuteTests.cs:37,111`, `UIFixture.cs:151`, `MessagePanelReportingTests.cs:147,185`. + - `RecipeFileViewModel`: `UIFixture.cs:158`, `MessagePanelReportingTests.cs:50,72,97`, `RecipeFileViewModelCanExecuteTests.cs:26`, `RecipeFileViewModelSaveResultTests.cs:31`. + - `GridStyleEditorViewModel`: `UIFixture.cs:166`, `GridStyleEditorViewModelTests.cs:252`, `GridStyleEditorWindowOwnerRoutingTests.cs:118`, `GridStyleEditorWindowTests.cs:31,59`. + +## Development Approach + +- Regular (code, then tests). Warnings are errors — build must stay clean. +- Tests use `NullLogger.Instance` (`Microsoft.Extensions.Logging.Abstractions`) except where a test asserts logging, which uses a capturing `ILogger`. +- Run `dotnet build SemiStep.slnx` and `dotnet test …` after each task; all green before the next. +- `dotnet format SemiStep.slnx` before finishing. + +## Acceptance Evidence + +Automatable: +1. `ReportThrownExceptions`: a command whose execute throws produces exactly one panel entry AND one `LogError(ex,…)` carrying the exception (capturing logger). `--filter "FullyQualifiedName~ReportThrownExceptions"`. +2. The three `MainWindowViewModel` subscriptions: a throw in each callback reports to the panel instead of escaping. `--filter "FullyQualifiedName~MainWindowViewModel"`. +3. `GridStyleEditorViewModel.SaveCommand`: a throw in `Save()` surfaces on `ErrorMessage` and logs, without crashing. `--filter "FullyQualifiedName~GridStyleEditor"`. +4. Regression: existing `MessagePanelReportingTests` stay green (same panel messages). + +Full suite green + `dotnet build SemiStep.slnx` (0 warnings) is the gate. + +## Progress Tracking + +Mark `[x]` on completion; `➕` new tasks; `⚠️` blockers. + +## Implementation Steps + +### Task 1: Add the `ReportThrownExceptions` extension + +**Files:** +- Create: `SemiStep/SemiStep.UI/MessageService/ReactiveCommandReportingExtensions.cs` +- Create: `SemiStep/SemiStep.Tests/UI/MessageService/ReactiveCommandReportingExtensionsTests.cs` + +- [x] add `static IDisposable ReportThrownExceptions(this ReactiveCommand command, MessagePanelViewModel panel, ILogger logger, string context)` that subscribes `ThrownExceptions` and, per exception, `logger.LogError(ex, "{Context} failed", context)` then `panel.ReportError($"{context}: {ex.Message}")`; no `ObserveOn` +- [x] `System` usings first, then others; tabs; braces on new line +- [x] test: a throwing command yields exactly one `ReportError` on the panel (assert via a real `MessagePanelViewModel`) and one `LogError` with the thrown exception (capturing `ILogger`) +- [x] test: a non-throwing execute produces no panel entry and no log +- [x] run `--filter "FullyQualifiedName~ReportThrownExceptions"` — green before next task + +### Task 2: Rewire `ClipboardViewModel` + +**Files:** +- Modify: `SemiStep/SemiStep.UI/Clipboard/ClipboardViewModel.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/Clipboard/ClipboardViewModelCanExecuteTests.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs` + +- [x] add `ILogger` ctor param + field +- [x] replace the three `ThrownExceptions` blocks (`:56-69`) with `command.ReportThrownExceptions(_messagePanel, _logger, "Copy failed"/"Cut failed"/"Paste failed").DisposeWith(_disposables)` (keep the existing user-facing text; extension appends `": {ex.Message}"`) +- [x] update the 5 direct construction sites to pass `NullLogger.Instance` +- [x] confirm existing clipboard reporting tests still pass (same messages) +- [x] run clipboard-related filters — green before next task + +### Task 3: Rewire `RecipeFileViewModel` + +**Files:** +- Modify: `SemiStep/SemiStep.UI/RecipeFile/RecipeFileViewModel.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelCanExecuteTests.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/RecipeFile/RecipeFileViewModelSaveResultTests.cs` + +- [x] add `ILogger` ctor param + field +- [x] replace the three `ThrownExceptions` blocks (`:37-50`) with the extension ("Save failed"/"Save As failed"/"Load failed") +- [x] update the 5 direct construction sites to pass `NullLogger.Instance` +- [x] run recipe-file filters — green before next task + +### Task 4: Rewire `MainWindowViewModel` + guard the three bare subscriptions + +**Files:** +- Modify: `SemiStep/SemiStep.UI/MainWindow/MainWindowViewModel.cs` +- Create/Modify: `SemiStep/SemiStep.Tests/UI/MainWindow/MainWindowViewModelReportingTests.cs` + +- [x] replace the two `ThrownExceptions` blocks (`:71-79`) with the extension ("Sync toggle failed"/"Style editor failed"); `_logger` already exists +- [x] give `PlcStateChanged` (`:85`), `PlcRecipeConflictDetected` (`:89`), and the `Interval` tick (`:94`) an `onError` that `_logger.LogError(ex,…)` + `MessagePanel.ReportError(…)` (do not let a callback throw escape to the fatal path) +- [x] test: a forced throw in each of the three callbacks reports to the panel and does not crash the headless app +- [x] run `--filter "FullyQualifiedName~MainWindowViewModel"` — green before next task + +### Task 5: Wire `GridStyleEditorViewModel.SaveCommand` to its own surface + log + +**Files:** +- Modify: `SemiStep/SemiStep.UI/StyleEditor/GridStyleEditorViewModel.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorViewModelTests.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowOwnerRoutingTests.cs` +- Modify: `SemiStep/SemiStep.Tests/UI/StyleEditor/GridStyleEditorWindowTests.cs` + +- [x] add `ILogger` to both constructors (thread it through the delegating `:42` ctor) +- [x] subscribe `SaveCommand.ThrownExceptions` → `_logger.LogError(ex, "Style editor save failed")` + set `ErrorMessage` to a message (the editor's own surface, not the shared panel); dispose with the VM +- [x] update the 4+ direct construction sites to pass `NullLogger.Instance` +- [x] test: a forced throw in `Save()` sets `ErrorMessage`, logs, and does not crash +- [x] run `--filter "FullyQualifiedName~GridStyleEditor"` — green before next task + +### Task 6: Verify + document + +**Files:** +- Modify: `Docs/architecture/error-reporting.md` + +- [x] full suite: `dotnet test SemiStep/SemiStep.Tests/SemiStep.Tests.csproj` (1472 passed, 0 failed) +- [x] `dotnet build SemiStep.slnx` — 0 warnings, 0 errors +- [x] `dotnet format SemiStep.slnx` (no changes) +- [x] update `error-reporting.md`: the exception-handler line now reads "route through `ReportThrownExceptions` (reports + logs with stack)"; note the concrete-panel decision +- [ ] ⚠️ move this plan to `docs/plans/completed/` (deferred to delivery — plan stays until branch is shipped; not performed) + +## Post-Completion + +- Open the PR: "feat(ui): route command exceptions through report+log extension". Note in the body that user-visible errors now carry a class-named stack trace to the log; no happy-path behavior change. +- Follow-on: PR-F2 (global backstop), then #113 rebased on both. + +**Executed by exec:** +- branch: report-log-extension + +## Verify it yourself + +The change is invisible on the happy path; it only shows up when a command or subscription faults. Prove it with the tests that reproduce each fault. + +- **Command exception reports + logs (the core outcome):** + `dotnet test SemiStep/SemiStep.Tests/SemiStep.Tests.csproj --filter "FullyQualifiedName~ReportThrownExceptions"` — a throwing `ReactiveCommand` produces exactly one panel entry AND one `LogError` carrying the same exception instance. Before this branch the log entry did not exist (handlers only reported). +- **Style-editor save crash is closed (was a live crash):** + `dotnet test ... --filter "FullyQualifiedName~GridStyleEditor"` — `SaveCommand_WhenSaveThrows_...` drives `SaveCommand.Execute()` through a throwing facade and asserts `ErrorMessage` is set + logged, no crash. On `master`, `SaveCommand` has no `ThrownExceptions` subscription, so that throw hits `RxApp.DefaultExceptionHandler` and kills the process. +- **Subscription-callback throws are contained (was fatal for the Interval tick):** + `dotnet test ... --filter "FullyQualifiedName~MainWindowViewModel"` — `Guarded_ThrowInOnNextBody_...` forces a throw inside the guarded onNext and asserts it reports without escaping. +- **No happy-path regression / no doubled log:** full suite `dotnet test SemiStep/SemiStep.Tests/SemiStep.Tests.csproj` (1474 pass) and `dotnet build SemiStep.slnx` (0 warnings). Existing reporting tests still assert the same user-facing panel messages ("Copy failed:", "Load failed:", …). +- **Manual (optional):** run the app, trigger a command failure (e.g. paste malformed clipboard content), confirm the panel message is unchanged AND the log file now carries the exception with `SourceContext` = the originating class.