From 9f6a3968906a2489fd8e7c2012abb9adf1c7401a Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 11:29:40 +0200 Subject: [PATCH 1/2] refactor: install the exception handler before startup The global exception handler is now installed right after the WinForms configuration, before directory and proxy setup, the WebView2 runtime check and the bootstrap. Until the bootstrap services are attached, a fatal exception is saved to the report and shown in the dialog without logging or telemetry, so a startup failure such as an unreadable policy key is no longer silent. A dialog that cannot be shown is logged instead of escaping the handler and ending the process through FailFast. --- .../0003-fatal-exceptions-end-the-process.md | 3 +- docs/adr/0005-policy-failures-stop-startup.md | 5 ++- docs/design/diagnostics.md | 17 ++++--- src/Host/BravoGlobalExceptionHandling.cs | 9 ++-- .../Diagnostics/ErrorReportTaskDialog.cs | 32 +++++++------- .../UnhandledExceptionHandler.Log.cs | 13 ++++++ .../Diagnostics/UnhandledExceptionHandler.cs | 44 +++++++++++++++---- src/Program.cs | 5 ++- .../Diagnostics/DiagnosticsFakes.cs | 11 +++++ .../GlobalExceptionHandlingTests.cs | 10 +---- .../UnhandledExceptionHandlerTests.cs | 35 +++++++++++++++ 11 files changed, 135 insertions(+), 49 deletions(-) create mode 100644 src/Infrastructure/Diagnostics/UnhandledExceptionHandler.Log.cs create mode 100644 test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs create mode 100644 test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs diff --git a/docs/adr/0003-fatal-exceptions-end-the-process.md b/docs/adr/0003-fatal-exceptions-end-the-process.md index 9ed7dc41..018425c8 100644 --- a/docs/adr/0003-fatal-exceptions-end-the-process.md +++ b/docs/adr/0003-fatal-exceptions-end-the-process.md @@ -34,6 +34,7 @@ that the application failed and need something they can pass to support. - No work survives a fatal exception; a crash is visible, never silent. - The report file is overwritten by the next crash: it serves the dialog that shows it, not as a history. -- Exceptions raised before the handler is installed are not covered; see +- The handler is installed before logging and telemetry exist: a fatal exception during startup gets the report and + the dialog, but no log entry or telemetry; see [diagnostics](../design/diagnostics.md#startup-coverage-and-fatal-reporting). - Code on the finalizer thread, where unobserved task exceptions arrive, must never show UI or wait. diff --git a/docs/adr/0005-policy-failures-stop-startup.md b/docs/adr/0005-policy-failures-stop-startup.md index 672f7cf1..ac20b9ad 100644 --- a/docs/adr/0005-policy-failures-stop-startup.md +++ b/docs/adr/0005-policy-failures-stop-startup.md @@ -25,8 +25,9 @@ key that cannot be read. ## Consequences -- A policy registry key that cannot be read ends the process before the global exception handler exists: there is - no report and no dialog, only the runtime's Event Log entry. See +- A policy registry key that cannot be read ends the process through the global exception handler: the user sees + the crash dialog and the report is saved, but nothing is logged or tracked, because the bootstrap services are + not attached to the handler yet. See [diagnostics](../design/diagnostics.md#startup-coverage-and-fatal-reporting). - Defaults replace user settings only for the current session; how a corrupt settings file is kept and reported to the user is outside this decision. diff --git a/docs/design/diagnostics.md b/docs/design/diagnostics.md index f953fcd7..fe8e6aec 100644 --- a/docs/design/diagnostics.md +++ b/docs/design/diagnostics.md @@ -7,7 +7,7 @@ Exception reporting in the .NET host. The frontend has a separate | Origin | Process outcome | Current reporting | | --- | --- | --- | -| `AppDomain.UnhandledException`, including WinForms exceptions routed through `ThrowException` | Runtime terminates after the handler returns | Error log; telemetry subject to consent; saved report and crash dialog | +| `AppDomain.UnhandledException`, including WinForms exceptions routed through `ThrowException` | Runtime terminates after the handler returns | Saved report and crash dialog; after the bootstrap, also error log and telemetry subject to consent | | `TaskScheduler.UnobservedTaskException` | Continues | Error log and telemetry subject to consent; no dialog or shutdown flush | | Exception caught by an operation that can recover or return an error | Continues | Reporting belongs to the operation or its API boundary; see [handled exceptions](#handled-exceptions) | @@ -18,13 +18,15 @@ ends the process: [ADR 0003](../adr/0003-fatal-exceptions-end-the-process.md). ## Startup coverage and fatal reporting -[Program](../../src/Program.cs) installs global handling after `BravoApplicationConfiguration.Initialize` and the -bootstrap, before building the host. Host construction and everything after it are covered. Directory and proxy -setup, the WebView2 runtime check and bootstrap creation are not: an exception there ends the process with only the -runtime's own Event Log entry, without a report or a dialog. +[Program](../../src/Program.cs) installs global handling right after `ApplicationConfiguration.Initialize`, which +enables the visual styles the task dialog needs, and before every other startup step. Logging and telemetry do not +exist yet: until the bootstrap context hands them to the handler through `AttachServices`, a fatal exception is +saved to the report and shown in the dialog, without a log entry from Bravo, without telemetry and without the Send +button. The runtime still writes its own Event Log entry when it ends the process. This covers directory and proxy +setup, the WebView2 runtime check and bootstrap creation. [BootstrapContextFactory](../../src/Host/BootstrapContextFactory.cs) degrades to default settings and disabled telemetry when they cannot be created, but a policy read failure propagates on purpose, so that administrative -restrictions are never bypassed: a policy registry key that cannot be read ends the process this way +restrictions are never bypassed: a policy registry key that cannot be read ends the process through this path ([ADR 0005](../adr/0005-policy-failures-stop-startup.md)). [UnhandledExceptionHandler](../../src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs) logs and tracks each @@ -33,7 +35,8 @@ other. For a fatal exception, a marker in `Exception.Data` suppresses a repeated suppress the earlier log and telemetry calls. [ErrorReport](../../src/Infrastructure/Diagnostics/ErrorReport.cs) saves environment diagnostics and the exception to -`ErrorReport.txt` under the application data folder. A failed save does not prevent the dialog. +`ErrorReport.txt` under the application data folder. A failed save does not prevent the dialog, and a dialog that +cannot be shown is logged and does not prevent the shutdown flush. [ErrorReportTaskDialog](../../src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs) runs on a dedicated STA thread and offers copying, closing and, when host telemetry is disabled, sending the exception through telemetry. Automatic tracking is an attempt, not confirmation of delivery. diff --git a/src/Host/BravoGlobalExceptionHandling.cs b/src/Host/BravoGlobalExceptionHandling.cs index e776cdb8..2a41d9fc 100644 --- a/src/Host/BravoGlobalExceptionHandling.cs +++ b/src/Host/BravoGlobalExceptionHandling.cs @@ -4,17 +4,16 @@ namespace Sqlbi.Bravo.Host; internal static class BravoGlobalExceptionHandling { - public static void Configure(BootstrapContext context) + public static UnhandledExceptionHandler Configure() { - var dialog = new ErrorReportTaskDialog(context.Telemetry); - - var handler = new UnhandledExceptionHandler( - dialog, context.Telemetry, context.LoggerFactory); + var handler = new UnhandledExceptionHandler(new ErrorReportTaskDialog()); GlobalExceptionHandling.Configure(handler.Handle); #if DEBUG CrashTest.RunIfRequested(); #endif + + return handler; } } diff --git a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs index fd2ef75b..21cf3292 100644 --- a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs +++ b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs @@ -14,35 +14,34 @@ namespace Sqlbi.Bravo.Infrastructure.Diagnostics; internal interface IErrorReportDialog { - void Show(ErrorReport report); + void Show(ErrorReport report, ITelemetryService? telemetry); } /// /// Implements an error report dialog to display error information to the user. /// -internal sealed class ErrorReportTaskDialog(ITelemetryService telemetry) : IErrorReportDialog +internal sealed class ErrorReportTaskDialog : IErrorReportDialog { - private readonly ITelemetryService _telemetry = telemetry; - - public void Show(ErrorReport report) + public void Show(ErrorReport report, ITelemetryService? telemetry) { // Always run on a dedicated STA thread: TaskDialog and Clipboard require STA, // while the caller could be executing on any arbitrary background thread. - ProcessHelper.RunOnSTAThread(() => ShowImpl(report)); + ProcessHelper.RunOnSTAThread(() => ShowImpl(report, telemetry)); } - private void ShowImpl(ErrorReport report) + private static void ShowImpl(ErrorReport report, ITelemetryService? telemetry) { // Ensure a WindowsFormsSynchronizationContext exists on this STA thread so that async continuations // post back to this thread's message loop, where the modal dialog can safely process UI updates. if (SynchronizationContext.Current is not WindowsFormsSynchronizationContext) SynchronizationContext.SetSynchronizationContext(new WindowsFormsSynchronizationContext()); + // When telemetry is enabled, reports are automatically submitted; offer Send only when it is disabled. + var sendTelemetry = telemetry is { TelemetryEnabled: false } ? telemetry : null; var sendButton = new TaskDialogCommandLinkButton("&Send report", "Sends the crash report to help diagnose the problem.") { AllowCloseDialog = false, - // When telemetry is enabled, reports are automatically submitted; hide this redundant button. - Visible = !_telemetry.TelemetryEnabled, + Visible = sendTelemetry is not null, }; var copyButton = new TaskDialogCommandLinkButton("&Copy to clipboard", "Copies the crash report details to the clipboard.") { @@ -66,27 +65,28 @@ private void ShowImpl(ErrorReport report) var page = builder.Build(); copyButton.Click += (_, _) => CopyToClipboard(page, text, report); - sendButton.Click += async (_, _) => await SendAsync(page, sendButton, text, report); + if (sendTelemetry is not null) + sendButton.Click += async (_, _) => await SendAsync(sendTelemetry, page, sendButton, text, report); builder.Show(); } - private async Task SendAsync(TaskDialogPage page, TaskDialogButton sendButton, string text, ErrorReport report) + private static async Task SendAsync(ITelemetryService telemetry, TaskDialogPage page, TaskDialogButton sendButton, string text, ErrorReport report) { - var previousTelemetryEnabled = _telemetry.TelemetryEnabled; + var previousTelemetryEnabled = telemetry.TelemetryEnabled; try { sendButton.Enabled = false; SetStatus(page, text, "Sending the report..."); - _telemetry.TelemetryEnabled = true; - _telemetry.TrackException(report.Exception, properties: new Dictionary + telemetry.TelemetryEnabled = true; + telemetry.TrackException(report.Exception, properties: new Dictionary { ["ExceptionKind"] = "Unhandled" }); using var timeout = new CancellationTokenSource(30_000); - var submitted = await _telemetry.FlushAsync(timeout.Token); + var submitted = await telemetry.FlushAsync(timeout.Token); SetStatus(page, text, submitted ? "The report has been submitted. Thank you." @@ -98,7 +98,7 @@ private async Task SendAsync(TaskDialogPage page, TaskDialogButton sendButton, s } finally { - _telemetry.TelemetryEnabled = previousTelemetryEnabled; + telemetry.TelemetryEnabled = previousTelemetryEnabled; } } diff --git a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.Log.cs b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.Log.cs new file mode 100644 index 00000000..cace86eb --- /dev/null +++ b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.Log.cs @@ -0,0 +1,13 @@ +using System; +using Microsoft.Extensions.Logging; + +namespace Sqlbi.Bravo.Infrastructure.Diagnostics; + +internal sealed partial class UnhandledExceptionHandler +{ + [LoggerMessage(EventId = 1, Level = LogLevel.Error, Message = "An unhandled exception occurred. Origin: {Origin}")] + private static partial void LogUnhandledException(ILogger logger, Exception exception, ExceptionOrigin origin); + + [LoggerMessage(EventId = 2, Level = LogLevel.Error, Message = "The error report dialog could not be shown.")] + private static partial void LogDialogFailed(ILogger logger, Exception exception); +} diff --git a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs index 3ef2753a..a5c672f7 100644 --- a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs +++ b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Sqlbi.Bravo.Infrastructure.Telemetry; namespace Sqlbi.Bravo.Infrastructure.Diagnostics; @@ -9,15 +10,21 @@ namespace Sqlbi.Bravo.Infrastructure.Diagnostics; /// Handles unhandled exceptions in the application, providing a mechanism /// to report and display error information to the user. /// -internal sealed class UnhandledExceptionHandler(IErrorReportDialog dialog, ITelemetryService telemetry, ILoggerFactory loggerFactory) +internal sealed partial class UnhandledExceptionHandler(IErrorReportDialog dialog) { private readonly IErrorReportDialog _dialog = dialog; - private readonly ITelemetryService _telemetry = telemetry; - private readonly ILogger _logger = loggerFactory.CreateLogger(); + private volatile ReportingServices _services = new(NullLogger.Instance, Telemetry: null); + + public void AttachServices(ILoggerFactory loggerFactory, ITelemetryService telemetry) + { + _services = new ReportingServices(loggerFactory.CreateLogger(), telemetry); + } public void Handle(Exception exception, ExceptionOrigin origin) { - Report(exception, origin); + var services = _services; + + Report(services, exception, origin); // Do not show UI for unobserved task exceptions (finalizer threads are runtime-critical). // The process keeps running, so the channel sends the telemetry on its own flush interval. @@ -30,22 +37,39 @@ public void Handle(Exception exception, ExceptionOrigin origin) var report = ErrorReport.Create(exception); _ = report.TrySave(); - _dialog.Show(report); + ShowDialog(services, report); - _ = _telemetry.TryFlushBeforeShutdown(10_000); + _ = services.Telemetry?.TryFlushBeforeShutdown(10_000); + } + + private void ShowDialog(ReportingServices services, ErrorReport report) + { + try + { + _dialog.Show(report, services.Telemetry); + } + catch (Exception ex) + { + // The report is already saved; a failing dialog must not turn the crash into a FailFast. + try + { + LogDialogFailed(services.Logger, ex); + } + catch { /* Ignore failures */ } + } } - private void Report(Exception exception, ExceptionOrigin origin) + private static void Report(ReportingServices services, Exception exception, ExceptionOrigin origin) { try { - _logger.LogError(exception, "An unhandled exception occurred. Origin: {Origin}", origin); + LogUnhandledException(services.Logger, exception, origin); } catch { /* Ignore failures */ } try { - _telemetry.TrackException(exception, properties: new Dictionary + services.Telemetry?.TrackException(exception, properties: new Dictionary { ["ExceptionOrigin"] = origin.ToString(), ["ExceptionKind"] = "Unhandled" @@ -72,4 +96,6 @@ private static bool TryMarkHandled(Exception exception) return true; } } + + private sealed record ReportingServices(ILogger Logger, ITelemetryService? Telemetry); } diff --git a/src/Program.cs b/src/Program.cs index 9f6eb41b..48d47081 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -8,13 +8,16 @@ internal static class Program [STAThread] private static void Main() { + // Visual styles first: the crash dialog is a task dialog, which requires them. ApplicationConfiguration.Initialize(); + var exceptionHandler = BravoGlobalExceptionHandling.Configure(); + BravoApplicationConfiguration.Initialize(); using var context = BootstrapContextFactory.CreateDefault(); - BravoGlobalExceptionHandling.Configure(context); + exceptionHandler.AttachServices(context.LoggerFactory, context.Telemetry); using var app = BravoApplication .CreateBuilder(context) diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs new file mode 100644 index 00000000..bfdfa830 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs @@ -0,0 +1,11 @@ +using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Sqlbi.Bravo.Infrastructure.Telemetry; + +namespace Bravo.Tests.Infrastructure.Diagnostics; + +internal sealed class RecordingDialog : IErrorReportDialog +{ + public int ShowCount { get; private set; } + + public void Show(ErrorReport report, ITelemetryService? telemetry) => ShowCount++; +} diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs index b4e8806a..8211bdcc 100644 --- a/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs @@ -101,7 +101,8 @@ public void RuntimeCallbacks_UnobservedTask_ReportsWithoutDialogOrFlush() { var dialog = new RecordingDialog(); var telemetry = Substitute.For(); - var handler = new UnhandledExceptionHandler(dialog, telemetry, NullLoggerFactory.Instance); + var handler = new UnhandledExceptionHandler(dialog); + handler.AttachServices(NullLoggerFactory.Instance, telemetry); var args = new UnobservedTaskExceptionEventArgs(new AggregateException("unobserved")); SetHandler(handler.Handle); @@ -129,11 +130,4 @@ private static void RaiseUnobserved(UnobservedTaskExceptionEventArgs args) => typeof(GlobalExceptionHandling) .GetMethod("OnUnobservedTaskException", BindingFlags.NonPublic | BindingFlags.Static)! .Invoke(null, [null, args]); - - private sealed class RecordingDialog : IErrorReportDialog - { - public int ShowCount { get; private set; } - - public void Show(ErrorReport report) => ShowCount++; - } } diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs new file mode 100644 index 00000000..b53610ee --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs @@ -0,0 +1,35 @@ +using System; +using Bravo.Tests.Host; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Xunit; + +namespace Bravo.Tests.Infrastructure.Diagnostics; + +public class UnhandledExceptionHandlerTests +{ + [Fact] + public void Handle_UnobservedTaskBeforeAttachServices_DoesNotThrow() + { + var handler = new UnhandledExceptionHandler(new RecordingDialog()); + + handler.Handle(new InvalidOperationException("unobserved"), ExceptionOrigin.UnobservedTask); + } + + [Fact] + public void Handle_UnobservedTaskAfterAttachServices_LogsAndTracks() + { + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + var handler = new UnhandledExceptionHandler(new RecordingDialog()); + var exception = new InvalidOperationException("unobserved"); + + handler.AttachServices(loggerFactory, telemetry); + handler.Handle(exception, ExceptionOrigin.UnobservedTask); + + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Error, entry.Level); + Assert.Same(exception, entry.Exception); + Assert.Same(exception, Assert.Single(telemetry.Exceptions)); + } +} From 9aed59b02a2d2294130518fb3160c0669db303b0 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 11:57:33 +0200 Subject: [PATCH 2/2] fix: keep crash reporting working before and after the bootstrap - Show a plain message box when the task dialog fails, since the types it reads can be the cause of the crash. - Resolve the report folder only when saving, so a failing AppEnvironment initialization still produces a dialog. - Attach the bootstrap services to the exception handler when the context is created and detach them before they are disposed, so a late crash never flushes a disposed telemetry service. A detach releases only the services it attached. - Treat NullTelemetryService as no telemetry, so the dialog does not offer a Send that always fails. --- docs/design/architecture.md | 2 +- docs/design/diagnostics.md | 16 ++- src/Host/BootstrapContext.cs | 7 +- src/Host/BootstrapContextFactory.cs | 16 ++- src/Host/BravoGlobalExceptionHandling.cs | 6 +- src/Infrastructure/Diagnostics/ErrorReport.cs | 4 +- .../Diagnostics/ErrorReportTaskDialog.cs | 36 +++++- .../Diagnostics/UnhandledExceptionHandler.cs | 40 +++++- src/Program.cs | 5 +- .../Bravo.Tests/Host/BootstrapContextTests.cs | 32 ++++- .../Diagnostics/DiagnosticsFakes.cs | 16 ++- .../GlobalExceptionHandlingTests.cs | 2 +- .../UnhandledExceptionHandlerTests.cs | 119 ++++++++++++++++-- 13 files changed, 260 insertions(+), 41 deletions(-) diff --git a/docs/design/architecture.md b/docs/design/architecture.md index 88945c19..fe7fa6dc 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -8,7 +8,7 @@ after the bootstrap services and the host are built, so a secondary process stil | Resource | Lifetime owner | Constraint for changes | | --- | --- | --- | -| Bootstrap telemetry and logger factory | [BootstrapContext](../../src/Host/BootstrapContext.cs) | Keep them available until host disposal completes; telemetry is disposed before the logger factory | +| Bootstrap telemetry and logger factory | [BootstrapContext](../../src/Host/BootstrapContext.cs) | Keep them available until host disposal completes; the exception handler is detached first, then telemetry is disposed before the logger factory | | Bootstrap policy service and user settings | Bootstrap creates or obtains them; the container receives those instances | Preserve the shared instances so consumers do not silently use different configuration snapshots | | Application instance and its pipe server | The host container owns `IBravoApplicationInstance`; that instance owns the pipe server | Releasing the pipe ends ownership of the application instance | | Main window | `BravoApplication.Run` | The window is created through DI but is not registered as a container-owned service | diff --git a/docs/design/diagnostics.md b/docs/design/diagnostics.md index fe8e6aec..227c1cee 100644 --- a/docs/design/diagnostics.md +++ b/docs/design/diagnostics.md @@ -20,10 +20,12 @@ ends the process: [ADR 0003](../adr/0003-fatal-exceptions-end-the-process.md). [Program](../../src/Program.cs) installs global handling right after `ApplicationConfiguration.Initialize`, which enables the visual styles the task dialog needs, and before every other startup step. Logging and telemetry do not -exist yet: until the bootstrap context hands them to the handler through `AttachServices`, a fatal exception is +exist yet: until `BootstrapContextFactory` attaches them to the handler, a fatal exception is saved to the report and shown in the dialog, without a log entry from Bravo, without telemetry and without the Send button. The runtime still writes its own Event Log entry when it ends the process. This covers directory and proxy -setup, the WebView2 runtime check and bootstrap creation. +setup, the WebView2 runtime check and bootstrap creation. `BootstrapContext` detaches the handler before it disposes +the services, so an exception raised during that disposal is reported the same way and never through disposed +services. [BootstrapContextFactory](../../src/Host/BootstrapContextFactory.cs) degrades to default settings and disabled telemetry when they cannot be created, but a policy read failure propagates on purpose, so that administrative restrictions are never bypassed: a policy registry key that cannot be read ends the process through this path @@ -35,8 +37,11 @@ other. For a fatal exception, a marker in `Exception.Data` suppresses a repeated suppress the earlier log and telemetry calls. [ErrorReport](../../src/Infrastructure/Diagnostics/ErrorReport.cs) saves environment diagnostics and the exception to -`ErrorReport.txt` under the application data folder. A failed save does not prevent the dialog, and a dialog that -cannot be shown is logged and does not prevent the shutdown flush. +`ErrorReport.txt` under the application data folder; the folder is resolved only when a report is saved, because +`AppEnvironment` initialization can be the failure being reported. A failed save does not prevent the dialog. If the +task dialog fails, for example because a type it reads failed to initialize, a plain message box shows the error and +the report path instead. The dialog failure is logged once the bootstrap services are attached, and it never prevents +the shutdown flush. [ErrorReportTaskDialog](../../src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs) runs on a dedicated STA thread and offers copying, closing and, when host telemetry is disabled, sending the exception through telemetry. Automatic tracking is an attempt, not confirmation of delivery. @@ -86,7 +91,8 @@ The `TelemetryEnabled` setter assigns the value it receives, so every caller res | Crash dialog Send | Temporarily enables host telemetry, tracks the exception and awaits one flush, then restores the previous value | A disabled service drops new tracked exceptions. If bootstrap supplied `NullTelemetryService`, enabling it or -requesting a flush still cannot send a report. +requesting a flush still cannot send a report, so the crash handler treats it as no telemetry and the dialog does not +offer Send. Never add personal data to telemetry messages or properties. The single-instance pipe name includes the user SID and must not be recorded. [DefaultTelemetryProcessor](../../src/Infrastructure/Telemetry/DefaultTelemetryProcessor.cs) diff --git a/src/Host/BootstrapContext.cs b/src/Host/BootstrapContext.cs index 1a730d8a..84d4f818 100644 --- a/src/Host/BootstrapContext.cs +++ b/src/Host/BootstrapContext.cs @@ -14,18 +14,21 @@ namespace Sqlbi.Bravo.Host; /// internal sealed class BootstrapContext : IDisposable { + private readonly IDisposable? _exceptionHandlerAttachment; private bool _disposed; internal BootstrapContext( ILoggerFactory loggerFactory, IPolicyService policyService, IUserSettings settings, - ITelemetryService telemetry) + ITelemetryService telemetry, + IDisposable? exceptionHandlerAttachment = null) { LoggerFactory = loggerFactory; PolicyService = policyService; Settings = settings; Telemetry = telemetry; + _exceptionHandlerAttachment = exceptionHandlerAttachment; } public ILoggerFactory LoggerFactory { get; } @@ -40,6 +43,8 @@ public void Dispose() _disposed = true; + // Detached first, so that an exception raised while the services are disposed is not reported through them. + _exceptionHandlerAttachment?.Dispose(); Telemetry.Dispose(); // LoggerFactory is disposed last so all other services can safely log during their disposal. LoggerFactory.Dispose(); diff --git a/src/Host/BootstrapContextFactory.cs b/src/Host/BootstrapContextFactory.cs index 748b299d..97ec0096 100644 --- a/src/Host/BootstrapContextFactory.cs +++ b/src/Host/BootstrapContextFactory.cs @@ -13,14 +13,16 @@ internal sealed partial class BootstrapContextFactory( ILoggerFactory loggerFactory, IPolicyServiceFactory policyFactory, IUserSettingsServiceFactory settingsFactory, - ITelemetryServiceFactory telemetryFactory) + ITelemetryServiceFactory telemetryFactory, + UnhandledExceptionHandler exceptionHandler) { private readonly ILoggerFactory _loggerFactory = loggerFactory; private readonly IPolicyServiceFactory _policyFactory = policyFactory; private readonly IUserSettingsServiceFactory _settingsFactory = settingsFactory; private readonly ITelemetryServiceFactory _telemetryFactory = telemetryFactory; + private readonly UnhandledExceptionHandler _exceptionHandler = exceptionHandler; - public static BootstrapContext CreateDefault() + public static BootstrapContext CreateDefault(UnhandledExceptionHandler exceptionHandler) { var loggerFactory = CreateLoggerFactory(); try @@ -29,7 +31,8 @@ public static BootstrapContext CreateDefault() loggerFactory, new PolicyServiceFactory(), new UserSettingsServiceFactory(), - new TelemetryServiceFactory()); + new TelemetryServiceFactory(), + exceptionHandler); return factory.Create(); } @@ -40,6 +43,10 @@ public static BootstrapContext CreateDefault() } } + /// + /// Creates the bootstrap services and attaches them to the exception handler, which reports through them until + /// the context is disposed. + /// public BootstrapContext Create() { var policies = CreatePolicies(); @@ -47,8 +54,9 @@ public BootstrapContext Create() var consent = TelemetryConsent.Resolve(policies.Current, settings); var telemetry = CreateTelemetry(consent.IsEnabled); + var exceptionHandlerAttachment = _exceptionHandler.AttachServices(_loggerFactory, telemetry); - return new BootstrapContext(_loggerFactory, policies, settings, telemetry); + return new BootstrapContext(_loggerFactory, policies, settings, telemetry, exceptionHandlerAttachment); } private static ILoggerFactory CreateLoggerFactory() diff --git a/src/Host/BravoGlobalExceptionHandling.cs b/src/Host/BravoGlobalExceptionHandling.cs index 2a41d9fc..cdbbe5ac 100644 --- a/src/Host/BravoGlobalExceptionHandling.cs +++ b/src/Host/BravoGlobalExceptionHandling.cs @@ -1,4 +1,5 @@ -using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Sqlbi.Bravo.Infrastructure; +using Sqlbi.Bravo.Infrastructure.Diagnostics; namespace Sqlbi.Bravo.Host; @@ -6,7 +7,8 @@ internal static class BravoGlobalExceptionHandling { public static UnhandledExceptionHandler Configure() { - var handler = new UnhandledExceptionHandler(new ErrorReportTaskDialog()); + // The path is read on a crash: AppEnvironment initialization can itself be the failure being reported. + var handler = new UnhandledExceptionHandler(new ErrorReportTaskDialog(), () => AppEnvironment.ApplicationDataPath); GlobalExceptionHandling.Configure(handler.Handle); diff --git a/src/Infrastructure/Diagnostics/ErrorReport.cs b/src/Infrastructure/Diagnostics/ErrorReport.cs index a718ea6f..9cdb929a 100644 --- a/src/Infrastructure/Diagnostics/ErrorReport.cs +++ b/src/Infrastructure/Diagnostics/ErrorReport.cs @@ -29,11 +29,11 @@ private ErrorReport(Exception exception, EnvironmentDiagnostics diagnostics) public string Text => _text.Value; public string? FilePath { get; private set; } - public bool TrySave() + public bool TrySave(string directoryPath) { try { - var directory = Directory.CreateDirectory(AppEnvironment.ApplicationDataPath); + var directory = Directory.CreateDirectory(directoryPath); var filePath = Path.Combine(directory.FullName, "ErrorReport.txt"); File.WriteAllText(filePath, Text, Encoding.UTF8); diff --git a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs index 21cf3292..e271c302 100644 --- a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs +++ b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs @@ -26,7 +26,41 @@ public void Show(ErrorReport report, ITelemetryService? telemetry) { // Always run on a dedicated STA thread: TaskDialog and Clipboard require STA, // while the caller could be executing on any arbitrary background thread. - ProcessHelper.RunOnSTAThread(() => ShowImpl(report, telemetry)); + ProcessHelper.RunOnSTAThread(() => + { + try + { + ShowImpl(report, telemetry); + } + catch + { + // The task dialog uses application types that can be the cause of the crash. + ShowFallback(report); + throw; + } + }); + } + + private static void ShowFallback(ErrorReport report) + { + try + { + var text = new StringBuilder(); + text.AppendLine("Bravo encountered an unexpected error and will close."); + + if (report.FilePath is not null) + { + text.AppendLine(); + text.AppendLine("A crash report has been saved:"); + text.AppendLine(report.FilePath); + } + + text.AppendLine(); + text.Append(report.Exception.GetType().Name).Append(": ").Append(report.Exception.Message); + + MessageBox.Show(text.ToString(), "Bravo for Power BI", MessageBoxButtons.OK, MessageBoxIcon.Error); + } + catch { /* Ignore failures */ } } private static void ShowImpl(ErrorReport report, ITelemetryService? telemetry) diff --git a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs index a5c672f7..d6a13554 100644 --- a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs +++ b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Threading; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Sqlbi.Bravo.Infrastructure.Telemetry; @@ -10,14 +11,25 @@ namespace Sqlbi.Bravo.Infrastructure.Diagnostics; /// Handles unhandled exceptions in the application, providing a mechanism /// to report and display error information to the user. /// -internal sealed partial class UnhandledExceptionHandler(IErrorReportDialog dialog) +internal sealed partial class UnhandledExceptionHandler(IErrorReportDialog dialog, Func reportDirectoryPath) { + private static readonly ReportingServices s_noServices = new(NullLogger.Instance, Telemetry: null); + private readonly IErrorReportDialog _dialog = dialog; - private volatile ReportingServices _services = new(NullLogger.Instance, Telemetry: null); + private readonly Func _reportDirectoryPath = reportDirectoryPath; + private volatile ReportingServices _services = s_noServices; - public void AttachServices(ILoggerFactory loggerFactory, ITelemetryService telemetry) + /// + /// Reports through the given services until the returned object is disposed. + /// + public IDisposable AttachServices(ILoggerFactory loggerFactory, ITelemetryService telemetry) { - _services = new ReportingServices(loggerFactory.CreateLogger(), telemetry); + // NullTelemetryService cannot deliver anything, so the dialog must not offer to send through it. + var deliverableTelemetry = telemetry is NullTelemetryService ? null : telemetry; + + var services = new ReportingServices(loggerFactory.CreateLogger(), deliverableTelemetry); + _services = services; + return new ServicesAttachment(this, services); } public void Handle(Exception exception, ExceptionOrigin origin) @@ -36,12 +48,21 @@ public void Handle(Exception exception, ExceptionOrigin origin) return; var report = ErrorReport.Create(exception); - _ = report.TrySave(); + SaveReport(report); ShowDialog(services, report); _ = services.Telemetry?.TryFlushBeforeShutdown(10_000); } + private void SaveReport(ErrorReport report) + { + try + { + _ = report.TrySave(_reportDirectoryPath()); + } + catch { /* The dialog shows the report without a file */ } + } + private void ShowDialog(ReportingServices services, ErrorReport report) { try @@ -98,4 +119,13 @@ private static bool TryMarkHandled(Exception exception) } private sealed record ReportingServices(ILogger Logger, ITelemetryService? Telemetry); + + private sealed class ServicesAttachment(UnhandledExceptionHandler owner, ReportingServices services) : IDisposable + { + private readonly UnhandledExceptionHandler _owner = owner; + private readonly ReportingServices _services = services; + + // Detaches only its own services, which a later attachment may have replaced. + public void Dispose() => Interlocked.CompareExchange(ref _owner._services, s_noServices, _services); + } } diff --git a/src/Program.cs b/src/Program.cs index 48d47081..fc5b3c0e 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -8,16 +8,13 @@ internal static class Program [STAThread] private static void Main() { - // Visual styles first: the crash dialog is a task dialog, which requires them. ApplicationConfiguration.Initialize(); var exceptionHandler = BravoGlobalExceptionHandling.Configure(); BravoApplicationConfiguration.Initialize(); - using var context = BootstrapContextFactory.CreateDefault(); - - exceptionHandler.AttachServices(context.LoggerFactory, context.Telemetry); + using var context = BootstrapContextFactory.CreateDefault(exceptionHandler); using var app = BravoApplication .CreateBuilder(context) diff --git a/test/Bravo.Tests/Host/BootstrapContextTests.cs b/test/Bravo.Tests/Host/BootstrapContextTests.cs index 7d84d6fa..393cc41c 100644 --- a/test/Bravo.Tests/Host/BootstrapContextTests.cs +++ b/test/Bravo.Tests/Host/BootstrapContextTests.cs @@ -1,11 +1,13 @@ using System; using System.Collections.Generic; using System.IO; +using Bravo.Tests.Infrastructure.Diagnostics; using Bravo.Tests.Infrastructure.Policies; using Microsoft.Extensions.Logging; using Sqlbi.Bravo.Host; using Sqlbi.Bravo.Infrastructure.Configuration; using Sqlbi.Bravo.Infrastructure.Configuration.Settings; +using Sqlbi.Bravo.Infrastructure.Diagnostics; using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.Telemetry; using Xunit; @@ -28,6 +30,23 @@ public void Dispose_DisposesTheOwnedServicesOnce() Assert.Equal(1, loggerFactory.DisposeCount); } + [Fact] + public void Create_AttachesTheServicesToTheExceptionHandlerUntilDisposed() + { + var exceptionHandler = CreateExceptionHandler(); + var policyFactory = new FakePolicyFactory(() => new PolicyService(new FakePolicyReader())); + var settingsFactory = new FakeSettingsFactory(() => new UserSettings()); + var factory = new BootstrapContextFactory(new FakeLoggerFactory(), policyFactory, settingsFactory, new FakeTelemetryFactory(), exceptionHandler); + + var context = factory.Create(); + var telemetry = Assert.IsType(context.Telemetry); + exceptionHandler.Handle(new InvalidOperationException("attached"), ExceptionOrigin.UnobservedTask); + context.Dispose(); + exceptionHandler.Handle(new InvalidOperationException("detached"), ExceptionOrigin.UnobservedTask); + + Assert.Equal("attached", Assert.Single(telemetry.Exceptions).Message); + } + [Theory] [InlineData(null, true, true)] [InlineData(null, false, false)] @@ -43,7 +62,7 @@ public void Create_LoadsPoliciesOnceBeforeCreatingTelemetry(bool? policy, bool p var policyFactory = new FakePolicyFactory(() => policyService); var settingsFactory = new FakeSettingsFactory(() => settings); var telemetryFactory = new FakeTelemetryFactory(); - var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory, CreateExceptionHandler()); Assert.Equal(0, reader.ReadCount); Assert.Equal(0, policyFactory.CreateCount); @@ -73,7 +92,7 @@ public void Create_ReadFailurePropagatesBeforeTelemetryIsCreated() var policyFactory = new FakePolicyFactory(() => new PolicyService(reader)); var settingsFactory = new FakeSettingsFactory(() => new UserSettings()); var telemetryFactory = new FakeTelemetryFactory(); - var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory, CreateExceptionHandler()); Assert.Same(failure, Assert.Throws(() => factory.Create())); Assert.Equal(0, settingsFactory.CreateCount); @@ -89,7 +108,7 @@ public void Create_PolicyCreationFailurePropagatesBeforeCreatingOtherServices() var policyFactory = new FakePolicyFactory(() => throw failure); var settingsFactory = new FakeSettingsFactory(() => new UserSettings()); var telemetryFactory = new FakeTelemetryFactory(); - var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory, CreateExceptionHandler()); Assert.Same(failure, Assert.Throws(() => factory.Create())); Assert.Equal(0, settingsFactory.CreateCount); @@ -105,7 +124,7 @@ public void Create_SettingsCreationFailureUsesDefaultsCreatesTelemetryAndLogsErr var policyFactory = new FakePolicyFactory(() => new PolicyService(new FakePolicyReader())); var settingsFactory = new FakeSettingsFactory(() => throw failure); var telemetryFactory = new FakeTelemetryFactory(); - var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory, CreateExceptionHandler()); using var context = factory.Create(); @@ -127,7 +146,7 @@ public void Create_TelemetryFailureUsesDisabledServiceAndLogsError() var policyFactory = new FakePolicyFactory(() => new PolicyService(new FakePolicyReader())); var settingsFactory = new FakeSettingsFactory(() => new UserSettings()); var telemetryFactory = new FakeTelemetryFactory { Failure = failure }; - var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory, CreateExceptionHandler()); using var context = factory.Create(); @@ -174,4 +193,7 @@ public ITelemetryService Create(bool enabled) return new FakeTelemetry { TelemetryEnabled = enabled }; } } + + private static UnhandledExceptionHandler CreateExceptionHandler() + => new(new RecordingDialog(), () => throw new InvalidOperationException("No report is saved in these tests.")); } diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs index bfdfa830..3960d0eb 100644 --- a/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs @@ -1,4 +1,5 @@ -using Sqlbi.Bravo.Infrastructure.Diagnostics; +using System; +using Sqlbi.Bravo.Infrastructure.Diagnostics; using Sqlbi.Bravo.Infrastructure.Telemetry; namespace Bravo.Tests.Infrastructure.Diagnostics; @@ -6,6 +7,17 @@ namespace Bravo.Tests.Infrastructure.Diagnostics; internal sealed class RecordingDialog : IErrorReportDialog { public int ShowCount { get; private set; } + public ErrorReport? LastReport { get; private set; } + public ITelemetryService? LastTelemetry { get; private set; } + public Exception? ExceptionToThrow { get; init; } - public void Show(ErrorReport report, ITelemetryService? telemetry) => ShowCount++; + public void Show(ErrorReport report, ITelemetryService? telemetry) + { + ShowCount++; + LastReport = report; + LastTelemetry = telemetry; + + if (ExceptionToThrow is not null) + throw ExceptionToThrow; + } } diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs index 8211bdcc..10fce73b 100644 --- a/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs @@ -101,7 +101,7 @@ public void RuntimeCallbacks_UnobservedTask_ReportsWithoutDialogOrFlush() { var dialog = new RecordingDialog(); var telemetry = Substitute.For(); - var handler = new UnhandledExceptionHandler(dialog); + var handler = new UnhandledExceptionHandler(dialog, () => throw new InvalidOperationException("No report is saved for an unobserved task.")); handler.AttachServices(NullLoggerFactory.Instance, telemetry); var args = new UnobservedTaskExceptionEventArgs(new AggregateException("unobserved")); SetHandler(handler.Handle); diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs index b53610ee..e722fee9 100644 --- a/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs @@ -1,35 +1,138 @@ using System; +using System.IO; using Bravo.Tests.Host; using Microsoft.Extensions.Logging; using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Sqlbi.Bravo.Infrastructure.Telemetry; using Xunit; namespace Bravo.Tests.Infrastructure.Diagnostics; -public class UnhandledExceptionHandlerTests +public sealed class UnhandledExceptionHandlerTests : IDisposable { + private readonly string _reportDirectory = Path.Combine(Path.GetTempPath(), $"Bravo.Tests.{Guid.NewGuid():N}"); + [Fact] - public void Handle_UnobservedTaskBeforeAttachServices_DoesNotThrow() + public void Handle_FatalBeforeAttachServices_SavesReportAndShowsDialogWithoutTelemetry() { - var handler = new UnhandledExceptionHandler(new RecordingDialog()); + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); - handler.Handle(new InvalidOperationException("unobserved"), ExceptionOrigin.UnobservedTask); + Assert.Equal(1, dialog.ShowCount); + Assert.Null(dialog.LastTelemetry); + Assert.True(File.Exists(dialog.LastReport?.FilePath)); } [Fact] - public void Handle_UnobservedTaskAfterAttachServices_LogsAndTracks() + public void Handle_FatalAfterAttachServices_LogsTracksAndPassesTelemetryToDialog() { var loggerFactory = new FakeLoggerFactory(); var telemetry = new FakeTelemetry(); - var handler = new UnhandledExceptionHandler(new RecordingDialog()); + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + var exception = new InvalidOperationException("fatal"); + + using var _ = handler.AttachServices(loggerFactory, telemetry); + handler.Handle(exception, ExceptionOrigin.AppDomain); + + Assert.Same(exception, Assert.Single(loggerFactory.Entries).Exception); + Assert.Same(exception, Assert.Single(telemetry.Exceptions)); + Assert.Same(telemetry, dialog.LastTelemetry); + } + + [Fact] + public void Handle_DialogThrows_LogsTheFailureWithoutThrowing() + { + var loggerFactory = new FakeLoggerFactory(); + var dialogFailure = new InvalidOperationException("dialog"); + var handler = CreateHandler(new RecordingDialog { ExceptionToThrow = dialogFailure }); + + using var _ = handler.AttachServices(loggerFactory, new FakeTelemetry()); + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Contains(loggerFactory.Entries, entry => ReferenceEquals(entry.Exception, dialogFailure)); + } + + [Fact] + public void Handle_FatalAfterAttachmentDisposed_DoesNotUseTheServices() + { + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + + handler.AttachServices(loggerFactory, telemetry).Dispose(); + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Empty(loggerFactory.Entries); + Assert.Empty(telemetry.Exceptions); + Assert.Null(dialog.LastTelemetry); + } + + [Fact] + public void Handle_EarlierAttachmentDisposed_KeepsTheLaterServices() + { + var telemetry = new FakeTelemetry(); + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + var earlier = handler.AttachServices(new FakeLoggerFactory(), new FakeTelemetry()); + + using var _ = handler.AttachServices(new FakeLoggerFactory(), telemetry); + earlier.Dispose(); + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Same(telemetry, dialog.LastTelemetry); + } + + [Fact] + public void Handle_AttachedNullTelemetryService_PassesNoTelemetryToDialog() + { + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + + using var _ = handler.AttachServices(new FakeLoggerFactory(), new NullTelemetryService()); + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Null(dialog.LastTelemetry); + } + + [Fact] + public void Handle_ReportDirectoryUnavailable_ShowsDialogWithoutFile() + { + var dialog = new RecordingDialog(); + var handler = new UnhandledExceptionHandler(dialog, () => throw new TypeInitializationException("AppEnvironment", null)); + + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Equal(1, dialog.ShowCount); + Assert.Null(dialog.LastReport?.FilePath); + } + + [Fact] + public void Handle_UnobservedTaskAfterAttachServices_LogsAndTracksWithoutDialog() + { + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); var exception = new InvalidOperationException("unobserved"); - handler.AttachServices(loggerFactory, telemetry); + using var _ = handler.AttachServices(loggerFactory, telemetry); handler.Handle(exception, ExceptionOrigin.UnobservedTask); var entry = Assert.Single(loggerFactory.Entries); Assert.Equal(LogLevel.Error, entry.Level); - Assert.Same(exception, entry.Exception); Assert.Same(exception, Assert.Single(telemetry.Exceptions)); + Assert.Equal(0, dialog.ShowCount); + } + + public void Dispose() + { + if (Directory.Exists(_reportDirectory)) + Directory.Delete(_reportDirectory, recursive: true); } + + private UnhandledExceptionHandler CreateHandler(IErrorReportDialog dialog) => new(dialog, () => _reportDirectory); }