diff --git a/AGENTS.md b/AGENTS.md index 1e029e10..6c409028 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,6 +56,8 @@ asked for it. | Document | Read it before | | --- | --- | +| [docs/design/architecture.md](docs/design/architecture.md) | changing startup, service ownership, UI threading, frontend/host contracts, API authentication, single-instance activation, or policy composition. | +| [docs/design/diagnostics.md](docs/design/diagnostics.md) | catching, logging, or reporting an exception, or touching the crash dialog, the diagnostics pane, or telemetry. | | [docs/design/versioning.md](docs/design/versioning.md) | touching `version.json`, the build number, or any code that compares versions. | | [ADR 0001](docs/adr/0001-releases-without-update-channels.md) | touching the update check, the update feed, or the release of a version. | | [ADR 0002](docs/adr/0002-shared-msi-upgrade-code.md) | changing `UpgradeCode`, `MajorUpgrade` or the product version in `installer/wix`. | diff --git a/docs/adr/0003-fatal-exceptions-end-the-process.md b/docs/adr/0003-fatal-exceptions-end-the-process.md new file mode 100644 index 00000000..9ed7dc41 --- /dev/null +++ b/docs/adr/0003-fatal-exceptions-end-the-process.md @@ -0,0 +1,39 @@ +# 0003. Fatal exceptions end the process after a report + +- Status: Accepted +- Date: 2026-10-20 + +## Context + +Bravo is a thin WinForms shell around a WebView2 UI and an in-process API. An exception that escapes a thread, +the UI thread included, leaves that shell in an unknown state. The users are business analysts, who need to know +that the application failed and need something they can pass to support. + +## Decision + +- `GlobalExceptionHandling` sets `UnhandledExceptionMode.ThrowException` for every thread and handles + `AppDomain.UnhandledException` and `TaskScheduler.UnobservedTaskException`. +- A fatal exception is logged, tracked through telemetry when consent allows it, saved to `ErrorReport.txt` in the + application data folder and shown in a task dialog. The runtime then ends the process. +- When telemetry is disabled, the dialog offers to send that one report; nothing is sent without that action. +- An unobserved task exception is logged and tracked, without a dialog, and the process continues. +- If the handler itself fails, `Environment.FailFast` ends the process. + +## Rejected alternatives + +- Continue after a UI-thread exception (`CatchException` with `Application.ThreadException`): the window stays + open in a broken state, and exceptions on other threads are not covered. +- Keep the process alive at any cost, parking faulting threads: the API and the WebView2 bridge cannot be trusted + after an unknown failure. +- Report through Windows Error Reporting: it adds native interop and an approval flow, for a report that a text + file and telemetry already carry. +- Track fatal exceptions only after the user confirms: crashes would be lost whenever the user closes the dialog, + although they consented to telemetry. + +## Consequences + +- 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 + [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/0004-bootstrap-services-outside-the-container.md b/docs/adr/0004-bootstrap-services-outside-the-container.md new file mode 100644 index 00000000..e90d5306 --- /dev/null +++ b/docs/adr/0004-bootstrap-services-outside-the-container.md @@ -0,0 +1,38 @@ +# 0004. Bootstrap services are created outside the container + +- Status: Accepted +- Date: 2026-10-20 + +## Context + +Logging, policies, user settings and telemetry are needed before the ASP.NET Core host exists: the global +exception handler and the crash dialog use them, and a failure while building the host must still be reported. +The host has its own dependency injection container. + +## Decision + +- `BootstrapContextFactory` creates the logger factory, the policy service, the user settings and the telemetry + service, in that order, before the host. `BootstrapContext` owns them and disposes telemetry before the logger + factory. +- The host container receives them as existing instances, so it resolves them but never disposes them. Its default + `ILoggerFactory` is replaced by the bootstrap one: bootstrap and host share one logging pipeline. +- No other container exists. Services that only the host needs are registered in the host container. + +## Rejected alternatives + +- A second `ServiceProvider` for the bootstrap: two root containers with separate lifetimes, singletons duplicated + when a type rather than an instance is registered twice, and an ambiguous disposal order. It also turns bootstrap + code into a service locator. +- Lazily created static services that construct one another: their creation order depends on first use, which + caused re-entrancy, and their failures surface at an arbitrary caller. +- Registering factories in the host container: the container would dispose services that the crash path still + needs after the host is gone. + +## Consequences + +- A registration in `BravoServiceCollectionExtensions` that turns one of these instances into a container-created + service changes who disposes it and when; see + [architecture](../design/architecture.md#composition-and-resource-ownership). +- The bootstrap order is explicit code, and each step can degrade independently; see + [ADR 0005](0005-policy-failures-stop-startup.md). +- Tests build a `BootstrapContext` from fakes without starting the host. diff --git a/docs/adr/0005-policy-failures-stop-startup.md b/docs/adr/0005-policy-failures-stop-startup.md new file mode 100644 index 00000000..672f7cf1 --- /dev/null +++ b/docs/adr/0005-policy-failures-stop-startup.md @@ -0,0 +1,33 @@ +# 0005. A policy read failure stops the startup + +- Status: Accepted +- Date: 2026-10-20 + +## Context + +Administrators configure Bravo through group policies in the registry: they can disable telemetry, update checks, +built-in or custom templates, or force the system browser for sign-in. User settings and telemetry are created in the +same bootstrap step, and each can fail: a corrupt settings file, an unreachable telemetry configuration, a registry +key that cannot be read. + +## Decision + +- `BootstrapContextFactory` reads the policies eagerly. A failure propagates and the application does not start. +- A user settings failure is logged and the defaults are used. +- A telemetry failure is logged and `NullTelemetryService` is used. + +## Rejected alternatives + +- Treat unreadable policies as not configured: Bravo would start with the restrictions an administrator imposed + silently removed, for example sending telemetry where a policy forbids it. +- Stop the startup on any bootstrap failure: a corrupt settings file or a telemetry problem would make Bravo + unusable, although neither puts an administrative restriction at risk. + +## 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 + [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. +- A new policy that restricts behavior is safe by construction: it cannot be bypassed by a read failure. diff --git a/docs/adr/0006-persistent-telemetry-channel.md b/docs/adr/0006-persistent-telemetry-channel.md new file mode 100644 index 00000000..b4b18d2d --- /dev/null +++ b/docs/adr/0006-persistent-telemetry-channel.md @@ -0,0 +1,34 @@ +# 0006. Telemetry is sent through a persistent channel + +- Status: Accepted +- Date: 2026-10-20 + +## Context + +Host telemetry carries exceptions, and the most valuable ones are tracked right before the process ends. With the +in-memory channel a transmission that fails or is still pending at exit is lost, and `Flush` gives no confirmation +of delivery. Consent comes from a policy or from the user preference and can change while Bravo runs. + +## Decision + +- `TelemetryService` uses `ServerTelemetryChannel` with a local store under `.cache\.telemetry` in the application + data folder. Transmissions that cannot be sent are persisted and retried at a later start. +- Every path that ends the process or confirms a report awaits `FlushAsync` with a timeout, because the + synchronous `Flush` returns before anything is sent. +- `TelemetryConsent.Resolve` decides whether telemetry is enabled: a configured policy wins over the user + preference. Every caller that enables or disables telemetry resolves the consent first. + +## Rejected alternatives + +- Keep the in-memory channel and call `Flush` after each exception: the call blocks on a network request, it + cannot report a failure, and anything still pending at exit is lost. +- Send crash reports through a separate endpoint or file upload: a second transport, with its own consent handling, + for data that the telemetry pipeline already carries. + +## Consequences + +- An exception tracked offline reaches the service at a later start instead of being lost. +- `FlushAsync` reports that data left the process, sent or persisted, not that it was ingested. +- Transmissions in the store are sent even after telemetry is disabled: they were generated while telemetry was + enabled and consent was given. +- The store folder must exist before the channel starts; `BravoApplicationConfiguration` creates it. diff --git a/docs/design/architecture.md b/docs/design/architecture.md new file mode 100644 index 00000000..88945c19 --- /dev/null +++ b/docs/design/architecture.md @@ -0,0 +1,92 @@ +# Architecture and runtime boundaries + +## Composition and resource ownership + +[BravoApplication.Run](../../src/Host/BravoApplication.cs) resolves the single-instance role before starting +Kestrel: a secondary process redirects its activation and exits without starting a server or a window. It does so +after the bootstrap services and the host are built, so a secondary process still pays for both. + +| 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 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 | + +[BravoServiceCollectionExtensions](../../src/Host/BravoServiceCollectionExtensions.cs) registers the bootstrap +services as existing instances, which the container does not dispose. Replacing them with container-owned factories +moves disposal to the container and must keep the order above. Global exception handling uses the bootstrap +services, not the container; its coverage is in [diagnostics](diagnostics.md#startup-coverage-and-fatal-reporting). +Why the bootstrap services live outside the container: +[ADR 0004](../adr/0004-bootstrap-services-outside-the-container.md). + +## UI and server threads + +[AppWindow](../../src/Infrastructure/AppWindow.cs) is created on the STA entry thread, which runs the WinForms +message loop. API work and single-instance activation callbacks do not have that UI-thread guarantee. + +- Marshal operations on the window or WebView2 to the UI thread. Activation handlers use + `ProcessHelper.InvokeOnUIThread` with the window as the control. +- Setting `SynchronizationContext.Current` is not thread dispatch. + [ProcessHelper.RunWithUISynchronizationContext](../../src/Infrastructure/Helpers/ProcessHelper.cs) changes the + ambient context only during a synchronous callback; do not treat it as permission to access controls from a + server thread. +- Keep window activation and document delivery distinct: the window can exist before WebView2 is ready to receive + a startup message. + +Violating these boundaries can cause cross-thread UI failures or lost activation requests. The +[single-instance contract](#single-instance-contract) describes the current startup limitation. + +## Frontend and API contract + +[AppWindow](../../src/Infrastructure/AppWindow.cs) injects `CONFIG` with the API address, token, options, policies +and environment information. The frontend uses [Host](../../src/Scripts/controllers/host.ts) for API calls and +WebView messages for desktop integration. The C# serializer and TypeScript consumers must agree on field names, +nullability and enum values; changing only one side can compile in C# while breaking the UI. Follow the full-build +requirement in [AGENTS.md](../../AGENTS.md#build-and-test) for changes to this contract. + +[BravoApplicationBuilder](../../src/Host/BravoApplicationBuilder.cs) configures Kestrel and endpoint authorization: + +| Build | Listening address | Controller authorization | +| --- | --- | --- | +| Debug | IPv4 loopback, port 5000 | No blanket authorization requirement; Swagger enabled | +| Release | IPv4 loopback, dynamically assigned port | Controllers require authorization | + +[AppAuthenticationHandler](../../src/Infrastructure/Authentication/AppAuthenticationHandler.cs) requires both a +loopback remote address and the application token in the `Authorization` header. The template-development token is +accepted only under the template-development controller path. Loopback binding and token validation are separate +checks; neither should be removed on the assumption that the other is sufficient. + +[HostingExtensions.AddAndConfigureProblemDetails](../../src/Infrastructure/Extensions/HostingExtensions.cs) maps +exceptions to responses in order, first match wins. The `SocketException` mapping also matches other exception types +that wrap a socket failure, such as `AMO.ConnectionException`, so it stays after the specific mappings and before the +catch-all. Reporting at the API boundary is in [diagnostics](diagnostics.md#api-boundary). + +## Single-instance contract + +[InstancePipeName](../../src/Host/InstancePipeName.cs) identifies one instance per Windows session and user SID, +independent of installation mode and elevation. Keep its application identifier, scope identifier and naming format +stable across releases: changing them allows versions that share user data to run as independent owners. + +[SingleInstanceServer](../../src/Infrastructure/SingleInstance/SingleInstanceServer.cs) arbitrates ownership +through a named pipe with one server instance. It disconnects between clients without releasing the pipe name. Do +not replace that with disposal and recreation between requests: another process could claim ownership in the gap. + +Current activation limits in [BravoApplicationInstance](../../src/Host/BravoApplicationInstance.cs): + +- A delivered payload does not guarantee that the requested document opened. Requests arriving before the window + or WebView subscriptions are ready are not buffered. +- Activation callbacks can run concurrently and in a different order from arrival. +- Different elevation levels still share an instance name, but pipe access can prevent activation redirection. A + redirect failure is logged and tracked without a user-facing dialog. + +## Configuration boundaries + +[PolicyService](../../src/Infrastructure/Policies/PolicyService.cs) exposes a snapshot initialized once. +[RegistryPolicyReader](../../src/Infrastructure/Policies/RegistryPolicyReader.cs) merges machine and user policies, +with configured machine values taking precedence. A null policy value means not configured; `false` is an explicit +policy, not a request to use a default. + +User preferences remain mutable through [UserPreferences](../../src/Infrastructure/Configuration/UserPreferences.cs), +and saving them does not reload policies. A consumer that combines a policy with a preference applies the precedence +itself, as [TelemetryConsent](../../src/Infrastructure/Telemetry/TelemetryConsent.cs) does for telemetry. diff --git a/docs/design/diagnostics.md b/docs/design/diagnostics.md new file mode 100644 index 00000000..f953fcd7 --- /dev/null +++ b/docs/design/diagnostics.md @@ -0,0 +1,109 @@ +# Diagnostics + +Exception reporting in the .NET host. The frontend has a separate +[telemetry client](../../src/Scripts/controllers/telemetry.ts); host flush and crash handling do not control it. + +## Reporting paths + +| 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 | +| `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) | + +The two runtime callbacks use +[GlobalExceptionHandling](../../src/Infrastructure/Diagnostics/GlobalExceptionHandling.cs). An unobserved task +exception is not a fatal crash, even though its telemetry has `ExceptionKind = Unhandled`. Why a fatal exception +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. +[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 +([ADR 0005](../adr/0005-policy-failures-stop-startup.md)). + +[UnhandledExceptionHandler](../../src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs) logs and tracks each +callback before attempting a fatal report. Logging and tracking failures are isolated so one does not prevent the +other. For a fatal exception, a marker in `Exception.Data` suppresses a repeated dialog and report; it does not +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. +[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. + +The fatal-handler guard suppresses another fatal arrival while a fatal handler runs. It does not coordinate +simultaneous crashes or guarantee that the first dialog remains open until the user responds. Unobserved task +reporting does not acquire that guard, so it cannot suppress a fatal report. + +If the global handler throws, `GlobalExceptionHandling` calls `Environment.FailFast`. Reporting failures must +therefore be contained where recovery is possible. Never show UI or wait for a shutdown flush on the unobserved-task +callback: it runs on the finalizer thread. + +## Handled exceptions + +For an operation with initialized logging and telemetry, report a caught failure through both `ILogger` and +`ITelemetryService.TrackException`: consent can disable the remote record, while the local log remains. Use the +reporting type's logger and add telemetry properties only for a specific diagnostic query. A failure that propagates +to the API boundary or to the global handler is reported there; reporting it again in a lower layer records it +twice. These are rules for changes; existing catch sites do not all follow them yet. + +### API boundary + +[HostingExtensions.AddAndConfigureProblemDetails](../../src/Infrastructure/Extensions/HostingExtensions.cs) owns +reporting for exceptions mapped to API responses. Its mappings track exceptions. Specific mappings add host +diagnostics at `Verbose`; the catch-all mapping adds diagnostics regardless of that setting. Exception details are +enabled for the UI to display and copy. Do not add duplicate tracking in each controller for exceptions that +propagate to these mappings. + +## Local logging and the diagnostics pane + +The host logger factory, created by [BootstrapContextFactory](../../src/Host/BootstrapContextFactory.cs), writes +`Warning` and above of every category, ASP.NET Core and Kestrel included, to the Windows Event Log and to +[AppEnvironment.Diagnostics](../../src/Infrastructure/AppEnvironment.cs). That store also receives explicit +`AddDiagnostics` calls, lives in memory for the whole process and has no eviction. The +[frontend logger](../../src/Scripts/controllers/logger.ts) polls `GetDiagnostics` only at diagnostic level +`Verbose`: a message can be stored without ever appearing in the pane. + +## Telemetry consent + +The host uses Application Insights through [TelemetryService](../../src/Infrastructure/Telemetry/TelemetryService.cs). +The `TelemetryEnabled` setter assigns the value it receives, so every caller resolves the consent first: + +| Entry point | Current behavior | +| --- | --- | +| Bootstrap | [TelemetryConsent.Resolve](../../src/Infrastructure/Telemetry/TelemetryConsent.cs) gives a configured policy precedence over the user preference | +| Preferences saved through `UpdateOptions` | [ApplicationController](../../src/Controllers/ApplicationController.cs) resolves the consent the same way before assigning it | +| 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. + +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) +redacts supported sensitive tags in exception messages; it is not a general sanitizer for arbitrary properties or +untagged data. + +### Flush and shutdown + +Why the channel persists telemetry: [ADR 0006](../adr/0006-persistent-telemetry-channel.md). +`TelemetryService.TrackException` calls the synchronous `Flush`, which moves telemetry into the channel's +transmission pipeline and returns without waiting for delivery. In the +[ServerTelemetryChannel 2.23.0 contract](https://raw.githubusercontent.com/microsoft/ApplicationInsights-dotnet/2.23.0/BASE/src/ServerTelemetryChannel/ServerTelemetryChannel.cs), +`FlushAsync` succeeds once the data has left the process, sent or persisted locally; success does not prove +ingestion. Every path that ends the process or confirms a report awaits `FlushAsync` with a bounded timeout: +`TelemetryService.Dispose`, the fatal handler after the dialog closes, and the crash dialog's Send. Do not replace +that wait with the synchronous `Flush`. + +`TryFlushBeforeShutdown` blocks the caller and returns `false` only on cancellation; other exceptions propagate. + +The channel stores unsent transmissions under `.cache\.telemetry` in the application data folder when it can persist +them; data still in memory at termination is lost. The +[telemetry tests](../../test/Bravo.Tests/Infrastructure/Telemetry/TelemetryServiceTests.cs) cover delivery at +shutdown and persistence when the endpoint is unavailable. diff --git a/src/Host/BootstrapContext.cs b/src/Host/BootstrapContext.cs new file mode 100644 index 00000000..1a730d8a --- /dev/null +++ b/src/Host/BootstrapContext.cs @@ -0,0 +1,47 @@ +using System; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Infrastructure.Configuration.Settings; +using Sqlbi.Bravo.Infrastructure.Policies; +using Sqlbi.Bravo.Infrastructure.Telemetry; + +namespace Sqlbi.Bravo.Host; + +/// +/// Holds the services used to bootstrap the application. +/// +/// +/// The context owns the lifetime of the services it holds. +/// +internal sealed class BootstrapContext : IDisposable +{ + private bool _disposed; + + internal BootstrapContext( + ILoggerFactory loggerFactory, + IPolicyService policyService, + IUserSettings settings, + ITelemetryService telemetry) + { + LoggerFactory = loggerFactory; + PolicyService = policyService; + Settings = settings; + Telemetry = telemetry; + } + + public ILoggerFactory LoggerFactory { get; } + public IPolicyService PolicyService { get; } + public IUserSettings Settings { get; } + public ITelemetryService Telemetry { get; } + + public void Dispose() + { + if (_disposed) + return; + + _disposed = true; + + Telemetry.Dispose(); + // LoggerFactory is disposed last so all other services can safely log during their disposal. + LoggerFactory.Dispose(); + } +} diff --git a/src/Host/BootstrapContextFactory.Log.cs b/src/Host/BootstrapContextFactory.Log.cs new file mode 100644 index 00000000..628f64d0 --- /dev/null +++ b/src/Host/BootstrapContextFactory.Log.cs @@ -0,0 +1,13 @@ +using System; +using Microsoft.Extensions.Logging; + +namespace Sqlbi.Bravo.Host; + +internal sealed partial class BootstrapContextFactory +{ + [LoggerMessage(EventId = 1, Level = LogLevel.Error, Message = "Telemetry could not be created. Telemetry is disabled for this session.")] + private static partial void LogTelemetryCreationFailed(ILogger logger, Exception exception); + + [LoggerMessage(EventId = 2, Level = LogLevel.Error, Message = "User settings could not be created. Default settings will be used for this session.")] + private static partial void LogUserSettingsCreationFailed(ILogger logger, Exception exception); +} diff --git a/src/Host/BootstrapContextFactory.cs b/src/Host/BootstrapContextFactory.cs new file mode 100644 index 00000000..748b299d --- /dev/null +++ b/src/Host/BootstrapContextFactory.cs @@ -0,0 +1,109 @@ +using System; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.EventLog; +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; + +namespace Sqlbi.Bravo.Host; + +internal sealed partial class BootstrapContextFactory( + ILoggerFactory loggerFactory, + IPolicyServiceFactory policyFactory, + IUserSettingsServiceFactory settingsFactory, + ITelemetryServiceFactory telemetryFactory) +{ + private readonly ILoggerFactory _loggerFactory = loggerFactory; + private readonly IPolicyServiceFactory _policyFactory = policyFactory; + private readonly IUserSettingsServiceFactory _settingsFactory = settingsFactory; + private readonly ITelemetryServiceFactory _telemetryFactory = telemetryFactory; + + public static BootstrapContext CreateDefault() + { + var loggerFactory = CreateLoggerFactory(); + try + { + var factory = new BootstrapContextFactory( + loggerFactory, + new PolicyServiceFactory(), + new UserSettingsServiceFactory(), + new TelemetryServiceFactory()); + + return factory.Create(); + } + catch + { + loggerFactory.Dispose(); + throw; + } + } + + public BootstrapContext Create() + { + var policies = CreatePolicies(); + var settings = CreateSettings(); + + var consent = TelemetryConsent.Resolve(policies.Current, settings); + var telemetry = CreateTelemetry(consent.IsEnabled); + + return new BootstrapContext(_loggerFactory, policies, settings, telemetry); + } + + private static ILoggerFactory CreateLoggerFactory() + { + return LoggerFactory.Create((builder) => + { + builder.AddEventSourceLogger(); + builder.AddEventLog(); + builder.AddFilter((level) => level >= LogLevel.Warning); + builder.AddDiagnosticMessages(); + builder.AddFilter((level) => level >= LogLevel.Warning); +#if DEBUG + builder.AddConsole(); + builder.AddDebug(); +#endif + }); + } + + /// + /// Policy initialization failures must halt the application startup + /// to prevent bypassing administrative restrictions. + /// + private IPolicyService CreatePolicies() + { + var factory = _policyFactory.Create(); + + // Force instantiation to detect any potential exceptions + _ = factory.Current; + + return factory; + } + + private IUserSettings CreateSettings() + { + try + { + return _settingsFactory.Create(); + } + catch (Exception ex) + { + LogUserSettingsCreationFailed(_loggerFactory.CreateLogger(), ex); + return new UserSettings(); + } + } + + private ITelemetryService CreateTelemetry(bool enabled) + { + try + { + return _telemetryFactory.Create(enabled); + } + catch (Exception ex) + { + LogTelemetryCreationFailed(_loggerFactory.CreateLogger(), ex); + return new NullTelemetryService(); + } + } +} diff --git a/src/Host/BravoApplication.cs b/src/Host/BravoApplication.cs index caa2bc4e..8e99c75d 100644 --- a/src/Host/BravoApplication.cs +++ b/src/Host/BravoApplication.cs @@ -14,9 +14,9 @@ internal sealed class BravoApplication : IDisposable { private readonly WebApplication _innerApplication; - public static BravoApplicationBuilder CreateBuilder(BravoApplicationInitializationContext context) + public static BravoApplicationBuilder CreateBuilder(BootstrapContext bootstrap) { - return new BravoApplicationBuilder(context); + return new BravoApplicationBuilder(bootstrap); } internal BravoApplication(WebApplication innerApplication) @@ -24,31 +24,40 @@ internal BravoApplication(WebApplication innerApplication) _innerApplication = innerApplication; } + internal IServiceProvider Services => _innerApplication.Services; + /// /// Runs the application and starts the message loop. /// public void Run() { + var instance = Services.GetRequiredService(); + if (!instance.IsPrimary) + { + instance.RedirectActivationToPrimary(); + return; + } + _innerApplication.Start(); try { // Create the main window through DI without registering it. // Its lifetime is owned by the WinForms message loop rather than the service container. - using var window = ActivatorUtilities.CreateInstance(_innerApplication.Services); -/* - // If the host stops independently of the UI, the message loop must also end. - // Otherwise the window could outlive the backend it depends on. - // ApplicationStopping is raised on a background thread, so closing the window - // is marshaled to the UI thread. - // The registration is disposed before StopAsync() is invoked below, preventing - // the normal window-close shutdown path from re-entering. - var lifetime = _innerApplication.Services.GetRequiredService(); - using var stopRegistration = lifetime.ApplicationStopping.Register(() => - { - if (window.IsHandleCreated) - window.BeginInvoke(window.Close); - }); -*/ + using var window = ActivatorUtilities.CreateInstance(Services); + + //// If the host stops independently of the UI, the message loop must also end. + //// Otherwise the window could outlive the backend it depends on. + //// ApplicationStopping is raised on a background thread, so closing the window + //// is marshaled to the UI thread. + //// The registration is disposed before StopAsync() is invoked below, preventing + //// the normal window-close shutdown path from re-entering. + //var lifetime = _innerApplication.Services.GetRequiredService(); + //using var stopRegistration = lifetime.ApplicationStopping.Register(() => + //{ + // if (window.IsHandleCreated) + // window.BeginInvoke(window.Close); + //}); + Application.Run(window); } finally diff --git a/src/Host/BravoApplicationBuilder.cs b/src/Host/BravoApplicationBuilder.cs index 1ab481d7..f283d9d4 100644 --- a/src/Host/BravoApplicationBuilder.cs +++ b/src/Host/BravoApplicationBuilder.cs @@ -5,10 +5,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.EventLog; using Sqlbi.Bravo.Infrastructure.Configuration.Settings; -using Sqlbi.Bravo.Infrastructure.Diagnostics; using Sqlbi.Bravo.Infrastructure.Services; namespace Sqlbi.Bravo.Host; @@ -16,21 +13,15 @@ namespace Sqlbi.Bravo.Host; /// /// Provides a builder for creating and configuring a instance. /// -/// -/// Everything is configured by the time the builder is returned, and is the only -/// method: the registrations are composed through , not -/// by mutating the builder from outside. Should a caller — a test, typically — ever need to override -/// a registration, expose as a pass-through property -/// then; it is deliberately not exposed while nothing needs it. -/// internal sealed class BravoApplicationBuilder { private readonly WebApplicationBuilder _innerBuilder; - internal BravoApplicationBuilder(BravoApplicationInitializationContext context) + internal BravoApplicationBuilder(BootstrapContext bootstrap) { // CreateEmptyBuilder, not CreateBuilder: no configuration sources, no environment variables and - // no implicit logging providers. Everything the host needs is declared explicitly below. + // no implicit logging providers. The logging pipeline comes from the bootstrap context, everything + // else the host needs is declared explicitly below. _innerBuilder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions { EnvironmentName = Environments.Production, @@ -38,10 +29,11 @@ internal BravoApplicationBuilder(BravoApplicationInitializationContext context) }); ConfigureHosting(_innerBuilder); - ConfigureLogging(_innerBuilder); - ConfigureServices(_innerBuilder, context); + ConfigureServices(_innerBuilder, bootstrap); } + internal IServiceCollection Services => _innerBuilder.Services; + /// /// Builds the host, configures its request pipeline, and returns the . /// @@ -77,28 +69,14 @@ private static void ConfigureHosting(WebApplicationBuilder builder) #endif } - private static void ConfigureLogging(WebApplicationBuilder builder) - { - builder.Logging.AddEventSourceLogger(); - builder.Logging.AddEventLog(); - builder.Logging.AddFilter((level) => level >= LogLevel.Warning); - builder.Logging.AddDiagnosticMessages(); - builder.Logging.AddFilter((level) => level >= LogLevel.Warning); -#if DEBUG - builder.Logging.AddConsole(); - builder.Logging.AddDebug(); -#endif - } - - private static void ConfigureServices(WebApplicationBuilder builder, BravoApplicationInitializationContext context) + private static void ConfigureServices(WebApplicationBuilder builder, BootstrapContext bootstrap) { builder.Services.AddOptions() .Configure((settings) => settings.FromCommandLineArguments()); //.ValidateDataAnnotations(); builder.Services.AddSingleton(); - builder.Services.AddBravoInitializationServices(context); builder.Services.AddBravoRestApi(); - builder.Services.AddBravoServices(); + builder.Services.AddBravoServices(bootstrap); } private static void ConfigurePipeline(WebApplication application) diff --git a/src/Host/BravoApplicationInitializer.cs b/src/Host/BravoApplicationConfiguration.cs similarity index 68% rename from src/Host/BravoApplicationInitializer.cs rename to src/Host/BravoApplicationConfiguration.cs index 6aaf6629..84c8340c 100644 --- a/src/Host/BravoApplicationInitializer.cs +++ b/src/Host/BravoApplicationConfiguration.cs @@ -10,25 +10,17 @@ namespace Sqlbi.Bravo.Host; /// -/// Initializes the application process, applying process-wide settings -/// and composing the necessary components before the host is created. +/// Configures the application environment before the application starts. /// -internal static class BravoApplicationInitializer +internal static class BravoApplicationConfiguration { - /// - /// Initializes the application process, applying process-wide settings - /// - public static BravoApplicationInitializationContext Initialize() + public static void Initialize() { - ConfigureWebProxy(); ConfigureDirectories(); + ConfigureWebProxy(); ConfigureRuntimeOptimization(); WebView2Helper.EnsureRuntimeIsInstalled(); - - var instance = BravoApplicationInstance.Create(); - - return new BravoApplicationInitializationContext(instance); } private static void ConfigureWebProxy() @@ -46,7 +38,7 @@ private static void ConfigureDirectories() private static void ConfigureRuntimeOptimization() { - ProfileOptimization.SetProfileRoot(AppEnvironment.ApplicationDataPath); + ProfileOptimization.SetProfileRoot(AppEnvironment.ApplicationCachePath); ProfileOptimization.StartProfile(".jitprofile"); } } diff --git a/src/Host/BravoApplicationInitializationContext.cs b/src/Host/BravoApplicationInitializationContext.cs deleted file mode 100644 index c2af5373..00000000 --- a/src/Host/BravoApplicationInitializationContext.cs +++ /dev/null @@ -1,28 +0,0 @@ -using System; - -namespace Sqlbi.Bravo.Host; - -/// -/// Carries what produced, as typed entries. -/// -/// -/// Not a container and not a facade: it resolves nothing, forwards nothing, and never leaves the -/// composition root. publishes each entry to the service -/// collection as an instance; ownership of the disposable entries stays here. -/// -internal sealed class BravoApplicationInitializationContext( - IBravoApplicationInstance instance) : IDisposable -{ - /// - /// Gets the single-instance binding; decides - /// whether this process runs the application or redirects its activation. - /// - public IBravoApplicationInstance Instance { get; } = instance; - - /// - /// Disposes the entries the initialization created and owns, in reverse creation order. Entries - /// that are still process-wide statics (telemetry, user settings, web proxy) own themselves and - /// are not disposed here until the initialization creates them. - /// - public void Dispose() => Instance.Dispose(); -} diff --git a/src/Host/BravoApplicationInstance.Log.cs b/src/Host/BravoApplicationInstance.Log.cs new file mode 100644 index 00000000..be9eafbe --- /dev/null +++ b/src/Host/BravoApplicationInstance.Log.cs @@ -0,0 +1,17 @@ +using System; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Infrastructure.SingleInstance; + +namespace Sqlbi.Bravo.Host; + +internal sealed partial class BravoApplicationInstance +{ + [LoggerMessage(EventId = 1, Level = LogLevel.Warning, Message = "The single-instance listener reported a failure and keeps listening.")] + private partial void LogListenerFailed(Exception exception); + + [LoggerMessage(EventId = 2, Level = LogLevel.Error, Message = "The activation could not be redirected to the primary instance ({Status}).")] + private partial void LogActivationRedirectFailed(Exception exception, SingleInstanceSendStatus status); + + [LoggerMessage(EventId = 3, Level = LogLevel.Warning, Message = "The activation payload could not be read; the window is activated without a document.")] + private partial void LogActivationPayloadUnreadable(Exception exception); +} diff --git a/src/Host/BravoApplicationInstance.cs b/src/Host/BravoApplicationInstance.cs index 2d882f25..57974382 100644 --- a/src/Host/BravoApplicationInstance.cs +++ b/src/Host/BravoApplicationInstance.cs @@ -1,9 +1,8 @@ using System; -using System.Diagnostics; using System.IO; using System.Text.Json; +using Microsoft.Extensions.Logging; using Sqlbi.Bravo.Infrastructure.Configuration.Settings; -using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Messages; using Sqlbi.Bravo.Infrastructure.SingleInstance; using Sqlbi.Bravo.Infrastructure.Telemetry; @@ -26,32 +25,25 @@ internal interface IBravoApplicationInstance : IInstanceEvents, IDisposable void RedirectActivationToPrimary(); } -internal sealed class BravoApplicationInstance : IBravoApplicationInstance +internal sealed partial class BravoApplicationInstance : IBravoApplicationInstance { private readonly SingleInstanceOptions _options; private readonly SingleInstanceServer? _server; + private readonly ITelemetryService _telemetry; + private readonly ILogger _logger; private bool _disposed; - /// - /// Creates a , claiming the primary role if no other process holds it. - /// - public static BravoApplicationInstance Create() - { - var options = new SingleInstanceOptions - { - PipeName = InstancePipeName.Create(), - }; - - _ = SingleInstanceServer.TryStart(options, out var server); - - return new BravoApplicationInstance(options, server); - } - - private BravoApplicationInstance(SingleInstanceOptions options, SingleInstanceServer? server) + public BravoApplicationInstance( + SingleInstanceOptions options, + SingleInstanceServer? server, + ITelemetryService telemetry, + ILoggerFactory loggerFactory) { _options = options; _server = server; + _telemetry = telemetry; + _logger = loggerFactory.CreateLogger(); if (_server is not null) { @@ -73,9 +65,11 @@ private void OnActivated(object? sender, SingleInstanceActivatedEventArgs e) { startupMessage = JsonSerializer.Deserialize(e.Payload); } - catch (JsonException) + catch (JsonException ex) { // An unreadable payload still activates the window, with no document to open. + LogActivationPayloadUnreadable(ex); + _telemetry.TrackException(ex); } // Known limitation: requests arriving while this instance is still starting up are dropped @@ -89,7 +83,13 @@ private void OnActivated(object? sender, SingleInstanceActivatedEventArgs e) ActivationRequested?.Invoke(this, new InstanceActivationRequestedEventArgs(startupMessage)); } - private void OnError(object? sender, ErrorEventArgs e) => Report(e.GetException()); + private void OnError(object? sender, ErrorEventArgs e) + { + var exception = e.GetException(); + + LogListenerFailed(exception); + _telemetry.TrackException(exception); + } /// public void RedirectActivationToPrimary() @@ -107,16 +107,11 @@ public void RedirectActivationToPrimary() // BUILTIN\Administrators, while the pipe name is keyed on WindowsIdentity.User // (https://github.com/dotnet/runtime/issues/123903, open, milestone .NET 12). // TODO: Consider surfacing activation failures to the user. - Report(result.Exception); + LogActivationRedirectFailed(result.Exception, result.Status); + _telemetry.TrackException(result.Exception); } } - private static void Report(Exception exception) - { - ExceptionHelper.WriteToEventLog(exception, EventLogEntryType.Warning); - TelemetryService.Instance.TrackException(exception); - } - /// public void Dispose() { diff --git a/src/Host/BravoGlobalExceptionHandling.cs b/src/Host/BravoGlobalExceptionHandling.cs new file mode 100644 index 00000000..e776cdb8 --- /dev/null +++ b/src/Host/BravoGlobalExceptionHandling.cs @@ -0,0 +1,20 @@ +using Sqlbi.Bravo.Infrastructure.Diagnostics; + +namespace Sqlbi.Bravo.Host; + +internal static class BravoGlobalExceptionHandling +{ + public static void Configure(BootstrapContext context) + { + var dialog = new ErrorReportTaskDialog(context.Telemetry); + + var handler = new UnhandledExceptionHandler( + dialog, context.Telemetry, context.LoggerFactory); + + GlobalExceptionHandling.Configure(handler.Handle); + +#if DEBUG + CrashTest.RunIfRequested(); +#endif + } +} diff --git a/src/Host/BravoServiceCollectionExtensions.cs b/src/Host/BravoServiceCollectionExtensions.cs index 14913186..a5b9d4be 100644 --- a/src/Host/BravoServiceCollectionExtensions.cs +++ b/src/Host/BravoServiceCollectionExtensions.cs @@ -1,9 +1,11 @@ using Dax.Formatter; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; using Sqlbi.Bravo.Infrastructure.Extensions; -using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.PowerBI; using Sqlbi.Bravo.Infrastructure.Services.PowerBI; +using Sqlbi.Bravo.Infrastructure.SingleInstance; using Sqlbi.Bravo.Infrastructure.Telemetry; using Sqlbi.Bravo.Services; @@ -15,24 +17,7 @@ namespace Sqlbi.Bravo.Host; internal static class BravoServiceCollectionExtensions { /// - /// Publishes what produced. These are instance - /// registrations: the container resolves them but never disposes them — their owner is the - /// initialization context. - /// - public static IServiceCollection AddBravoInitializationServices(this IServiceCollection services, BravoApplicationInitializationContext context) - { - // The single instance goes in as its activation events only: IsPrimary and the activation - // redirection belong to the entry point and stay out of the container. - services.AddSingleton(context.Instance); - - services.AddSingleton(new PolicyServiceFactory().Create()); - services.AddSingleton(TelemetryService.Instance); - - return services; - } - - /// - /// Registers the REST API surface. + /// Registers and configures the services required for the Bravo REST API. /// public static IServiceCollection AddBravoRestApi(this IServiceCollection services) { @@ -48,10 +33,12 @@ public static IServiceCollection AddBravoRestApi(this IServiceCollection service } /// - /// Registers the core services that implement the application's functionality. + /// Registers and configures the services required for the Bravo application. /// - public static IServiceCollection AddBravoServices(this IServiceCollection services) + public static IServiceCollection AddBravoServices(this IServiceCollection services, BootstrapContext bootstrap) { + services.AddBootstrapServices(bootstrap); + services.AddBravoApplicationInstance(); services.AddHttpClient(); services.AddPowerBI(); services.AddSingleton(); @@ -67,4 +54,36 @@ public static IServiceCollection AddBravoServices(this IServiceCollection servic return services; } + + private static IServiceCollection AddBravoApplicationInstance(this IServiceCollection services) + { + services.AddSingleton((provider) => + { + var telemetry = provider.GetRequiredService(); + var loggerFactory = provider.GetRequiredService(); + var options = new SingleInstanceOptions + { + PipeName = InstancePipeName.Create(), + }; + _ = SingleInstanceServer.TryStart(options, out var server); + + return new BravoApplicationInstance(options, server, telemetry, loggerFactory); + }); + + return services; + } + + private static IServiceCollection AddBootstrapServices(this IServiceCollection services, BootstrapContext bootstrap) + { + // Instance registrations: the container resolves them without creating or disposing them. + + // Replace the default ILoggerFactory registered by the WebApplicationBuilder + services.Replace(ServiceDescriptor.Singleton(bootstrap.LoggerFactory)); + + services.AddSingleton(bootstrap.PolicyService); + services.AddSingleton(bootstrap.Settings); + services.AddSingleton(bootstrap.Telemetry); + + return services; + } } diff --git a/src/Host/CrashTest.cs b/src/Host/CrashTest.cs new file mode 100644 index 00000000..5d1b6cf8 --- /dev/null +++ b/src/Host/CrashTest.cs @@ -0,0 +1,77 @@ +#if DEBUG +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Sqlbi.Bravo.Host; + +/// +/// Provides a mechanism to test the application's global exception handling +/// by throwing exceptions in different contexts. +/// +/// +/// Set the environment variable BRAVO_CrashTest before starting a Debug build: startup +/// throws before the message loop, ui on the UI thread, thread on a background thread, and +/// unobserved in a faulted task that is never observed. The last three throw after 3 seconds. +/// +internal static class CrashTest +{ + private static System.Windows.Forms.Timer? s_timer; + + public static void RunIfRequested() + { + switch (Environment.GetEnvironmentVariable("BRAVO_CrashTest")) + { + case "startup": + throw new InvalidOperationException("Crash test: before the message loop."); + + case "ui": + // Ticks on the thread running the message loop, so the exception escapes a window procedure. + s_timer = new System.Windows.Forms.Timer { Interval = 3_000 }; + s_timer.Tick += (_, _) => throw new InvalidOperationException("Crash test: UI thread."); + s_timer.Start(); + break; + + case "thread": + StartDelayed(() => throw new InvalidOperationException("Crash test: background thread.")); + break; + + case "unobserved": + StartDelayed(() => + { + ThrowUnobserved(); + + // The event is raised when the faulted task is finalized, not when it faults. + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + }); + break; + } + } + + private static void StartDelayed(Action action) + { + var thread = new Thread(() => + { + Thread.Sleep(3_000); + action(); + }) + { + IsBackground = true, + }; + + thread.Start(); + } + + // In its own method: a local in the caller would keep the task alive. + private static void ThrowUnobserved() + { + var task = Task.Run(() => throw new InvalidOperationException("Crash test: unobserved task.")); + + // Wait for the task to complete so that it can be finalized + while (!task.IsCompleted) + Thread.Sleep(10); + } +} +#endif diff --git a/src/Infrastructure/AppWindow.cs b/src/Infrastructure/AppWindow.cs index 93d105b4..6a9f4a06 100644 --- a/src/Infrastructure/AppWindow.cs +++ b/src/Infrastructure/AppWindow.cs @@ -41,7 +41,7 @@ internal partial class AppWindow : Form };"; public static SynchronizationContext? UISynchronizationContext { get; set; } - private readonly IInstanceEvents _instanceEvents; + private readonly IInstanceEvents _applicationInstance; private readonly IServerAddressProvider _serverAddressProvider; private readonly IOptions _startupSettingsOptionsAccessor; private readonly WebView2ProxyAuthHandler _proxyAuthHandler; @@ -49,12 +49,12 @@ internal partial class AppWindow : Form private readonly IPolicyService _policyService; public AppWindow( - IInstanceEvents instanceEvents, + IBravoApplicationInstance applicationInstance, IServerAddressProvider serverAddressProvider, IOptions startupSettingsOptionsAccessor, IPolicyService policyService) { - _instanceEvents = instanceEvents; + _applicationInstance = applicationInstance; _serverAddressProvider = serverAddressProvider; _startupSettingsOptionsAccessor = startupSettingsOptionsAccessor; _policyService = policyService; @@ -183,19 +183,19 @@ private void OnFormLoad(object? sender, EventArgs e) CenterToScreen(); - _instanceEvents.ActivationRequested += OnActivationRequestedRestoreWindowToForeground; + _applicationInstance.ActivationRequested += OnActivationRequestedRestoreWindowToForeground; if (AppEnvironment.IsDiagnosticLevelVerbose) { - var content = EnvironmentInfo.Collect().ToDictionary(); - AppEnvironment.AddDiagnostics(DiagnosticMessageType.Json, name: $"{nameof(AppWindow)}.{nameof(EnvironmentInfo)}", content: JsonSerializer.Serialize(content)); + var content = EnvironmentDiagnosticsCollector.Collect(); + AppEnvironment.AddDiagnostics(DiagnosticMessageType.Json, name: $"{nameof(AppWindow)}.{nameof(EnvironmentDiagnostics)}", content: JsonSerializer.Serialize(content)); } } private void OnFormClosed(object? sender, FormClosedEventArgs e) { - _instanceEvents.ActivationRequested -= OnActivationRequestedRestoreWindowToForeground; - _instanceEvents.ActivationRequested -= OnActivationRequestedSendStartupWebMessage; + _applicationInstance.ActivationRequested -= OnActivationRequestedRestoreWindowToForeground; + _applicationInstance.ActivationRequested -= OnActivationRequestedSendStartupWebMessage; } private void OnWebViewDOMContentLoaded(object? sender, CoreWebView2DOMContentLoadedEventArgs e) @@ -208,7 +208,7 @@ private void OnWebViewDOMContentLoaded(object? sender, CoreWebView2DOMContentLoa BackgroundImage = null; SendAppStartupWebMessage(); - _instanceEvents.ActivationRequested += OnActivationRequestedSendStartupWebMessage; + _applicationInstance.ActivationRequested += OnActivationRequestedSendStartupWebMessage; } } diff --git a/src/Infrastructure/Configuration/UserSettingsServiceFactory.cs b/src/Infrastructure/Configuration/UserSettingsServiceFactory.cs new file mode 100644 index 00000000..2adc7188 --- /dev/null +++ b/src/Infrastructure/Configuration/UserSettingsServiceFactory.cs @@ -0,0 +1,13 @@ +using Sqlbi.Bravo.Infrastructure.Configuration.Settings; + +namespace Sqlbi.Bravo.Infrastructure.Configuration; + +internal interface IUserSettingsServiceFactory +{ + IUserSettings Create(); +} + +internal sealed class UserSettingsServiceFactory : IUserSettingsServiceFactory +{ + public IUserSettings Create() => UserPreferences.Current; +} diff --git a/src/Infrastructure/Diagnostics/EnvironmentDiagnostics.cs b/src/Infrastructure/Diagnostics/EnvironmentDiagnostics.cs new file mode 100644 index 00000000..508c0a73 --- /dev/null +++ b/src/Infrastructure/Diagnostics/EnvironmentDiagnostics.cs @@ -0,0 +1,103 @@ +using System; +using System.Runtime.InteropServices; +using System.Text.Json; +using System.Text.Json.Serialization; +using Sqlbi.Bravo.Infrastructure.Telemetry; + +namespace Sqlbi.Bravo.Infrastructure.Diagnostics; + +internal sealed record EnvironmentDiagnostics( + [property: JsonPropertyName("timestampUtc")] string TimestampUtc, + [property: JsonPropertyName("timestampLocal")] string TimestampLocal, + [property: JsonPropertyName("application")] ApplicationDiagnostics Application, + [property: JsonPropertyName("process")] ProcessDiagnostics Process, + [property: JsonPropertyName("os")] OperatingSystemDiagnostics OS, + [property: JsonPropertyName("runtime")] RuntimeDiagnostics Runtime, + [property: JsonPropertyName("components")] ComponentDiagnostics Components) +{ + private static readonly JsonSerializerOptions? s_options = new() + { + WriteIndented = true + }; + + public string ToJsonString() + => JsonSerializer.Serialize(this, s_options); +} + +internal sealed record ApplicationDiagnostics( + [property: JsonPropertyName("sessionId")] string SessionId, + [property: JsonPropertyName("version")] string Version, + [property: JsonPropertyName("publishMode")] string PublishMode, + [property: JsonPropertyName("deploymentMode")] string DeploymentMode, + [property: JsonPropertyName("dataPath")] string DataPath +); + +internal sealed record ProcessDiagnostics( + [property: JsonPropertyName("id")] string Id, + [property: JsonPropertyName("path")] string Path, + [property: JsonPropertyName("architecture")] string Architecture, + [property: JsonPropertyName("processorCount")] string ProcessorCount +); + +public sealed record OperatingSystemDiagnostics( + [property: JsonPropertyName("version")] string Version, + [property: JsonPropertyName("description")] string Description, + [property: JsonPropertyName("architecture")] string Architecture +); + +internal sealed record RuntimeDiagnostics( + [property: JsonPropertyName("identifier")] string Identifier, + [property: JsonPropertyName("description")] string Description +); + +internal sealed record ComponentDiagnostics( + [property: JsonPropertyName("webView2Version")] string WebView2Version +); + +internal static class EnvironmentDiagnosticsCollector +{ + public static EnvironmentDiagnostics Collect() + { + return new EnvironmentDiagnostics( + TimestampUtc: DateTimeOffset.UtcNow.ToString("O"), + TimestampLocal: DateTime.Now.ToString("O"), + Application: new( + SessionId: SafeRead(() => TelemetrySessionInfo.SessionId), + Version: SafeRead(() => AppVersion.InformationalVersion), + PublishMode: SafeRead(() => AppEnvironment.PublishMode.ToString()), + DeploymentMode: SafeRead(() => AppEnvironment.DeploymentMode.ToString()), + DataPath: SafeRead(() => AppEnvironment.ApplicationDataPath) + ), + Process: new( + Id: SafeRead(() => Environment.ProcessId.ToString()), + Path: SafeRead(() => Environment.ProcessPath), + Architecture: SafeRead(() => RuntimeInformation.ProcessArchitecture.ToString()), + ProcessorCount: SafeRead(() => Environment.ProcessorCount.ToString()) + ), + OS: new( + Version: SafeRead(() => Environment.OSVersion.ToString()), + Description: SafeRead(() => RuntimeInformation.OSDescription), + Architecture: SafeRead(() => RuntimeInformation.OSArchitecture.ToString()) + ), + Runtime: new( + Identifier: SafeRead(() => RuntimeInformation.RuntimeIdentifier), + Description: SafeRead(() => RuntimeInformation.FrameworkDescription) + ), + Components: new( + WebView2Version: SafeRead(() => AppEnvironment.WebView2VersionInfo) + ) + ); + } + + private static string SafeRead(Func read, string fallback = "n/a") + { + try + { + return read() ?? fallback; + } + catch (Exception ex) + { + return $"{fallback} ({ex.GetType().Name})"; + } + } +} diff --git a/src/Infrastructure/Diagnostics/EnvironmentInfo.cs b/src/Infrastructure/Diagnostics/EnvironmentInfo.cs deleted file mode 100644 index bba4e43a..00000000 --- a/src/Infrastructure/Diagnostics/EnvironmentInfo.cs +++ /dev/null @@ -1,93 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using System.Linq; -using System.Runtime.InteropServices; -using System.Text; -using System.Text.Json; -using Sqlbi.Bravo.Infrastructure.Telemetry; - -namespace Sqlbi.Bravo.Infrastructure.Diagnostics; - -/// -/// Provides diagnostic information about the environment in which the application is running. -/// -internal sealed class EnvironmentInfo -{ - private readonly IReadOnlyList> _entries; - - /// - /// Collects the environment information and returns an instance of . - /// - public static EnvironmentInfo Collect() - { - var entries = new KeyValuePair[] - { - new("TimestampUtc", DateTime.UtcNow.ToString("O", CultureInfo.InvariantCulture)), - new("TimestampLocal", DateTime.Now.ToString("O", CultureInfo.InvariantCulture)), - // Application - new("ApplicationSessionId", SafeRead(() => TelemetrySessionInfo.SessionId)), - new("ApplicationVersion", SafeRead(() => AppVersion.InformationalVersion)), - new("ApplicationPublishMode", SafeRead(() => AppEnvironment.PublishMode.ToString())), - new("ApplicationDeploymentMode", SafeRead(() => AppEnvironment.DeploymentMode.ToString())), - new("ApplicationDataPath", SafeRead(() => AppEnvironment.ApplicationDataPath)), - // Process - new("ProcessId", SafeRead(() => Environment.ProcessId.ToString(CultureInfo.InvariantCulture))), - new("ProcessPath", SafeRead(() => Environment.ProcessPath)), - new("ProcessSessionId", SafeRead(() => AppEnvironment.SessionId.ToString(CultureInfo.InvariantCulture))), - new("ProcessArchitecture", SafeRead(() => RuntimeInformation.ProcessArchitecture.ToString())), - new("ProcessProcessorCount", SafeRead(() => Environment.ProcessorCount.ToString(CultureInfo.InvariantCulture))), - // Machine - new("OSDescription", SafeRead(() => RuntimeInformation.OSDescription)), - new("OSArchitecture", SafeRead(() => RuntimeInformation.OSArchitecture.ToString())), - // Runtime - new("RuntimeDescription", SafeRead(() => RuntimeInformation.FrameworkDescription)), - new("RuntimeIdentifier", SafeRead(() => RuntimeInformation.RuntimeIdentifier)), - // Components - new("WebView2Version", SafeRead(() => AppEnvironment.WebView2VersionInfo)), - }; - - return new EnvironmentInfo(entries); - } - - private EnvironmentInfo(IReadOnlyList> entries) - { - _entries = entries; - } - - /// - /// Returns the environment information as a text block. - /// - public string ToText() - { - var builder = new StringBuilder(); - - builder.AppendLine("# Environment Information"); - builder.AppendLine(); - - foreach (var (name, value) in _entries) - builder.Append($"- {name}: ").AppendLine(value); - - return builder.ToString(); - } - - /// - /// Returns the environment information as dictionary. - /// - public Dictionary ToDictionary() - { - return _entries.ToDictionary((entry) => entry.Key, (entry) => entry.Value); - } - - internal static string SafeRead(Func read, string fallback = "n/a") - { - try - { - return read() ?? fallback; - } - catch (Exception ex) - { - return $"unavailable ({ex.GetType().Name})"; - } - } -} diff --git a/src/Infrastructure/Diagnostics/ErrorReport.cs b/src/Infrastructure/Diagnostics/ErrorReport.cs index 9980086d..a718ea6f 100644 --- a/src/Infrastructure/Diagnostics/ErrorReport.cs +++ b/src/Infrastructure/Diagnostics/ErrorReport.cs @@ -1,8 +1,6 @@ using System; using System.IO; -using System.Runtime.InteropServices; using System.Text; -using System.Windows.Forms; namespace Sqlbi.Bravo.Infrastructure.Diagnostics; @@ -11,47 +9,33 @@ namespace Sqlbi.Bravo.Infrastructure.Diagnostics; /// internal sealed class ErrorReport { - private const string FileName = "ErrorReport.txt"; - - private readonly EnvironmentInfo _environment; + private readonly EnvironmentDiagnostics _diagnostics; private readonly Lazy _text; - /// - /// Creates an instance for the given exception. - /// public static ErrorReport Create(Exception exception) { - var environment = EnvironmentInfo.Collect(); - return new ErrorReport(exception, environment); + var diagnostics = EnvironmentDiagnosticsCollector.Collect(); + return new ErrorReport(exception, diagnostics); } - internal ErrorReport(Exception exception, EnvironmentInfo environment) + private ErrorReport(Exception exception, EnvironmentDiagnostics diagnostics) { Exception = exception; - _environment = environment; + _diagnostics = diagnostics; _text = new Lazy(GenerateText); } public Exception Exception { get; } - - /// - /// The text representation of the error report. - /// public string Text => _text.Value; - - /// - /// The file path where the error report was saved, or null if saving failed. - /// public string? FilePath { get; private set; } - /// - /// Attempts to save the error report to a file in the application data folder. - /// public bool TrySave() { try { - var filePath = Path.Combine(AppEnvironment.ApplicationDataPath, FileName); + var directory = Directory.CreateDirectory(AppEnvironment.ApplicationDataPath); + var filePath = Path.Combine(directory.FullName, "ErrorReport.txt"); + File.WriteAllText(filePath, Text, Encoding.UTF8); FilePath = filePath; @@ -59,41 +43,29 @@ public bool TrySave() } catch (Exception) { + FilePath = null; return false; } } - /// - /// Attempts to copy the report to the clipboard. - /// - public bool TryCopyToClipboard() - { - try - { - Clipboard.SetDataObject( - data: new DataObject(DataFormats.UnicodeText, Text), - copy: true, - retryTimes: 10, - retryDelay: 100); - - return true; - } - catch (ExternalException) - { - return false; - } - } - - public override string ToString() => Text; - private string GenerateText() { var builder = new StringBuilder(); - builder.AppendLine(_environment.ToText()); + builder.AppendLine("# Bravo for Power BI — Error Report"); - builder.AppendLine("# Error Details"); builder.AppendLine(); + builder.AppendLine("## Environment information"); + builder.AppendLine(); + + builder.AppendLine("```json"); + builder.AppendLine(_diagnostics.ToJsonString()); + builder.AppendLine("```"); + + builder.AppendLine(); + builder.AppendLine("## Exception details"); + builder.AppendLine(); + builder.AppendLine("```"); builder.AppendLine(Exception.ToString()); builder.AppendLine("```"); diff --git a/src/Infrastructure/Diagnostics/ErrorReportDialog.cs b/src/Infrastructure/Diagnostics/ErrorReportDialog.cs deleted file mode 100644 index 55901869..00000000 --- a/src/Infrastructure/Diagnostics/ErrorReportDialog.cs +++ /dev/null @@ -1,85 +0,0 @@ -using System; -using System.Text; -using System.Windows.Forms; -using Sqlbi.Bravo.Infrastructure.Helpers; -using Sqlbi.Bravo.Infrastructure.Windows.Dialogs; - -namespace Sqlbi.Bravo.Infrastructure.Diagnostics; - -/// -/// Provides a dialog to report an error that occurred in the application. -/// -internal static class ErrorReportDialog -{ - //private const string BugReportUrl = "https://github.com/sql-bi/Bravo/issues/new"; - - public static void Show(ErrorReport report) - { - //var reportButton = new TaskDialogCommandLinkButton("&Report this issue", "Copies the report to the clipboard and opens the Bravo issue tracker in your browser, where you can paste it."); - var copyButton = new TaskDialogCommandLinkButton("&Copy to clipboard", "Copies the crash report to the clipboard.") - { - // Keep the dialog open after clicking the button - AllowCloseDialog = false, - }; - - copyButton.Click += (_, _) => _ = report.TryCopyToClipboard(); - - var clickedButton = TaskDialogBuilder.Create() - .WithCaption(AppEnvironment.ApplicationMainWindowTitle) - .WithIcon(TaskDialogIcon.Error) - .WithStartupLocation(TaskDialogStartupLocation.CenterScreen) - .WithAllowCancel() - .WithSizeToContent() - .WithEnableLinks((href) => _ = ProcessHelper.Open(href)) - .WithHeading("Bravo encountered an unexpected error.") - .WithText(GetText(report)) - .AddButtons(/*reportButton,*/ copyButton, TaskDialogButton.Close) - .WithDefaultButton(copyButton) - .Show(); - - //if (clickedButton == reportButton) - //{ - // _ = report.TryCopyToClipboard(); - // _ = ProcessHelper.OpenBrowser(new Uri(BugReportUrl, UriKind.Absolute)); - //} - } - - private static string GetText(ErrorReport report) - { - var text = new StringBuilder(); - - text.AppendLine(report.FilePath is not null - ? "A crash report has been saved and can be used to diagnose the problem." - : "The crash report could not be saved."); - - if (report.FilePath is not null) - { - text.AppendLine(); - text.AppendLine("Report file:"); - text.Append("").Append(report.FilePath).AppendLine(""); - } - - text.AppendLine(); - text.AppendLine("Exception:"); - text.AppendLine($"{report.Exception.GetType().FullName}: {report.Exception.Message}"); - - text.AppendLine(); - text.AppendLine("Location:"); - text.AppendLine(GetExceptionLocation(report.Exception)); - - return text.ToString(); - - static string GetExceptionLocation(Exception exception) - { - if (exception.TargetSite is { } method) - { - if (method.DeclaringType is { } type) - { - return $"{type.FullName}.{method.Name}"; - } - } - - return exception.Source ?? ""; - } - } -} diff --git a/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs new file mode 100644 index 00000000..fd2ef75b --- /dev/null +++ b/src/Infrastructure/Diagnostics/ErrorReportTaskDialog.cs @@ -0,0 +1,199 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using System.Windows.Forms; +using Sqlbi.Bravo.Infrastructure.Helpers; +using Sqlbi.Bravo.Infrastructure.Telemetry; +using Sqlbi.Bravo.Infrastructure.Windows.Dialogs; + +namespace Sqlbi.Bravo.Infrastructure.Diagnostics; + +internal interface IErrorReportDialog +{ + void Show(ErrorReport report); +} + +/// +/// Implements an error report dialog to display error information to the user. +/// +internal sealed class ErrorReportTaskDialog(ITelemetryService telemetry) : IErrorReportDialog +{ + private readonly ITelemetryService _telemetry = telemetry; + + public void Show(ErrorReport report) + { + // 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)); + } + + private void ShowImpl(ErrorReport report) + { + // 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()); + + 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, + }; + var copyButton = new TaskDialogCommandLinkButton("&Copy to clipboard", "Copies the crash report details to the clipboard.") + { + AllowCloseDialog = false, + }; + + var text = GetText(report); + var builder = TaskDialogBuilder.Create() + .WithCaption(AppEnvironment.ApplicationMainWindowTitle) + .WithIcon(TaskDialogIcon.ShieldErrorRedBar) + .WithCurrentProcessMainWindowOwner() + .WithStartupLocation(TaskDialogStartupLocation.CenterOwner) + .WithAllowCancel() + .WithSizeToContent() + .WithEnableLinks((page, linkHref) => OpenFile(page, linkHref, text)) + .WithHeading("Bravo encountered an unexpected error.") + .WithText(text) + .AddButtons(sendButton, copyButton, TaskDialogButton.Close) + .WithDefaultButton(sendButton.Visible ? sendButton : copyButton); + + var page = builder.Build(); + + copyButton.Click += (_, _) => CopyToClipboard(page, text, report); + sendButton.Click += async (_, _) => await SendAsync(page, sendButton, text, report); + + builder.Show(); + } + + private async Task SendAsync(TaskDialogPage page, TaskDialogButton sendButton, string text, ErrorReport report) + { + var previousTelemetryEnabled = _telemetry.TelemetryEnabled; + try + { + sendButton.Enabled = false; + SetStatus(page, text, "Sending the report..."); + + _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); + + SetStatus(page, text, submitted + ? "The report has been submitted. Thank you." + : "The report could not be submitted. Copy the report to keep its details."); + } + catch (Exception ex) + { + SetStatus(page, text, "The report could not be submitted. Copy the report to keep its details.", ex); + } + finally + { + _telemetry.TelemetryEnabled = previousTelemetryEnabled; + } + } + + private static void CopyToClipboard(TaskDialogPage page, string text, ErrorReport report) + { + try + { + Clipboard.SetDataObject( + data: new DataObject(DataFormats.UnicodeText, report.Text), + copy: true, + retryTimes: 10, + retryDelay: 100); + } + catch (Exception ex) + { + SetStatus(page, text, "The report could not be copied to the clipboard.", ex); + } + } + + private static void OpenFile(TaskDialogPage page, string linkHref, string text) + { + try + { + // The href must be a file path and the file must exist. + if (!File.Exists(linkHref)) + { + SetStatus(page, text, "The report file is no longer available."); + return; + } + + using var _ = Process.Start(new ProcessStartInfo + { + FileName = linkHref, + UseShellExecute = true, + }); + } + catch (Exception ex) + { + SetStatus(page, text, "The report file could not be opened.", ex); + } + } + + private static void SetStatus(TaskDialogPage page, string text, string status, Exception? exception = null) + { + if (page.BoundDialog is null) + return; + + if (exception is not null) + status = $"{status} ({exception.GetType().Name}: {exception.Message})"; + + page.Text = $""" + {text} + ---- + + {status} + """; + } + + private static string GetText(ErrorReport report) + { + var text = new StringBuilder(); + + text.AppendLine(report.FilePath is not null + ? "A crash report has been saved and can be used to diagnose the problem." + : "The crash report could not be saved locally."); + + text.AppendLine(); + text.AppendLine("SessionId:"); + text.AppendLine(TelemetrySessionInfo.SessionId); + + if (report.FilePath is not null) + { + text.AppendLine(); + text.AppendLine("Report file:"); + text.Append("").Append(report.FilePath).AppendLine(""); + } + + text.AppendLine(); + text.AppendLine("Exception:"); + text.AppendLine($"{report.Exception.GetType().Name}: {report.Exception.Message}"); + + //text.AppendLine(); + //text.AppendLine("Location:"); + //text.AppendLine(GetExceptionLocation(report.Exception)); + + return text.ToString(); + + //static string GetExceptionLocation(Exception exception) + //{ + // if (exception.TargetSite is { } method && method.DeclaringType is { } type) + // { + // return $"{type.FullName}.{method.Name}"; + // } + + // return exception.Source ?? ""; + //} + } +} diff --git a/src/Infrastructure/Diagnostics/GlobalExceptionHandling.cs b/src/Infrastructure/Diagnostics/GlobalExceptionHandling.cs new file mode 100644 index 00000000..48cd11c5 --- /dev/null +++ b/src/Infrastructure/Diagnostics/GlobalExceptionHandling.cs @@ -0,0 +1,96 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using System.Windows.Forms; + +namespace Sqlbi.Bravo.Infrastructure.Diagnostics; + +/// +/// Provides global exception handling for the application. +/// +internal static class GlobalExceptionHandling +{ + private static GlobalExceptionHandler? s_handler; + private static int s_isHandlingFatalException; + + /// + /// Configures global exception handling for the application. + /// + public static void Configure(GlobalExceptionHandler handler) + { + if (s_handler is not null) + throw new InvalidOperationException("Global exception handling has already been configured."); + + s_handler = handler; + + // mode: ThrowException: Prevents WinForms from swallowing UI exceptions or showing its "Continue/Quit" dialog. + // - Preserves the exact stack trace at the throw site for crash dumps and debugger break. + // - Routes UI exceptions to AppDomain.UnhandledException (consistent with background threads). + // threadScope: false: Sets this mode globally across all threads, ensuring additional STA threads are covered. + Application.SetUnhandledExceptionMode(UnhandledExceptionMode.ThrowException, threadScope: false); + + AppDomain.CurrentDomain.UnhandledException += OnUnhandledException; + TaskScheduler.UnobservedTaskException += OnUnobservedTaskException; + } + + private static void OnUnhandledException(object? sender, UnhandledExceptionEventArgs e) + { + // ExceptionObject may contain a non-CLS exception value. + var exception = e.ExceptionObject as Exception + ?? new InvalidOperationException($"Non-CLS-compliant exception object of type '{e.ExceptionObject?.GetType()}': {e.ExceptionObject}."); + + // Prevent fatal exception handling from being reentered, which could lead to stack overflow or deadlock. + if (Interlocked.CompareExchange(ref s_isHandlingFatalException, value: 1, comparand: 0) != 0) + return; + + try + { + InvokeHandler(exception, ExceptionOrigin.AppDomain); + } + finally + { + Interlocked.Exchange(ref s_isHandlingFatalException, 0); + } + } + + private static void OnUnobservedTaskException(object? sender, UnobservedTaskExceptionEventArgs e) + { + // Prevent the task exception from being raised again if handling fails. + e.SetObserved(); + + InvokeHandler(e.Exception, ExceptionOrigin.UnobservedTask); + } + + private static void InvokeHandler(Exception exception, ExceptionOrigin origin) + { + try + { + s_handler?.Invoke(exception, origin); + } + catch (Exception handlerException) + { + Environment.FailFast($"The unhandled exception handler failed while handling: {exception}", handlerException); + } + } +} + +/// +/// Handles an exception captured by . +/// +internal delegate void GlobalExceptionHandler(Exception exception, ExceptionOrigin origin); + +/// +/// Identifies where an exception was captured and the constraints of handling it. +/// +internal enum ExceptionOrigin +{ + /// + /// An unhandled exception from any thread. The runtime terminates the process after the handler returns. + /// + AppDomain, + + /// + /// An unobserved task exception raised on the finalizer thread. Do not block or show UI. + /// + UnobservedTask, +} diff --git a/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs new file mode 100644 index 00000000..3ef2753a --- /dev/null +++ b/src/Infrastructure/Diagnostics/UnhandledExceptionHandler.cs @@ -0,0 +1,75 @@ +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Infrastructure.Telemetry; + +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) +{ + private readonly IErrorReportDialog _dialog = dialog; + private readonly ITelemetryService _telemetry = telemetry; + private readonly ILogger _logger = loggerFactory.CreateLogger(); + + public void Handle(Exception exception, ExceptionOrigin origin) + { + Report(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. + if (origin == ExceptionOrigin.UnobservedTask) + return; + + // Prevent multiple handling of the same exception + if (!TryMarkHandled(exception)) + return; + + var report = ErrorReport.Create(exception); + _ = report.TrySave(); + _dialog.Show(report); + + _ = _telemetry.TryFlushBeforeShutdown(10_000); + } + + private void Report(Exception exception, ExceptionOrigin origin) + { + try + { + _logger.LogError(exception, "An unhandled exception occurred. Origin: {Origin}", origin); + } + catch { /* Ignore failures */ } + + try + { + _telemetry.TrackException(exception, properties: new Dictionary + { + ["ExceptionOrigin"] = origin.ToString(), + ["ExceptionKind"] = "Unhandled" + }); + } + catch { /* Ignore failures */ } + } + + private static bool TryMarkHandled(Exception exception) + { + const string HandledMarker = "Bravo.UnhandledExceptionHandler.Handled"; + + try + { + if (exception.Data.Contains(HandledMarker)) + return false; + + exception.Data[HandledMarker] = true; + return true; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException) + { + // Data is read-only or rejects the key: report the exception anyway + return true; + } + } +} diff --git a/src/Infrastructure/Helpers/ProcessHelper.cs b/src/Infrastructure/Helpers/ProcessHelper.cs index 7a171728..a3a009dc 100644 --- a/src/Infrastructure/Helpers/ProcessHelper.cs +++ b/src/Infrastructure/Helpers/ProcessHelper.cs @@ -179,13 +179,12 @@ public static bool OpenShellExecute(string path, bool waitForStarted, [NotNullWh { if (File.Exists(path)) { - const string Txt = ".txt"; const string Pbix = ".pbix"; const string Xlsx = ".xlsx"; const string CodeWorkspace = ".code-workspace"; var extension = Path.GetExtension(path); - var isAllowed = (new[] { Txt, Pbix, Xlsx, CodeWorkspace }).Any((ext) => ext.EqualsI(extension)); + var isAllowed = (new[] { Pbix, Xlsx, CodeWorkspace }).Any((ext) => ext.EqualsI(extension)); var isPbix = extension.EqualsI(Pbix); if (isAllowed) diff --git a/src/Infrastructure/Telemetry/ITelemetryServiceFactory.cs b/src/Infrastructure/Telemetry/ITelemetryServiceFactory.cs new file mode 100644 index 00000000..0641d2e2 --- /dev/null +++ b/src/Infrastructure/Telemetry/ITelemetryServiceFactory.cs @@ -0,0 +1,12 @@ +namespace Sqlbi.Bravo.Infrastructure.Telemetry; + +internal interface ITelemetryServiceFactory +{ + ITelemetryService Create(bool enabled); +} + +internal sealed class TelemetryServiceFactory : ITelemetryServiceFactory +{ + public ITelemetryService Create(bool enabled) + => TelemetryService.Create(TelemetrySessionInfo.ConnectionString, enabled, TelemetryService.DefaultStorageFolder); +} diff --git a/src/Infrastructure/Telemetry/NullTelemetryService.cs b/src/Infrastructure/Telemetry/NullTelemetryService.cs new file mode 100644 index 00000000..4ad4f578 --- /dev/null +++ b/src/Infrastructure/Telemetry/NullTelemetryService.cs @@ -0,0 +1,18 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +namespace Sqlbi.Bravo.Infrastructure.Telemetry; + +/// +/// An implementation of that does not perform any telemetry operations. +/// +internal sealed class NullTelemetryService : ITelemetryService +{ + public bool TelemetryEnabled { get => false; set { } } + public void TrackException(Exception exception, IDictionary? properties = null) { } + public Task FlushAsync(CancellationToken cancellationToken = default) => Task.FromResult(false); + public bool TryFlushBeforeShutdown(int timeoutMilliseconds) => false; + public void Dispose() { } +} diff --git a/src/Infrastructure/Telemetry/TelemetryService.cs b/src/Infrastructure/Telemetry/TelemetryService.cs index a5c39e91..af2fad8d 100644 --- a/src/Infrastructure/Telemetry/TelemetryService.cs +++ b/src/Infrastructure/Telemetry/TelemetryService.cs @@ -8,8 +8,6 @@ using Microsoft.ApplicationInsights.Channel; using Microsoft.ApplicationInsights.Extensibility; using Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel; -using Sqlbi.Bravo.Infrastructure.Configuration; -using Sqlbi.Bravo.Infrastructure.Policies; namespace Sqlbi.Bravo.Infrastructure.Telemetry; @@ -26,8 +24,6 @@ internal sealed class TelemetryService : ITelemetryService public static readonly string DefaultStorageFolder = Path.Combine(AppEnvironment.ApplicationCachePath, ".telemetry"); - private static readonly Lazy s_instance = new(CreateInstance, isThreadSafe: true); - private readonly TelemetryConfiguration _configuration; private readonly ITelemetryChannel _channel; private readonly TelemetryClient _client; @@ -40,21 +36,6 @@ private TelemetryService(TelemetryConfiguration configuration) _client = new TelemetryClient(configuration); } - /// - /// Gets the process-wide instance, created on first use with the consent resolved from policies - /// and user settings. - /// - public static TelemetryService Instance => s_instance.Value; - - /// - /// Disposes the process-wide instance, if it was created. - /// - public static void DisposeInstance() - { - if (s_instance.IsValueCreated) - s_instance.Value.Dispose(); - } - public bool TelemetryEnabled { get => _configuration.DisableTelemetry == false; @@ -144,12 +125,4 @@ public static TelemetryService Create(string connectionString, bool telemetryEna return new TelemetryService(configuration); } - - private static TelemetryService CreateInstance() - { - var policies = new PolicyServiceFactory().Create().Current; - var consent = TelemetryConsent.Resolve(policies, UserPreferences.Current); - - return Create(TelemetrySessionInfo.ConnectionString, consent.IsEnabled, DefaultStorageFolder); - } } diff --git a/src/Infrastructure/Windows/Dialogs/TaskDialogBuilder.cs b/src/Infrastructure/Windows/Dialogs/TaskDialogBuilder.cs index 7ecb6e1f..699edc0f 100644 --- a/src/Infrastructure/Windows/Dialogs/TaskDialogBuilder.cs +++ b/src/Infrastructure/Windows/Dialogs/TaskDialogBuilder.cs @@ -101,6 +101,16 @@ public TaskDialogBuilder WithEnableLinks(Action linkClicked) return this; } + /// + /// Enables clickable links in the dialog text and footnote, and invokes the specified callback, with the page the link belongs to, when a link is clicked. + /// + public TaskDialogBuilder WithEnableLinks(Action linkClicked) + { + _page.EnableLinks = true; + _page.LinkClicked += (_, e) => linkClicked(_page, e.LinkHref); + return this; + } + /// /// Sets the dialog to automatically size to its content. If false, the dialog will have a fixed size. /// @@ -168,12 +178,7 @@ public TaskDialogPage Build() /// public TaskDialogButton Show() { - if (_ownerHandle != IntPtr.Zero) - { - return TaskDialog.ShowDialog(_ownerHandle, _page, _startupLocation); - } - - return TaskDialog.ShowDialog(_page, _startupLocation); + return TaskDialog.ShowDialog(_ownerHandle, _page, _startupLocation); } private static TaskDialogIcon? LoadCurrentProcessIcon() diff --git a/src/Program.cs b/src/Program.cs index 6562c5d0..9f6eb41b 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -1,7 +1,5 @@ using System; using Sqlbi.Bravo.Host; -using Sqlbi.Bravo.Infrastructure.Diagnostics; -using Sqlbi.Bravo.Infrastructure.Telemetry; namespace Sqlbi.Bravo; @@ -11,38 +9,17 @@ internal static class Program private static void Main() { ApplicationConfiguration.Initialize(); - try - { - using var context = BravoApplicationInitializer.Initialize(); - if (!context.Instance.IsPrimary) - { - context.Instance.RedirectActivationToPrimary(); - return; - } + BravoApplicationConfiguration.Initialize(); - using var application = BravoApplication - .CreateBuilder(context) - .Build(); + using var context = BootstrapContextFactory.CreateDefault(); - application.Run(); - } - catch (Exception ex) - { - TelemetryService.Instance.TrackException(ex); + BravoGlobalExceptionHandling.Configure(context); - var report = ErrorReport.Create(ex); - report.TrySave(); - ErrorReportDialog.Show(report); + using var app = BravoApplication + .CreateBuilder(context) + .Build(); - // An unhandled rethrow can end the process before the finally block below runs. - _ = TelemetryService.Instance.TryFlushBeforeShutdown(10_000); - - throw; - } - finally - { - TelemetryService.DisposeInstance(); - } + app.Run(); } } diff --git a/test/Bravo.Tests/Host/BootstrapContextTests.cs b/test/Bravo.Tests/Host/BootstrapContextTests.cs new file mode 100644 index 00000000..7d84d6fa --- /dev/null +++ b/test/Bravo.Tests/Host/BootstrapContextTests.cs @@ -0,0 +1,177 @@ +using System; +using System.Collections.Generic; +using System.IO; +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.Policies; +using Sqlbi.Bravo.Infrastructure.Telemetry; +using Xunit; + +namespace Bravo.Tests.Host; + +public class BootstrapContextTests +{ + [Fact] + public void Dispose_DisposesTheOwnedServicesOnce() + { + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + var context = new BootstrapContext(loggerFactory, new PolicyService(new FakePolicyReader()), new UserSettings(), telemetry); + + context.Dispose(); + context.Dispose(); + + Assert.Equal(1, telemetry.DisposeCount); + Assert.Equal(1, loggerFactory.DisposeCount); + } + + [Theory] + [InlineData(null, true, true)] + [InlineData(null, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, true)] + public void Create_LoadsPoliciesOnceBeforeCreatingTelemetry(bool? policy, bool preference, bool expected) + { + var loggerFactory = new FakeLoggerFactory(); + var snapshot = PolicySnapshot.NotConfigured with { TelemetryEnabled = policy }; + var reader = new FakePolicyReader { Snapshot = snapshot }; + var policyService = new PolicyService(reader); + var settings = new UserSettings { TelemetryEnabled = preference }; + var policyFactory = new FakePolicyFactory(() => policyService); + var settingsFactory = new FakeSettingsFactory(() => settings); + var telemetryFactory = new FakeTelemetryFactory(); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + + Assert.Equal(0, reader.ReadCount); + Assert.Equal(0, policyFactory.CreateCount); + Assert.Equal(0, settingsFactory.CreateCount); + Assert.Empty(telemetryFactory.EnabledValues); + + using var context = factory.Create(); + reader.Snapshot = PolicySnapshot.NotConfigured with { TelemetryEnabled = !expected }; + + Assert.Equal([expected], telemetryFactory.EnabledValues); + Assert.Equal(expected, context.Telemetry.TelemetryEnabled); + Assert.Same(loggerFactory, context.LoggerFactory); + Assert.Same(policyService, context.PolicyService); + Assert.Same(snapshot, context.PolicyService.Current); + Assert.Same(settings, context.Settings); + Assert.Equal(1, reader.ReadCount); + Assert.Equal(1, policyFactory.CreateCount); + Assert.Equal(1, settingsFactory.CreateCount); + } + + [Fact] + public void Create_ReadFailurePropagatesBeforeTelemetryIsCreated() + { + var failure = new IOException("Registry unavailable"); + var reader = new FakePolicyReader { OnRead = () => throw failure }; + using var loggerFactory = new FakeLoggerFactory(); + 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); + + Assert.Same(failure, Assert.Throws(() => factory.Create())); + Assert.Equal(0, settingsFactory.CreateCount); + Assert.Empty(telemetryFactory.EnabledValues); + Assert.Equal(0, loggerFactory.DisposeCount); + } + + [Fact] + public void Create_PolicyCreationFailurePropagatesBeforeCreatingOtherServices() + { + var failure = new InvalidOperationException("Policy service unavailable"); + using var loggerFactory = new FakeLoggerFactory(); + var policyFactory = new FakePolicyFactory(() => throw failure); + var settingsFactory = new FakeSettingsFactory(() => new UserSettings()); + var telemetryFactory = new FakeTelemetryFactory(); + var factory = new BootstrapContextFactory(loggerFactory, policyFactory, settingsFactory, telemetryFactory); + + Assert.Same(failure, Assert.Throws(() => factory.Create())); + Assert.Equal(0, settingsFactory.CreateCount); + Assert.Empty(telemetryFactory.EnabledValues); + Assert.Equal(0, loggerFactory.DisposeCount); + } + + [Fact] + public void Create_SettingsCreationFailureUsesDefaultsCreatesTelemetryAndLogsError() + { + var failure = new IOException("Settings unavailable"); + using var loggerFactory = new FakeLoggerFactory(); + 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); + + using var context = factory.Create(); + + Assert.IsType(context.Settings); + Assert.Equal(UserSettings.DefaultTelemetryEnabled, context.Settings.TelemetryEnabled); + Assert.Equal([UserSettings.DefaultTelemetryEnabled], telemetryFactory.EnabledValues); + Assert.Equal(UserSettings.DefaultTelemetryEnabled, context.Telemetry.TelemetryEnabled); + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Error, entry.Level); + Assert.Equal(2, entry.EventId.Id); + Assert.Same(failure, entry.Exception); + } + + [Fact] + public void Create_TelemetryFailureUsesDisabledServiceAndLogsError() + { + var failure = new InvalidOperationException("Telemetry unavailable"); + var loggerFactory = new FakeLoggerFactory(); + 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); + + using var context = factory.Create(); + + Assert.IsType(context.Telemetry); + Assert.False(context.Telemetry.TelemetryEnabled); + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Error, entry.Level); + Assert.Same(failure, entry.Exception); + } + + private sealed class FakePolicyFactory(Func create) : IPolicyServiceFactory + { + public int CreateCount { get; private set; } + + public IPolicyService Create() + { + CreateCount++; + return create(); + } + } + + private sealed class FakeSettingsFactory(Func create) : IUserSettingsServiceFactory + { + public int CreateCount { get; private set; } + + public IUserSettings Create() + { + CreateCount++; + return create(); + } + } + + private sealed class FakeTelemetryFactory : ITelemetryServiceFactory + { + public List EnabledValues { get; } = []; + public Exception? Failure { get; init; } + + public ITelemetryService Create(bool enabled) + { + EnabledValues.Add(enabled); + if (Failure is not null) + throw Failure; + + return new FakeTelemetry { TelemetryEnabled = enabled }; + } + } +} diff --git a/test/Bravo.Tests/Host/BravoApplicationBuilderTests.cs b/test/Bravo.Tests/Host/BravoApplicationBuilderTests.cs index da2845d4..9527fb31 100644 --- a/test/Bravo.Tests/Host/BravoApplicationBuilderTests.cs +++ b/test/Bravo.Tests/Host/BravoApplicationBuilderTests.cs @@ -1,20 +1,63 @@ -using Sqlbi.Bravo.Host; +using Bravo.Tests.Infrastructure.Policies; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Host; +using Sqlbi.Bravo.Infrastructure.Configuration.Settings; +using Sqlbi.Bravo.Infrastructure.Policies; +using Sqlbi.Bravo.Infrastructure.Telemetry; using Xunit; namespace Bravo.Tests.Host; public class BravoApplicationBuilderTests { - /// - /// Composition smoke test: builds the full service graph through the real builder. In DEBUG the - /// builder enables ValidateOnBuild/ValidateScopes, so this fails on any unresolvable - /// registration. Building does not bind Kestrel or start the host, so no port is claimed. - /// [Fact] - public void Build_ComposesTheFullServiceGraph() + public void Build_ResolvesTheBootstrapInstancesWithoutOwningTheirLifetime() { - using var context = FakeInitializationContext.Create(); + var loggerFactory = new FakeLoggerFactory(); + var telemetry = new FakeTelemetry(); + var policies = new PolicyService(new FakePolicyReader()); + var settings = new UserSettings(); + using var bootstrap = new BootstrapContext(loggerFactory, policies, settings, telemetry); + var instance = new FakeInstance(); + var builder = BravoApplication.CreateBuilder(bootstrap); + builder.Services.Replace(ServiceDescriptor.Singleton((_) => instance)); - using var application = BravoApplication.CreateBuilder(context).Build(); + using (var application = builder.Build()) + { + Assert.Same(loggerFactory, application.Services.GetRequiredService()); + Assert.Same(telemetry, application.Services.GetRequiredService()); + Assert.Same(policies, application.Services.GetRequiredService()); + Assert.Same(settings, application.Services.GetRequiredService()); + Assert.Same(instance, application.Services.GetRequiredService()); + Assert.False(application.Services.GetRequiredService().ApplicationStarted.IsCancellationRequested); + } + + // The container disposes the services it creates, not the registered bootstrap instances. + Assert.Equal(1, instance.DisposeCount); + Assert.Equal(0, telemetry.DisposeCount); + Assert.Equal(0, loggerFactory.DisposeCount); + } + + [Fact] + public void Build_WithoutRunningDoesNotClaimAnInstance() + { + var instanceCreations = 0; + using var bootstrap = new BootstrapContext(new FakeLoggerFactory(), new PolicyService(new FakePolicyReader()), new UserSettings(), new FakeTelemetry()); + var builder = BravoApplication.CreateBuilder(bootstrap); + builder.Services.Replace(ServiceDescriptor.Singleton((_) => + { + instanceCreations++; + return new FakeInstance(); + })); + + using (var application = builder.Build()) + { + Assert.Equal(0, instanceCreations); + } + + Assert.Equal(0, instanceCreations); } -} +} \ No newline at end of file diff --git a/test/Bravo.Tests/Host/BravoApplicationInitializationContextTests.cs b/test/Bravo.Tests/Host/BravoApplicationInitializationContextTests.cs deleted file mode 100644 index 713b0b93..00000000 --- a/test/Bravo.Tests/Host/BravoApplicationInitializationContextTests.cs +++ /dev/null @@ -1,51 +0,0 @@ -using Sqlbi.Bravo.Host; -using Sqlbi.Bravo.Infrastructure.Configuration.Settings; -using Sqlbi.Bravo.Infrastructure.Policies; -using Xunit; - -namespace Bravo.Tests.Host; - -/// -/// Verifies that carries the initialization -/// entries as-is and owns only what the initialization created. -/// -public class BravoApplicationInitializationContextTests -{ - [Fact] - public void Entries_ReturnTheSameObjectsTheInitializationProduced() - { - var instance = new FakeInstance { IsPrimary = true }; - //var policies = Policies.None; - //var userSettings = new UserSettings(); - //var telemetry = new FakeTelemetry(); - //var webProxy = new FakeWebProxy(); - - using var context = new BravoApplicationInitializationContext(instance); - - Assert.Same(instance, context.Instance); - } - - [Fact] - public void Dispose_DisposesTheInstance() - { - var instance = new FakeInstance(); - var context = FakeInitializationContext.Create(instance); - - context.Dispose(); - - Assert.True(instance.Disposed); - } - - //[Fact] - //public void Dispose_DoesNotDisposeEntriesTheInitializationDidNotCreate() - //{ - // // Telemetry is still a process-wide static that owns itself; the context must not dispose it - // // until the initialization creates it. - // var telemetry = new FakeTelemetry(); - // var context = FakeInitializationContext.Create(telemetry: telemetry); - - // context.Dispose(); - - // Assert.False(telemetry.Disposed); - //} -} diff --git a/test/Bravo.Tests/Host/BravoApplicationInstanceTests.cs b/test/Bravo.Tests/Host/BravoApplicationInstanceTests.cs new file mode 100644 index 00000000..5bf96259 --- /dev/null +++ b/test/Bravo.Tests/Host/BravoApplicationInstanceTests.cs @@ -0,0 +1,109 @@ +using System; +using System.IO; +using System.IO.Pipes; +using System.Text; +using System.Text.Json; +using System.Threading; +using Microsoft.Extensions.Logging; +using Sqlbi.Bravo.Host; +using Sqlbi.Bravo.Infrastructure.SingleInstance; +using Xunit; + +namespace Bravo.Tests.Host; + +/// +/// Exercises the reporting paths against a real pipe. Each test uses a unique pipe name so they +/// can run in parallel and never claim the production one. +/// +public class BravoApplicationInstanceTests +{ + private static readonly TimeSpan s_timeout = TimeSpan.FromSeconds(5); + + private static SingleInstanceOptions CreateOptions() => new() + { + PipeName = $"Bravo.Tests.{Guid.NewGuid():N}", + ConnectTimeout = TimeSpan.FromSeconds(1), + }; + + [Fact] + public void RedirectActivationToPrimary_NoPrimary_LogsAnErrorAndTracksTheException() + { + var options = CreateOptions(); + var telemetry = new FakeTelemetry(); + var loggerFactory = new FakeLoggerFactory(); + + using var instance = new BravoApplicationInstance(options, server: null, telemetry, loggerFactory); + + Assert.False(instance.IsPrimary); + + instance.RedirectActivationToPrimary(); + + var exception = Assert.Single(telemetry.Exceptions); + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Error, entry.Level); + Assert.Equal(2, entry.EventId.Id); + Assert.Same(exception, entry.Exception); + Assert.Contains(nameof(SingleInstanceSendStatus.OwnerUnavailable), entry.Message); + } + + [Fact] + public void Activated_ListenerReportsAFailure_LogsAWarningAndTracksTheException() + { + var options = CreateOptions(); + var telemetry = new FakeTelemetry(); + var loggerFactory = new FakeLoggerFactory(); + + Assert.True(SingleInstanceServer.TryStart(options, out var server)); + using var instance = new BravoApplicationInstance(options, server, telemetry, loggerFactory); + + Assert.True(instance.IsPrimary); + + // Connects and closes without writing: the listener reports an empty payload. + using (var silentClient = new NamedPipeClientStream(".", options.PipeName, PipeDirection.Out, PipeOptions.CurrentUserOnly)) + { + silentClient.Connect((int)s_timeout.TotalMilliseconds); + } + + Assert.True(SpinWait.SpinUntil(() => telemetry.Exceptions.Count > 0, s_timeout)); + + // Empty payload when the owner accepted before the close; otherwise the accept itself + // fails with ERROR_NO_DATA. + var exception = Assert.Single(telemetry.Exceptions); + Assert.True(exception is InvalidDataException or IOException, exception.ToString()); + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Warning, entry.Level); + Assert.Equal(1, entry.EventId.Id); + Assert.Same(exception, entry.Exception); + } + + [Fact] + public void Activated_UnreadablePayload_ActivatesWithoutADocumentAndReports() + { + var options = CreateOptions(); + var telemetry = new FakeTelemetry(); + var loggerFactory = new FakeLoggerFactory(); + + Assert.True(SingleInstanceServer.TryStart(options, out var server)); + using var instance = new BravoApplicationInstance(options, server, telemetry, loggerFactory); + + using var activated = new ManualResetEventSlim(); + InstanceActivationRequestedEventArgs? activation = null; + instance.ActivationRequested += (_, e) => + { + activation = e; + activated.Set(); + }; + + Assert.True(SingleInstanceClient.Send(options, Encoding.UTF8.GetBytes("not json")).IsDelivered); + Assert.True(activated.Wait(s_timeout)); + + Assert.NotNull(activation); + Assert.Null(activation.StartupMessage); + + var exception = Assert.IsAssignableFrom(Assert.Single(telemetry.Exceptions)); + var entry = Assert.Single(loggerFactory.Entries); + Assert.Equal(LogLevel.Warning, entry.Level); + Assert.Equal(3, entry.EventId.Id); + Assert.Same(exception, entry.Exception); + } +} diff --git a/test/Bravo.Tests/Host/HostFakes.cs b/test/Bravo.Tests/Host/HostFakes.cs index d2f09e33..9f1bedbd 100644 --- a/test/Bravo.Tests/Host/HostFakes.cs +++ b/test/Bravo.Tests/Host/HostFakes.cs @@ -1,26 +1,20 @@ using System; using System.Collections.Generic; -using System.Net; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Sqlbi.Bravo.Host; -using Sqlbi.Bravo.Infrastructure.Configuration.Settings; -using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.Telemetry; namespace Bravo.Tests.Host; -/// -/// Fakes for the initialization entries. A fake rather than a real -/// is required: the real type always claims the production -/// named-pipe name, so constructing one in a test would risk colliding with an actually-running -/// Bravo instance. -/// +// Avoid claiming the production named pipe in tests. internal sealed class FakeInstance : IBravoApplicationInstance { public bool IsPrimary { get; init; } - - public bool Disposed { get; private set; } + public int DisposeCount { get; private set; } + public Action? OnDispose { get; init; } public event EventHandler? ActivationRequested { @@ -28,47 +22,75 @@ public event EventHandler? ActivationReque remove { } } - public void RedirectActivationToPrimary() + public void RedirectActivationToPrimary() { } + + public void Dispose() { + DisposeCount++; + OnDispose?.Invoke(); } - - public void Dispose() => Disposed = true; } internal sealed class FakeTelemetry : ITelemetryService { - public bool Disposed { get; private set; } + private readonly List _exceptions = []; + public int DisposeCount { get; private set; } + public Action? OnDispose { get; init; } public bool TelemetryEnabled { get; set; } + public IReadOnlyList Exceptions + { + get { lock (_exceptions) return [.. _exceptions]; } + } + public void TrackException(Exception exception, IDictionary? properties = null) { + lock (_exceptions) _exceptions.Add(exception); } - public Task FlushAsync(CancellationToken cancellationToken = default) => Task.FromResult(true); + public Task FlushAsync(CancellationToken cancellationToken = default) => Task.FromResult(false); - public bool TryFlushBeforeShutdown(int timeoutMilliseconds) => true; + public bool TryFlushBeforeShutdown(int timeoutMilliseconds) => false; - public void Dispose() => Disposed = true; + public void Dispose() + { + DisposeCount++; + OnDispose?.Invoke(); + } } -internal sealed class FakeWebProxy : IWebProxy +internal sealed record FakeLogEntry(LogLevel Level, EventId EventId, Exception? Exception, string Message); + +internal sealed class FakeLoggerFactory : ILoggerFactory { - public ICredentials? Credentials { get; set; } + private readonly List _entries = []; - public Uri? GetProxy(Uri destination) => null; + public int DisposeCount { get; private set; } - public bool IsBypassed(Uri host) => true; -} + public IReadOnlyList Entries + { + get { lock (_entries) return [.. _entries]; } + } -internal static class FakeInitializationContext -{ - /// - /// Creates a whose entries are all fakes. - /// - public static BravoApplicationInitializationContext Create(FakeInstance? instance = null) + public void AddProvider(ILoggerProvider provider) { } + + public ILogger CreateLogger(string categoryName) => new FakeLogger(this); + + public void Dispose() => DisposeCount++; + + private sealed class FakeLogger(FakeLoggerFactory owner) : ILogger { - return new BravoApplicationInitializationContext( - instance ?? new FakeInstance()); + private readonly FakeLoggerFactory _owner = owner; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => logLevel != LogLevel.None; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + var entry = new FakeLogEntry(logLevel, eventId, exception, formatter(state, exception)); + lock (_owner._entries) _owner._entries.Add(entry); + } } } diff --git a/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs new file mode 100644 index 00000000..b4e8806a --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Diagnostics/GlobalExceptionHandlingTests.cs @@ -0,0 +1,139 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Sqlbi.Bravo.Infrastructure.Diagnostics; +using Sqlbi.Bravo.Infrastructure.Telemetry; +using Xunit; + +namespace Bravo.Tests.Infrastructure.Diagnostics; + +[CollectionDefinition(nameof(GlobalExceptionHandlingCollection), DisableParallelization = true)] +public sealed class GlobalExceptionHandlingCollection +{ +} + +[Collection(nameof(GlobalExceptionHandlingCollection))] +public class GlobalExceptionHandlingTests : IDisposable +{ + private static readonly TimeSpan s_timeout = TimeSpan.FromSeconds(10); + private static readonly FieldInfo s_handlerField = typeof(GlobalExceptionHandling) + .GetField("s_handler", BindingFlags.NonPublic | BindingFlags.Static)!; + + private readonly object? _previousHandler = s_handlerField.GetValue(null); + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task RuntimeCallbacks_FatalAndUnobservedReportingOverlap_DeliversBoth(bool unobservedFirst) + { + using var releaseFirst = new ManualResetEventSlim(); + var firstEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var reported = new ConcurrentQueue(); + var fatal = new InvalidOperationException("fatal"); + var unobserved = new UnobservedTaskExceptionEventArgs(new AggregateException("unobserved")); + var firstOrigin = unobservedFirst ? ExceptionOrigin.UnobservedTask : ExceptionOrigin.AppDomain; + + SetHandler((exception, origin) => + { + reported.Enqueue(exception); + if (origin == firstOrigin) + { + firstEntered.TrySetResult(); + releaseFirst.Wait(s_timeout); + } + }); + + var first = Task.Run(() => Raise(unobservedFirst)); + var second = Task.CompletedTask; + try + { + await firstEntered.Task.WaitAsync(s_timeout); + + second = Task.Run(() => Raise(!unobservedFirst)); + await second.WaitAsync(s_timeout); + + Assert.False(first.IsCompleted, "The second callback must complete while the first report is still blocked."); + Assert.Contains(fatal, reported); + Assert.Contains(unobserved.Exception, reported); + Assert.Equal(2, reported.Count); + Assert.True(unobserved.Observed); + } + finally + { + releaseFirst.Set(); + await Task.WhenAll(first, second).WaitAsync(s_timeout); + } + + void Raise(bool isUnobserved) + { + if (isUnobserved) + RaiseUnobserved(unobserved); + else + RaiseFatal(fatal); + } + } + + [Fact] + public void RuntimeCallbacks_ReentrantFatalException_ReportsOnlyTheFirst() + { + var reported = new List(); + var first = new InvalidOperationException("first"); + var nested = new InvalidOperationException("nested"); + SetHandler((exception, _) => + { + reported.Add(exception); + if (ReferenceEquals(exception, first)) + RaiseFatal(nested); + }); + + RaiseFatal(first); + + Assert.Same(first, Assert.Single(reported)); + } + + [Fact] + public void RuntimeCallbacks_UnobservedTask_ReportsWithoutDialogOrFlush() + { + var dialog = new RecordingDialog(); + var telemetry = Substitute.For(); + var handler = new UnhandledExceptionHandler(dialog, telemetry, NullLoggerFactory.Instance); + var args = new UnobservedTaskExceptionEventArgs(new AggregateException("unobserved")); + SetHandler(handler.Handle); + + RaiseUnobserved(args); + + Assert.True(args.Observed); + Assert.Equal(0, dialog.ShowCount); + telemetry.Received(1).TrackException(args.Exception, Arg.Is>( + properties => properties["ExceptionOrigin"] == nameof(ExceptionOrigin.UnobservedTask))); + telemetry.DidNotReceive().TryFlushBeforeShutdown(Arg.Any()); + telemetry.DidNotReceive().FlushAsync(Arg.Any()); + } + + public void Dispose() => s_handlerField.SetValue(null, _previousHandler); + + private static void SetHandler(GlobalExceptionHandler handler) => s_handlerField.SetValue(null, handler); + + // Calling the callbacks directly avoids installing fatal exception hooks in the test process. + private static void RaiseFatal(Exception exception) + => typeof(GlobalExceptionHandling) + .GetMethod("OnUnhandledException", BindingFlags.NonPublic | BindingFlags.Static)! + .Invoke(null, [null, new UnhandledExceptionEventArgs(exception, isTerminating: true)]); + + 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++; + } +}