-
-
Notifications
You must be signed in to change notification settings - Fork 375
feat(BootstrapBlazorRoot): add ShowErrorLoggerToast parameter #7485
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
Reviewer's GuideAdds finer-grained control over global error logging and toast behavior via new parameters on BootstrapBlazorRoot and Layout, wires those options through ErrorLogger and error boundary components, updates the error-handling interfaces/method names to async variants, and extends unit tests and test infrastructure to cover the new behavior with logging enabled. Sequence diagram for exception handling with new logging and toast controlssequenceDiagram
participant Component as ChildComponent
participant Boundary as BootstrapBlazorErrorBoundary
participant Logger as ErrorLogger
participant Handler as IHandlerException
participant DotNetLogger as ILogger
Component->>Component: Throw Exception
Component-->>Boundary: Propagate Exception
Boundary->>Logger: HandlerExceptionAsync(exception)
alt EnableErrorLogger
alt EnableILogger
Logger->>DotNetLogger: Log exception
end
alt ShowToast
Logger->>Logger: Render error toast
end
end
alt HostEnvironment IsDevelopment and Handler not null
Boundary->>Handler: HandlerExceptionAsync(exception, ExceptionContent)
Handler->>Handler: Build error content
Handler-->>Boundary: Task completed
Boundary->>Boundary: Display custom error UI
else Production or no handler
Boundary->>Boundary: Fallback error UI or rethrow
end
Class diagram for updated error logging components and interfacesclassDiagram
class BootstrapBlazorRoot {
+bool? EnableErrorLogger
+bool? EnableErrorLoggerILogger
+bool? ShowErrorLoggerToast
+string? ToastTitle
+RenderFragment? ChildContent
+ToastContainer? ToastContainer
+Func~ILogger,Exception,Task~? OnErrorHandleAsync
-bool EnableErrorLoggerValue
-bool EnableErrorLoggerILoggerValue
-bool ShowToastValue
}
class Layout {
+bool? EnableErrorLogger
+bool? EnableErrorLoggerILogger
+bool? ShowErrorLoggerToast
+string? ErrorLoggerToastTitle
+Func~ILogger,Exception,Task~? OnErrorHandleAsync
-bool EnableLogger
-bool EnableILogger
-bool ShowToast
+Task HandlerExceptionAsync(Exception ex, RenderFragment~Exception~ errorContent)
}
class ErrorLogger {
+bool EnableErrorLogger
+bool EnableILogger
+bool ShowToast
+string? ToastTitle
+Func~ILogger,Exception,Task~? OnErrorHandleAsync
+Task HandlerExceptionAsync(Exception ex)
}
class IErrorLogger {
+bool EnableErrorLogger
+bool EnableILogger
+bool ShowToast
+string? ToastTitle
+Task HandlerExceptionAsync(Exception ex)
+void Register(IHandlerException handler)
}
class IHandlerException {
+Task HandlerExceptionAsync(Exception ex, RenderFragment~Exception~ errorContent)
}
class BootstrapBlazorErrorBoundary {
+ErrorLogger? Logger
+Func~ILogger,Exception,Task~? OnErrorHandleAsync
+bool EnableILogger
+bool ShowToast
+string? ToastTitle
+Task RenderException(Exception exception, IHandlerException? handler)
}
class ModalDialog {
+Task HandlerExceptionAsync(Exception ex, RenderFragment~Exception~ errorContent)
}
class TabItemContent {
+Task HandlerExceptionAsync(Exception ex, RenderFragment~Exception~ errorContent)
}
class ToastContainer
class ILogger
BootstrapBlazorRoot --> ErrorLogger : uses
Layout --> ErrorLogger : uses
BootstrapBlazorErrorBoundary --> ErrorLogger : uses
ErrorLogger ..|> IErrorLogger
Layout ..|> IHandlerException
ModalDialog ..|> IHandlerException
TabItemContent ..|> IHandlerException
IErrorLogger --> IHandlerException : registers
BootstrapBlazorErrorBoundary --> IHandlerException : delegates HandlerExceptionAsync
ErrorLogger --> ILogger : logs exceptions
BootstrapBlazorRoot --> ToastContainer : owns
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've found 3 issues, and left some high level feedback:
- The new
HandlerExceptionAsyncimplementations inLayout,ModalDialog, etc. set state but never return aTask, so they should returnTask.CompletedTask(or be markedasyncandawaitsomething) to compile and correctly fulfill the interface contract.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `HandlerExceptionAsync` implementations in `Layout`, `ModalDialog`, etc. set state but never return a `Task`, so they should return `Task.CompletedTask` (or be marked `async` and `await` something) to compile and correctly fulfill the interface contract.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Extensions/IServiceCollectionExtensions.cs:38-47` </location>
<code_context>
return services;
}
+ public static ILoggingBuilder AddMockLoggerProvider(this ILoggingBuilder builder)
+ {
+ return builder.AddProvider(new TestLoggerProvider());
+ }
+
+ class TestLoggerProvider : ILoggerProvider
+ {
+ public ILogger CreateLogger(string categoryName) => new TestLogger();
+
+ public void Dispose() { }
+ }
+
+ class TestLogger : ILogger
+ {
+ public IDisposable? BeginScope<TState>(TState state) where TState : notnull
+ {
+ return null;
+ }
+
+ public bool IsEnabled(LogLevel logLevel) => true;
+
+ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+ {
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider capturing log entries in TestLogger and adding assertions for logging behavior (especially when EnableErrorLoggerILogger is toggled).
Right now `TestLogger` always returns `true` from `IsEnabled` and ignores `Log` calls, so tests don’t validate that toggling `EnableErrorLoggerILogger` actually changes logging behavior. Please have `TestLogger` capture log calls (e.g., count, last message, or log levels) and add assertions that logs are emitted when `EnableErrorLoggerILogger` is true and not when it’s false, to confirm the wiring of these options.
Suggested implementation:
```csharp
using System.Collections.Concurrent;
namespace Microsoft.Extensions.DependencyInjection;
```
```csharp
public static ILoggingBuilder AddMockLoggerProvider(this ILoggingBuilder builder)
{
// Keep a single provider instance so tests can inspect captured logs
var provider = new TestLoggerProvider();
return builder.AddProvider(provider);
}
/// <summary>
/// Captures log entries emitted through ILogger for test assertions.
/// </summary>
class TestLoggerProvider : ILoggerProvider
{
internal readonly ConcurrentQueue<TestLogEntry> LogEntries = new();
public ILogger CreateLogger(string categoryName) => new TestLogger(LogEntries);
public void Dispose()
{
// No unmanaged resources, but clear captured logs on dispose
while (LogEntries.TryDequeue(out _)) { }
}
}
/// <summary>
/// Simple DTO for captured log entries.
/// </summary>
readonly record struct TestLogEntry(
LogLevel LogLevel,
EventId EventId,
string Message,
Exception? Exception);
/// <summary>
/// Test logger that records all log calls into a shared collection.
/// </summary>
class TestLogger : ILogger
{
private readonly ConcurrentQueue<TestLogEntry> _entries;
public TestLogger(ConcurrentQueue<TestLogEntry> entries)
{
_entries = entries;
}
public IDisposable? BeginScope<TState>(TState state) where TState : notnull
{
// Scopes are not needed for tests; return a no-op disposable
return null;
}
public bool IsEnabled(LogLevel logLevel) => true;
public void Log<TState>(
LogLevel logLevel,
EventId eventId,
TState state,
Exception? exception,
Func<TState, Exception?, string> formatter)
{
if (formatter is null)
{
throw new ArgumentNullException(nameof(formatter));
}
var message = formatter(state, exception);
_entries.Enqueue(new TestLogEntry(
logLevel,
eventId,
message,
exception));
}
```
To fully implement your test intent (assert behavior when `EnableErrorLoggerILogger` is toggled), you should:
1. In the relevant test(s), obtain the `TestLoggerProvider` or its `LogEntries` queue:
- Since the provider is currently created inside `AddMockLoggerProvider`, you can either:
- Refactor `AddMockLoggerProvider` to accept an `out TestLoggerProvider provider` so tests can access `provider.LogEntries`, **or**
- Expose a static holder in the test project (e.g., a static `TestLoggerSink` class) that stores the last created provider and its entries.
2. Before each test, clear captured logs:
```csharp
while (provider.LogEntries.TryDequeue(out _)) { }
```
3. In tests, configure services twice:
- Once with `EnableErrorLoggerILogger = true` and assert that `provider.LogEntries` contains at least one entry with the expected `LogLevel`/message.
- Once with `EnableErrorLoggerILogger = false` and assert that no such entries were captured (or that the count did not increase).
You’ll need to integrate access to `TestLoggerProvider` in the test code according to how you build the service provider and configure options in your existing tests.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Components/ErrorLoggerTest.cs:133-142` </location>
<code_context>
[Fact]
- public void Root_Ok()
+ public async Task Root_Ok()
{
Exception? exception = null;
</code_context>
<issue_to_address>
**suggestion (testing):** Add a dedicated test to cover the behavior when EnableErrorLoggerILogger is false (logging disabled) while error handling and/or toasts remain enabled.
The current `Root_Ok` change covers the case where `EnableErrorLoggerILogger` is true and later disabling `EnableErrorLogger` makes the exception bubble. We still need coverage for `EnableErrorLogger == true` and `EnableErrorLoggerILogger == false`, where error handling/toasts remain active but logging is disabled. Once `TestLogger` records calls, please add a test for this configuration that triggers an exception and asserts that the handler/toast executes but no log entry is produced, confirming the separation of handling vs. logging behavior.
Suggested implementation:
```csharp
[Fact]
public async Task Root_LoggingDisabled_HandlerAndToastStillWork()
{
// Arrange
var handlerCalled = false;
var testLogger = new TestLogger<BootstrapBlazorRoot>();
var cut = Context.Render<BootstrapBlazorRoot>(pb =>
{
pb.Add(a => a.EnableErrorLogger, true);
pb.Add(a => a.EnableErrorLoggerILogger, false);
pb.Add(a => a.ShowErrorLoggerToast, true);
pb.Add(a => a.ToastTitle, "Test");
pb.Add(a => a.Logger, testLogger);
pb.Add(a => a.OnErrorHandleAsync, (logger, ex) =>
{
handlerCalled = true;
return Task.CompletedTask;
});
});
// Act: trigger the same action that causes an exception in Root_Ok
var button = cut.Find("button");
await cut.InvokeAsync(() => button.Click());
// Assert: handler executed
Assert.True(handlerCalled);
// Assert: toast still rendered (error handling active)
var toast = cut.Find(".toast-body");
Assert.Contains("Test", toast.TextContent);
// Assert: no log entries were recorded (logging disabled)
Assert.Empty(testLogger.Logs);
}
[Fact]
public async Task Root_Ok()
{
Exception? exception = null;
var cut = Context.Render<BootstrapBlazorRoot>(pb =>
{
pb.Add(a => a.EnableErrorLogger, true);
pb.Add(a => a.EnableErrorLoggerILogger, true);
pb.Add(a => a.ShowErrorLoggerToast, false);
pb.Add(a => a.ToastTitle, "Test");
pb.Add(a => a.OnErrorHandleAsync, (logger, ex) =>
{
});
```
To fully integrate this test you may need to:
1. Adjust the logger injection:
- If the component expects a logger factory or uses a different parameter than `pb.Add(a => a.Logger, testLogger);`, update that line to match your existing pattern (e.g., `LoggerFactory` or a DI registration).
2. Align with your `TestLogger` API:
- If `TestLogger` exposes logged entries under a different property than `.Logs` (e.g., `.Entries`, `.Messages`, or `.LogEntries`), update `Assert.Empty(testLogger.Logs);` accordingly.
3. Ensure the toast selector matches your markup:
- If the toast body uses a different CSS class or structure, adjust `cut.Find(".toast-body")` and the text assertion to match the actual rendered toast.
4. Reuse the same exception trigger as `Root_Ok`:
- If `Root_Ok` does not use a button click to trigger the exception, mirror whatever mechanism it uses (e.g., calling a specific callback or rendering a child component that throws) in the new test so they exercise the same failure path.
</issue_to_address>
### Comment 3
<location> `test/UnitTest/Components/ErrorLoggerTest.cs:166-169` </location>
<code_context>
var cut = Context.Render<BootstrapBlazorRoot>(pb =>
{
pb.Add(a => a.EnableErrorLogger, true);
- pb.Add(a => a.ShowToast, true);
+ pb.Add(a => a.ShowErrorLoggerToast, true);
pb.AddChildContent<Button>(pb =>
{
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a regression test to validate the obsolete ShowToast parameter still correctly proxies to ShowErrorLoggerToast.
The updated test now sets `ShowErrorLoggerToast` directly and no longer exercises the obsolete `ShowToast` property on `BootstrapBlazorRoot`. Please add a test that sets only `ShowToast = true` and asserts that the toast is shown, so we continue to verify the forwarding behavior for existing callers until `ShowToast` is removed.
Suggested implementation:
```csharp
[Fact]
public async Task OnErrorAsync_Ok()
{
var cut = Context.Render<BootstrapBlazorRoot>(pb =>
{
// Use the new parameter directly
pb.Add(e => e.ShowErrorLoggerToast, true);
});
var button = cut.Find("button");
// Trigger the error scenario and verify the toast is shown
await cut.InvokeAsync(() => button.Click());
Assert.Contains("toast", cut.Markup, StringComparison.OrdinalIgnoreCase);
}
[Fact]
public async Task OnErrorAsync_ShowToast_ProxiesToShowErrorLoggerToast()
{
var cut = Context.Render<BootstrapBlazorRoot>(pb =>
{
// Regression test: use the obsolete ShowToast parameter and ensure
// it still forwards to ShowErrorLoggerToast for existing callers.
pb.Add(e => e.ShowToast, true);
});
var button = cut.Find("button");
// Trigger the error scenario in the same way as the main test
await cut.InvokeAsync(() => button.Click());
// Assert that the toast/error logger UI is shown
Assert.Contains("toast", cut.Markup, StringComparison.OrdinalIgnoreCase);
```
These edits assume:
1. `OnErrorAsync_Ok` is fully contained in the snippet you provided (i.e., it ends after `var button = cut.Find("button");`).
2. The presence of `"toast"` in `cut.Markup` is the correct way to assert that the error logger toast is displayed, and `using System;` is already imported so `StringComparison` is available.
If the actual `OnErrorAsync_Ok` test already contains more detailed assertions or different UI selectors, you should:
- Mirror those same post-click assertions in `OnErrorAsync_ShowToast_ProxiesToShowErrorLoggerToast` instead of the generic `Assert.Contains("toast", ...)`.
- If assertions rely on specific CSS classes or component markup, copy that exact assertion pattern to the new test so both tests validate the same behavior, only differing in which parameter is set (`ShowErrorLoggerToast` vs `ShowToast`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7485 +/- ##
===========================================
+ Coverage 99.98% 100.00% +0.01%
===========================================
Files 749 749
Lines 32941 32943 +2
Branches 4576 4577 +1
===========================================
+ Hits 32937 32943 +6
+ Misses 3 0 -3
+ Partials 1 0 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 pull request adds the ShowErrorLoggerToast parameter to the BootstrapBlazorRoot component and related error handling components, improving the API design for error logging configuration. The changes also include renaming a method to follow async naming conventions and reorganizing parameter order for consistency.
Key Changes
- Added new
EnableErrorLoggerILoggerandShowErrorLoggerToastparameters toBootstrapBlazorRoot, with the oldShowToastparameter marked as obsolete - Renamed
HandlerExceptionmethod toHandlerExceptionAsyncacross theIHandlerExceptioninterface and all implementing classes for consistency with async naming conventions - Added mock logger infrastructure to test utilities to support testing of error logging functionality
- Updated all tests to use async/await patterns and new parameter names
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor.cs | Added EnableErrorLoggerILogger and ShowErrorLoggerToast parameters; deprecated ShowToast with backward compatibility |
| src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor | Updated to pass EnableILogger parameter to ErrorLogger component |
| src/BootstrapBlazor/Components/ErrorLogger/ErrorLogger.cs | Reordered parameters for consistency; updated documentation |
| src/BootstrapBlazor/Components/ErrorLogger/IErrorLogger.cs | Reordered property documentation for consistency |
| src/BootstrapBlazor/Components/ErrorLogger/IHandlerException.cs | Renamed method to HandlerExceptionAsync; improved documentation |
| src/BootstrapBlazor/Components/ErrorLogger/BootstrapBlazorErrorBoundary.cs | Updated to call renamed HandlerExceptionAsync method; reordered parameters |
| src/BootstrapBlazor/Components/Layout/Layout.razor.cs | Renamed method implementation; reordered parameters and documentation |
| src/BootstrapBlazor/Components/Layout/Layout.razor | Reordered ErrorLogger parameters for consistency |
| src/BootstrapBlazor/Components/Modal/ModalDialog.razor.cs | Renamed method implementation to HandlerExceptionAsync |
| src/BootstrapBlazor/Components/Tab/TabItemContent.cs | Renamed method implementation to HandlerExceptionAsync |
| test/UnitTest/Extensions/IServiceCollectionExtensions.cs | Added mock logger provider and test logger for testing error logging |
| test/UnitTest/Core/BootstrapBlazorTestBase.cs | Integrated mock logger into test setup; reordered configuration |
| test/UnitTest/Components/ErrorLoggerTest.cs | Updated tests to use new parameter names and async patterns |
| test/UnitTest/Components/BootstrapBlazorRootTest.cs | Removed unused import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #7484
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce configurable error logging behavior and toast display options for BootstrapBlazorRoot and related components, aligning error-handling APIs and tests with the new options.
New Features:
Bug Fixes:
Enhancements:
Tests: