Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions Docs/architecture/error-reporting.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<TParam, TResult>(this ReactiveCommand<TParam, TResult>, 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<TVm>`, 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.
4 changes: 2 additions & 2 deletions Docs/architecture/headless-ui-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <see cref="Save"/>.
/// </summary>
public sealed class GridStyleEditorFacade
public sealed class GridStyleEditorFacade : IGridStyleEditorFacade
{
private readonly GridStyleWriter _gridStyleWriter = new();

Expand Down
16 changes: 16 additions & 0 deletions SemiStep/SemiStep.Core/Configuration/IGridStyleEditorFacade.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using FluentResults;

namespace SemiStep.Core.Configuration;

/// <summary>
/// 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.
/// </summary>
public interface IGridStyleEditorFacade
{
Task<Result<GridStyleOptions>> Load(string configDir);

Result Validate(GridStyleOptions options);

Result Save(string configDir, GridStyleOptions options);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -39,7 +41,8 @@ public async ValueTask InitializeAsync()
_surface,
clipboardSerializer,
importedRecipeValidator,
_fixture.MessagePanel);
_fixture.MessagePanel,
NullLogger<ClipboardViewModel>.Instance);
}

public async ValueTask DisposeAsync()
Expand Down Expand Up @@ -113,7 +116,8 @@ public void Cut_CanExecuteTrue_WhenSelectionExistsBeforeConstruction()
_surface,
new ClipboardSerializer(_fixture.RecipeMetadataRegistry),
new ImportedRecipeValidator(_fixture.RecipeMetadataRegistry),
_fixture.MessagePanel);
_fixture.MessagePanel,
NullLogger<ClipboardViewModel>.Instance);

((ICommand)clipboard.CutStepCommand).CanExecute(null).Should().BeTrue();
}
Expand Down
16 changes: 11 additions & 5 deletions SemiStep/SemiStep.Tests/UI/Helpers/UIFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ public void SeedRecipe(int stepCount)
}

public MainWindowViewModel CreateMainWindowViewModel(
Func<GridStyleEditorViewModel>? styleEditorFactory = null)
Func<GridStyleEditorViewModel>? styleEditorFactory = null,
ILogger<MainWindowViewModel>? logger = null)
{
var grid = CreateActiveSurface();

Expand All @@ -153,9 +154,13 @@ public MainWindowViewModel CreateMainWindowViewModel(
grid,
clipboardSerializer,
importedRecipeValidator,
MessagePanel);
MessagePanel,
NullLogger<ClipboardViewModel>.Instance);

var recipeFile = new RecipeFileViewModel(Coordinator, MessagePanel);
var recipeFile = new RecipeFileViewModel(
Coordinator,
MessagePanel,
NullLogger<RecipeFileViewModel>.Instance);

var plcMonitor = new PlcMonitorViewModel(
Coordinator,
Expand All @@ -166,7 +171,8 @@ public MainWindowViewModel CreateMainWindowViewModel(
() => new GridStyleEditorViewModel(
new GridStyleEditorFacade(),
@"C:\does-not-exist",
AppConfiguration.GridStyle));
AppConfiguration.GridStyle,
NullLogger<GridStyleEditorViewModel>.Instance));

return new MainWindowViewModel(
Coordinator,
Expand All @@ -177,7 +183,7 @@ public MainWindowViewModel CreateMainWindowViewModel(
MessagePanel,
plcMonitor,
gridStyleEditorViewModelFactory,
NullLogger<MainWindowViewModel>.Instance);
logger ?? NullLogger<MainWindowViewModel>.Instance);
}

public void SetSyncEnabled(bool isSyncEnabled)
Expand Down
Original file line number Diff line number Diff line change
@@ -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<MainWindowViewModel> _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);
}
}
23 changes: 18 additions & 5 deletions SemiStep/SemiStep.Tests/UI/MessagePanelReportingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

using FluentAssertions;

using Microsoft.Extensions.Logging.Abstractions;

using ReactiveUI;

using SemiStep.Core.Recipes.Clipboard;
Expand Down Expand Up @@ -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<RecipeFileViewModel>.Instance);
var filePath = Path.Combine(Path.GetTempPath(), $"semistep-save-{Guid.NewGuid():N}.csv");
recipeFile.SaveFileInteraction.RegisterHandler(context => context.SetOutput(filePath));

Expand All @@ -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<RecipeFileViewModel>.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));
Expand All @@ -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<RecipeFileViewModel>.Instance);
var missingPath = Path.Combine(Path.GetTempPath(), $"semistep-missing-{Guid.NewGuid():N}.csv");
recipeFile.OpenFileInteraction.RegisterHandler(context => context.SetOutput(missingPath));

Expand Down Expand Up @@ -149,7 +160,8 @@ public async Task Clipboard_PasteInvalidContent_ReportsError()
_surface,
clipboardSerializer,
importedRecipeValidator,
_fixture.MessagePanel);
_fixture.MessagePanel,
NullLogger<ClipboardViewModel>.Instance);

var window = new Window();
window.Show();
Expand Down Expand Up @@ -187,7 +199,8 @@ public async Task Clipboard_PasteValidContent_LeavesNoOperationError()
_surface,
clipboardSerializer,
importedRecipeValidator,
_fixture.MessagePanel);
_fixture.MessagePanel,
NullLogger<ClipboardViewModel>.Instance);

var window = new Window();
window.Show();
Expand Down
Original file line number Diff line number Diff line change
@@ -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<object>();
var failure = new InvalidOperationException("boom");
var command = ReactiveCommand.Create<Unit, Unit>(_ => 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<object>();
var command = ReactiveCommand.Create<Unit, Unit>(_ => 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<Unit, Unit> command)
{
try
{
await command.Execute();
}
catch (InvalidOperationException)
{
// The command routes the throw to ThrownExceptions; Execute also rethrows to the awaiter.
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

using FluentAssertions;

using Microsoft.Extensions.Logging.Abstractions;

using SemiStep.Tests.UI.Helpers;

using SemiStep.UI.RecipeFile;
Expand All @@ -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<RecipeFileViewModel>.Instance);
}

public async ValueTask DisposeAsync()
Expand Down
Loading