From cd292e2e39e060e0a93168420627af5d9bdca95e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 7 Jun 2026 21:00:30 +0200 Subject: [PATCH] Report composition failures non-fatally and surface them to the user A plugin whose assembly won't load, or an exported command whose constructor can't be satisfied, previously either blanked the app into the startup error window or threw mid-build and took the whole menu/toolbar down with it. Collect such failures in a CompositionErrors sink instead, skip the offending part, keep running, and show the errors in a document tab via an ITextOutput once the window is up. Add a Composition log category for opt-in file logging. Assisted-by: Claude:claude-opus-4-8:Claude Code --- ILSpy.Tests/AppEnv/CompositionErrorsTests.cs | 58 +++++++++++++++++ ILSpy/AppEnv/AppComposition.cs | 4 +- ILSpy/AppEnv/AppLog.cs | 8 +++ ILSpy/AppEnv/CompositionErrors.cs | 67 ++++++++++++++++++++ ILSpy/Docking/DockWorkspace.cs | 15 +++++ ILSpy/Views/MainMenu.axaml.cs | 18 +++++- ILSpy/Views/MainToolBar.axaml.cs | 13 +++- ILSpy/Views/MainWindow.axaml.cs | 24 +++++++ 8 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 ILSpy.Tests/AppEnv/CompositionErrorsTests.cs create mode 100644 ILSpy/AppEnv/CompositionErrors.cs diff --git a/ILSpy.Tests/AppEnv/CompositionErrorsTests.cs b/ILSpy.Tests/AppEnv/CompositionErrorsTests.cs new file mode 100644 index 000000000..d32522316 --- /dev/null +++ b/ILSpy.Tests/AppEnv/CompositionErrorsTests.cs @@ -0,0 +1,58 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; + +using AwesomeAssertions; + +using ICSharpCode.Decompiler; + +using ILSpy.AppEnv; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.AppEnv; + +/// +/// Composition errors are collected non-fatally and can be rendered to an +/// for display to the user (the source label and the exception text both appear). +/// +[TestFixture] +public class CompositionErrorsTests +{ + // The sink is a process-global static. Leaving entries behind would make every later test that + // boots MainWindow pop a "Composition Errors" tab, so clear it after each test here. + [TearDown] + public void TearDown() => CompositionErrors.Clear(); + + [Test] + public void Report_Collects_The_Error_And_WriteTo_Renders_It() + { + var marker = "Plugin 'Marker_" + Guid.NewGuid().ToString("N") + "'"; + CompositionErrors.Report(marker, new InvalidOperationException("boom-message")); + + CompositionErrors.Any.Should().BeTrue(); + + var output = new PlainTextOutput(); + CompositionErrors.WriteTo(output); + var text = output.ToString(); + + text.Should().Contain(marker, "the failing part's label is shown"); + text.Should().Contain("boom-message", "the exception detail is shown"); + } +} diff --git a/ILSpy/AppEnv/AppComposition.cs b/ILSpy/AppEnv/AppComposition.cs index f78877e34..116e073b7 100644 --- a/ILSpy/AppEnv/AppComposition.cs +++ b/ILSpy/AppEnv/AppComposition.cs @@ -107,7 +107,9 @@ namespace ILSpy.AppEnv } catch (Exception ex) { - StartupExceptions.Items.Add(new ExceptionData(ex) { PluginName = name }); + // Non-fatal: a plugin that won't load is skipped and reported to the user, + // rather than aborting startup into the error window. + CompositionErrors.Report($"Plugin '{name}'", ex); } if (assembly != null) yield return assembly; diff --git a/ILSpy/AppEnv/AppLog.cs b/ILSpy/AppEnv/AppLog.cs index b8b638721..ca9ce3dea 100644 --- a/ILSpy/AppEnv/AppLog.cs +++ b/ILSpy/AppEnv/AppLog.cs @@ -67,6 +67,14 @@ namespace ILSpy.AppEnv /// default; opt in with ILSPY_LOG=DBUSDEBUG (matched case-insensitively). /// public const string DBusDebug = "DBusDebug"; + + /// + /// Non-fatal MEF composition failures collected by — a + /// plugin assembly that won't load, or an exported command whose constructor throws when + /// the menu/toolbar builder materialises it. Errors are always shown to the user in a + /// document tab; opt in with ILSPY_LOG=Composition to also log them to file. + /// + public const string Composition = "Composition"; } static readonly Stopwatch sw = Stopwatch.StartNew(); diff --git a/ILSpy/AppEnv/CompositionErrors.cs b/ILSpy/AppEnv/CompositionErrors.cs new file mode 100644 index 000000000..6f505a972 --- /dev/null +++ b/ILSpy/AppEnv/CompositionErrors.cs @@ -0,0 +1,67 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; + +using ICSharpCode.Decompiler; + +namespace ILSpy.AppEnv +{ + /// A single non-fatal composition failure: which part failed, and why. + public sealed record CompositionError(string Source, Exception Exception); + + /// + /// Collects non-fatal MEF composition failures -- a plugin assembly that fails to load, or an + /// exported command whose constructor throws when the menu/toolbar builder materialises it. + /// Unlike (fatal startup errors that replace the app with an + /// error window), these are recoverable: the offending part is skipped, the app keeps running, + /// and the collected errors are surfaced to the user in a document tab (see ) + /// and, when is enabled, written to the log file. + /// + public static class CompositionErrors + { + static readonly List items = new(); + + public static IReadOnlyList Items => items; + + public static bool Any => items.Count > 0; + + /// Records a composition failure and logs it under the Composition category. + public static void Report(string source, Exception exception) + { + items.Add(new CompositionError(source, exception)); + AppLog.Write(AppLog.Category.Composition, $"{source}: {exception}"); + } + + /// Clears the collected errors. Intended for test isolation. + public static void Clear() => items.Clear(); + + /// Renders every collected error into for display. + public static void WriteTo(ITextOutput output) + { + ArgumentNullException.ThrowIfNull(output); + foreach (var error in items) + { + output.WriteLine($"[{error.Source}]"); + output.WriteLine(error.Exception.ToString()); + output.WriteLine(); + } + } + } +} diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index e100ffaf4..cf014d817 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -1111,6 +1111,21 @@ namespace ILSpy.Docking /// public void ShowToolPane(string contentId) => factory.ShowToolPane(contentId); + /// + /// Opens a fresh frozen tab showing the supplied pre-rendered text output. Used to surface + /// reports that aren't tied to a tree node (e.g. composition-error listings). + /// + public void ShowTextInNewTab(string title, TextView.AvaloniaEditTextOutput output) + { + ArgumentNullException.ThrowIfNull(output); + var content = new TextView.DecompilerTabPageModel { + Language = languageService.CurrentLanguage, + Title = title, + }; + OpenNewTab(content); + content.ShowText(output); + } + void ExecuteShowSearch() { ShowToolPane(ILSpy.Search.SearchPaneModel.PaneContentId); diff --git a/ILSpy/Views/MainMenu.axaml.cs b/ILSpy/Views/MainMenu.axaml.cs index 2b764f84a..8fee6c3bb 100644 --- a/ILSpy/Views/MainMenu.axaml.cs +++ b/ILSpy/Views/MainMenu.axaml.cs @@ -22,13 +22,13 @@ using System.Composition; using System.Linq; using System.Windows.Input; +using CommunityToolkit.Mvvm.Input; + using global::Avalonia; using global::Avalonia.Controls; using global::Avalonia.Data; using global::Avalonia.Input; -using CommunityToolkit.Mvvm.Input; - using ICSharpCode.ILSpy.Properties; using ILSpy.AppEnv; @@ -232,7 +232,19 @@ public static class MainMenu } else { - var command = entry.CreateExport().Value; + ICommand command; + try + { + // Isolate each entry: instantiating a command export can throw for a + // misbehaving plugin (e.g. a DI ctor without [ImportingConstructor]); one + // bad command must not take down the whole menu bar. + command = entry.CreateExport().Value; + } + catch (Exception ex) + { + AppEnv.CompositionErrors.Report($"Main-menu command '{entry.Metadata?.Header}'", ex); + continue; + } // No explicit IsEnabled: assigning Command lets NativeMenuItem track the // command's CanExecute, so OS-gated commands (e.g. Open from GAC, which is // Windows-only) grey out correctly. A hard-coded IsEnabled would override diff --git a/ILSpy/Views/MainToolBar.axaml.cs b/ILSpy/Views/MainToolBar.axaml.cs index bbdcd887e..6ab39d2b7 100644 --- a/ILSpy/Views/MainToolBar.axaml.cs +++ b/ILSpy/Views/MainToolBar.axaml.cs @@ -194,7 +194,18 @@ public partial class MainToolBar : UserControl static Button? BuildButton(ExportFactory entry) { - var command = entry.CreateExport().Value; + System.Windows.Input.ICommand command; + try + { + // Isolate each entry: a misbehaving plugin command (e.g. a DI ctor without + // [ImportingConstructor]) must not take down the whole toolbar. + command = entry.CreateExport().Value; + } + catch (System.Exception ex) + { + ILSpy.AppEnv.CompositionErrors.Report($"Toolbar command '{entry.Metadata.ToolTip}'", ex); + return null; + } var button = new Button { Tag = entry.Metadata.ToolTip, Command = command, diff --git a/ILSpy/Views/MainWindow.axaml.cs b/ILSpy/Views/MainWindow.axaml.cs index 00d352aa8..f35bb32a2 100644 --- a/ILSpy/Views/MainWindow.axaml.cs +++ b/ILSpy/Views/MainWindow.axaml.cs @@ -68,10 +68,34 @@ namespace ILSpy.Views AppLog.Mark("MainWindow.Opened handler returning"); if (App.CommandLineArguments is { } args) await viewModel.AssemblyTreeModel.HandleCommandLineArgumentsAsync(args); + // Surface any non-fatal composition failures (failed plugins, uninstantiable menu/ + // toolbar commands) once the menu and toolbar have finished building. Deferred to a + // later dispatcher turn so those builders' own Loaded handlers have run first. + Avalonia.Threading.Dispatcher.UIThread.Post(SurfaceCompositionErrors, + Avalonia.Threading.DispatcherPriority.Background); }; AppLog.Mark("MainWindow ctor exited"); } + static void SurfaceCompositionErrors() + { + if (!AppEnv.CompositionErrors.Any) + return; + try + { + var output = new TextView.AvaloniaEditTextOutput { Title = "Composition Errors" }; + AppEnv.CompositionErrors.WriteTo(output); + AppEnv.AppComposition.Current + .GetExport() + .ShowTextInNewTab("Composition Errors", output); + } + catch (System.Exception ex) + { + // Surfacing the report must never itself crash startup. + System.Diagnostics.Debug.WriteLine($"[MainWindow] SurfaceCompositionErrors failed: {ex}"); + } + } + void ApplySessionSettings(SessionSettings session) { Position = session.WindowPosition;