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/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 f953fcd7..227c1cee 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,17 @@ 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 `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. `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 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 +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. +`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. @@ -83,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 e776cdb8..cdbbe5ac 100644 --- a/src/Host/BravoGlobalExceptionHandling.cs +++ b/src/Host/BravoGlobalExceptionHandling.cs @@ -1,20 +1,21 @@ -using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Sqlbi.Bravo.Infrastructure; +using Sqlbi.Bravo.Infrastructure.Diagnostics; 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); + // 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); #if DEBUG CrashTest.RunIfRequested(); #endif + + return handler; } } 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 fd2ef75b..e271c302 100644 --- a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs +++ b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs @@ -14,35 +14,68 @@ 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(() => + { + 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 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 +99,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 +132,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..d6a13554 100644 --- a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs +++ b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using System.Threading; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Sqlbi.Bravo.Infrastructure.Telemetry; namespace Sqlbi.Bravo.Infrastructure.Diagnostics; @@ -9,15 +11,32 @@ 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, Func reportDirectoryPath) { + private static readonly ReportingServices s_noServices = new(NullLogger.Instance, Telemetry: null); + private readonly IErrorReportDialog _dialog = dialog; - private readonly ITelemetryService _telemetry = telemetry; - private readonly ILogger _logger = loggerFactory.CreateLogger(); + private readonly Func _reportDirectoryPath = reportDirectoryPath; + private volatile ReportingServices _services = s_noServices; + + /// + /// Reports through the given services until the returned object is disposed. + /// + public IDisposable AttachServices(ILoggerFactory loggerFactory, ITelemetryService 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) { - 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. @@ -29,23 +48,49 @@ public void Handle(Exception exception, ExceptionOrigin origin) return; var report = ErrorReport.Create(exception); - _ = report.TrySave(); - _dialog.Show(report); + 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 */ } + } - _ = _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 +117,15 @@ private static bool TryMarkHandled(Exception exception) return true; } } + + 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 9f6eb41b..fc5b3c0e 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -10,11 +10,11 @@ private static void Main() { ApplicationConfiguration.Initialize(); - BravoApplicationConfiguration.Initialize(); + var exceptionHandler = BravoGlobalExceptionHandling.Configure(); - using var context = BootstrapContextFactory.CreateDefault(); + BravoApplicationConfiguration.Initialize(); - BravoGlobalExceptionHandling.Configure(context); + 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 new file mode 100644 index 00000000..3960d0eb --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/DiagnosticsFakes.cs @@ -0,0 +1,23 @@ +using System; +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 ErrorReport? LastReport { get; private set; } + public ITelemetryService? LastTelemetry { get; private set; } + public Exception? ExceptionToThrow { get; init; } + + 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 b4e8806a..10fce73b 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, () => 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); @@ -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..e722fee9 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/UnhandledExceptionHandlerTests.cs @@ -0,0 +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 sealed class UnhandledExceptionHandlerTests : IDisposable +{ + private readonly string _reportDirectory = Path.Combine(Path.GetTempPath(), $"Bravo.Tests.{Guid.NewGuid():N}"); + + [Fact] + public void Handle_FatalBeforeAttachServices_SavesReportAndShowsDialogWithoutTelemetry() + { + var dialog = new RecordingDialog(); + var handler = CreateHandler(dialog); + + handler.Handle(new InvalidOperationException("fatal"), ExceptionOrigin.AppDomain); + + Assert.Equal(1, dialog.ShowCount); + Assert.Null(dialog.LastTelemetry); + Assert.True(File.Exists(dialog.LastReport?.FilePath)); + } + + [Fact] + public void Handle_FatalAfterAttachServices_LogsTracksAndPassesTelemetryToDialog() + { + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + 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"); + + 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, 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); +}