-
Notifications
You must be signed in to change notification settings - Fork 670
Fix Blazor error logging to telemetry #9304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the issue of Blazor error logging by replacing the TelemetryErrorBoundary with an ILoggerProvider that sends error details to telemetry, ensuring that errors are logged in the app host console.
- Introduces a new TelemetryLoggerProvider and associated TelemetryLogger class.
- Updates dependency injection in DashboardWebApplication to register the new logger provider.
- Removes the TelemetryErrorBoundary component.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Aspire.Dashboard/Telemetry/TelemetryLoggerProvider.cs | Adds a new logger provider to capture and report telemetry for Blazor errors. |
src/Aspire.Dashboard/DashboardWebApplication.cs | Registers the new TelemetryLoggerProvider in the DI container. |
src/Aspire.Dashboard/Components/Controls/TelemetryErrorBoundary.cs | Removes the obsolete error boundary that was previously used for error logging. |
compilation errors |
@@ -240,6 +240,7 @@ public DashboardWebApplication( | |||
builder.Services.TryAddScoped<ComponentTelemetryContextProvider>(); | |||
builder.Services.TryAddSingleton<DashboardTelemetryService>(); | |||
builder.Services.TryAddSingleton<IDashboardTelemetrySender, DashboardTelemetrySender>(); | |||
builder.Services.AddSingleton<ILoggerProvider, TelemetryLoggerProvider>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this usage pattern adds a further ILoggerProvider
rather than replacing the one that would already have been in place, so you still get normal logging output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's what happens when you call LoggerFactory.AddProvider
: https://github.com/dotnet/runtime/blob/f76d75739482939fcf2661010bf490a3d81163ed/src/libraries/Microsoft.Extensions.Logging/src/LoggingBuilderExtensions.cs#L37
Other methods to add customer loggers use TryAddEnumerable
. Example. I think that's to avoid double registration of a provider which isn't a problem in our case because we control all registrations in the dashboard app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SteveSandersonMS I added a test.
9fd1906
to
f478bbd
Compare
/backport to release/9.3 |
Started backporting to release/9.3: https://github.com/dotnet/aspire/actions/runs/15023976015 |
Description
The error boundary in the dashboard prevents errors from being logged to the console and causes the page to go blank.
There is an error boundary in the dashboard to receive unhandled Blazor errors and send them to telemetry. This works. But the error boundary has negative side effects of blanking the page and preventing the error being logged to the app host console.
The impact on the user is they don't know why the page went blank. They don't get an error feedback in the browser or the console.
The fix is to remove the error boundary and use a logger provider instead. Blazor always writes an error message and we can use that as a hook to send the exception to telemetry.
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template