feat(ErrorLogger): support native html element throw exception #7493
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/BootstrapBlazor/Components/ErrorLogger/BootstrapBlazorErrorBoundary.cs
Contributor
Reviewer's GuideExtends the ErrorLogger and tab components to support configurable, centralized exception handling (including for native HTML element errors) via a shared IErrorLogger, new Tab parameters, and handler-based routing of errors to dialogs or toasts. Sequence diagram for Tab content exception handling via IErrorLogger and handlerssequenceDiagram
actor User
participant Browser
participant TabItemContent
participant BootstrapBlazorErrorBoundary as ErrorBoundary
participant ErrorLogger
participant Handler as TabItemContentHandler
participant DialogService
participant ToastService
User->>Browser: Interact with tab content
Browser->>TabItemContent: Invoke component render and events
TabItemContent->>Browser: Render native HTML and child content
Browser-->>ErrorBoundary: Native HTML element throws exception
ErrorBoundary->>ErrorBoundary: Capture exception (CurrentException)
ErrorBoundary->>ErrorBoundary: ResetException()
ErrorBoundary->>ErrorLogger: GetLastOrDefaultHandler()
ErrorLogger-->>ErrorBoundary: return TabItemContentHandler
ErrorBoundary->>ErrorLogger: RenderException(exception, TabItemContentHandler)
activate ErrorLogger
ErrorLogger->>ErrorLogger: OnErrorAsync(exception)
alt Custom OnErrorHandleAsync provided
ErrorLogger->>ErrorLogger: OnErrorHandleAsync(Logger, exception)
end
ErrorLogger->>TabItemContentHandler: HandlerExceptionAsync(exception, ExceptionContent)
deactivate ErrorLogger
activate TabItemContentHandler
TabItemContentHandler->>TabItemContentHandler: Read DetailedErrors from configuration
alt DetailedErrors == true
TabItemContentHandler->>DialogService: ShowErrorHandlerDialog(errorContent(exception))
else DetailedErrors == false and ShowErrorLoggerToast == true
TabItemContentHandler->>ToastService: Error(ToastTitle, exception.Message)
else fallback
TabItemContentHandler->>DialogService: ShowErrorHandlerDialog(errorContent(exception))
end
deactivate TabItemContentHandler
ErrorBoundary->>Browser: Render ChildContent (normal UI)
Browser-->>User: Display error dialog or toast, UI continues rendering
Class diagram for centralized error handling with IErrorLogger and Tab integrationclassDiagram
class IErrorLogger {
<<interface>>
+Task HandlerExceptionAsync(Exception exception)
+void Register(IHandlerException handler)
}
class IHandlerException {
<<interface>>
+Task HandlerExceptionAsync(Exception exception, RenderFragment~Exception~ errorContent)
}
class ErrorLogger {
+bool EnableErrorLogger
+bool EnableILogger
+bool ShowToast
+string ToastTitle
+Func~IErrorLogger, Task~ OnInitializedCallback
+Func~ILogger, Exception, Task~ OnErrorHandleAsync
+Task HandlerExceptionAsync(Exception exception)
+void Register(IHandlerException handler)
+IHandlerException GetLastOrDefaultHandler()
}
class BootstrapBlazorErrorBoundary {
+RenderFragment ChildContent
+Func~ILogger, Exception, Task~ OnErrorHandleAsync
+string ToastTitle
-IErrorLogger ErrorLogger
-Exception _exception
+override void BuildRenderTree(RenderTreeBuilder builder)
+Task RenderException(Exception exception, IHandlerException handler)
-IHandlerException GetLastOrDefaultHandler()
-Task ShowErrorToast(Exception exception)
-Task OnErrorAsync(Exception exception)
-void ResetException()
}
class Tab {
+bool? EnableErrorLogger
+bool? EnableErrorLoggerILogger
+bool? ShowErrorLoggerToast
+string ErrorLoggerToastTitle
+Func~ILogger, Exception, Task~ OnErrorHandleAsync
}
class TabItemContent {
+TabItem Item
+Tab TabSet
-IErrorLogger _logger
-bool EnableErrorLogger
-bool EnableErrorLoggerILogger
-bool ShowErrorLoggerToast
-string ToastTitle
-bool? _errorDetails
+void Attach(RenderHandle renderHandle)
+void SetParameters(ParameterView parameters)
+void Render()
+Task HandlerExceptionAsync(Exception ex, RenderFragment~Exception~ errorContent)
+void Dispose()
}
class Layout {
-IErrorLogger _errorLogger
-Task OnErrorLoggerInitialized(IErrorLogger logger)
+bool EnableLogger
+bool EnableErrorLoggerILogger
+bool ShowErrorLoggerToast
+string ErrorLoggerToastTitle
+Func~ILogger, Exception, Task~ OnErrorHandleAsync
}
class DialogService {
+Task ShowErrorHandlerDialog(RenderFragment content)
}
class ToastService {
+Task Error(string title, string message)
}
Layout --> Tab : provides parameters
Tab --> TabItemContent : owns TabItemContent
TabItemContent ..> IHandlerException : implements
TabItemContent ..> IComponent : implements
TabItemContent --> IErrorLogger : uses _logger
TabItemContent --> DialogService : uses
TabItemContent --> ToastService : uses
ErrorLogger ..> IErrorLogger : implements
ErrorLogger --> BootstrapBlazorErrorBoundary : contains
ErrorLogger --> IHandlerException : registers handlers
BootstrapBlazorErrorBoundary --> IErrorLogger : cascading ErrorLogger
BootstrapBlazorErrorBoundary --> IHandlerException : uses handler for exceptions
Layout --> IErrorLogger : holds reference
Layout --> ErrorLogger : OnInitializedCallback
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In
ErrorLogger,GetLastOrDefaultHandler()now returns_cache.FirstOrDefault(), which both contradicts the method name and changes existing behavior fromLastOrDefault(); if the intent is to keep the previous semantics, this should remainLastOrDefault()or the method should be renamed to reflect the new behavior. - In
BootstrapBlazorErrorBoundary.BuildRenderTree, the exception path now callsRenderException(ex, handler)in a fire-and-forget way (_ = ...) and still rendersChildContentregardless of errors; consider whether this change in UI behavior is intentional and, if so, whetherRenderExceptionshould be invoked viaInvokeAsyncor otherwise wrapped so that any thrown exceptions are observed and logged.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ErrorLogger`, `GetLastOrDefaultHandler()` now returns `_cache.FirstOrDefault()`, which both contradicts the method name and changes existing behavior from `LastOrDefault()`; if the intent is to keep the previous semantics, this should remain `LastOrDefault()` or the method should be renamed to reflect the new behavior.
- In `BootstrapBlazorErrorBoundary.BuildRenderTree`, the exception path now calls `RenderException(ex, handler)` in a fire-and-forget way (`_ = ...`) and still renders `ChildContent` regardless of errors; consider whether this change in UI behavior is intentional and, if so, whether `RenderException` should be invoked via `InvokeAsync` or otherwise wrapped so that any thrown exceptions are observed and logged.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/ErrorLogger/BootstrapBlazorErrorBoundary.cs:89-90` </location>
<code_context>
+ ResetException();
+
+ // 渲染异常
+ var handler = GetLastOrDefaultHandler();
+ if (handler != null)
{
- _ = OnErrorHandleAsync(Logger, ex);
</code_context>
<issue_to_address>
**issue (bug_risk):** No error handling occurs when no handler is registered, changing previous fallback behavior.
Previously the boundary always rendered fallback error UI (`ExceptionContent(ex)`) when no external `OnErrorHandleAsync` was provided. Now, because `RenderException(ex, handler)` is only called when `handler != null`, the code just calls `ResetException()` and proceeds to render `ChildContent`, swallowing the error and risking repeated failures. You can preserve the old behavior by always calling `RenderException(ex, handler)` and letting it handle a `null` handler (e.g., `RenderException(ex, null)` to trigger `ShowErrorToast`).
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/ErrorLogger/BootstrapBlazorErrorBoundary.cs:96-97` </location>
<code_context>
- // 渲染异常内容
- builder.AddContent(0, ExceptionContent(ex));
+ // 渲染正常内容
+ builder.AddContent(1, ChildContent);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Rendering `ChildContent` even when an exception exists may cause loops and hides the error UI.
With the current logic, when `ex != null` you clear the exception, optionally render the error via `RenderException`, and then always render `ChildContent`. That both bypasses the dedicated error UI and can immediately re-trigger the same exception on the next render, potentially causing a render loop. Consider only rendering `ChildContent` when `ex == null` (or after an explicit recovery) to keep correct error-boundary behavior.
</issue_to_address>
### Comment 3
<location> `src/BootstrapBlazor/Components/ErrorLogger/ErrorLogger.cs:140` </location>
<code_context>
private readonly List<IHandlerException> _cache = [];
+ internal IHandlerException? GetLastOrDefaultHandler() => _cache.FirstOrDefault();
+
/// <summary>
</code_context>
<issue_to_address>
**issue (bug_risk):** Method name `GetLastOrDefaultHandler` conflicts with its implementation using `FirstOrDefault`, changing semantics from the previous `LastOrDefault` usage.
`HandlerExceptionAsync` previously used `_cache.LastOrDefault()`, so the most recently registered handler ran first. After the refactor, `GetLastOrDefaultHandler()` now returns `_cache.FirstOrDefault()`, reversing that priority and making the method name misleading. If last-registered should still win, this should use `_cache.LastOrDefault()` to preserve behavior and avoid unexpected ordering changes.
</issue_to_address>
### Comment 4
<location> `src/BootstrapBlazor/Components/Layout/Layout.razor:152-153` </location>
<code_context>
OnToolbarRefreshCallback="OnToolbarRefreshCallback" TabHeader="TabHeader" OnCloseTabItemAsync="OnCloseTabItemAsync"
Body="@Main" NotAuthorized="NotAuthorized!" NotFound="NotFound!" NotFoundTabText="@NotFoundTabText"
- EnableErrorLogger="@EnableLogger" ErrorLoggerToastTitle="@ErrorLoggerToastTitle">
+ EnableErrorLogger="@EnableLogger" EnableErrorLoggerILogger="@EnableErrorLoggerILogger"
+ ShowErrorLoggerToast="ShowErrorLoggerToast" ErrorLoggerToastTitle="@ErrorLoggerToastTitle"
+ OnErrorHandleAsync="OnErrorHandleAsync">
</Tab>;
</code_context>
<issue_to_address>
**issue (bug_risk):** The `ShowErrorLoggerToast` parameter is bound as a string literal instead of a C# expression.
In Razor, `ShowErrorLoggerToast="ShowErrorLoggerToast"` passes the literal string to a `bool?` parameter on `Tab`, which will not work correctly. To bind the property, use `ShowErrorLoggerToast="@ShowErrorLoggerToast"`, matching the other bound attributes.
</issue_to_address>
### Comment 5
<location> `src/BootstrapBlazor/Components/Tab/TabItemContent.cs:103` </location>
<code_context>
/// <param name="ex"></param>
/// <param name="errorContent"></param>
- public Task HandlerExceptionAsync(Exception ex, RenderFragment<Exception> errorContent) => DialogService.ShowErrorHandlerDialog(errorContent(ex));
+ public async Task HandlerExceptionAsync(Exception ex, RenderFragment<Exception> errorContent)
+ {
+ _errorDetails ??= Configuration.GetValue("DetailedErrors", false);
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new error-handling logic by collapsing duplicate branches and encapsulating the effective error-display strategy behind helper properties/methods.
The new error-handling flow can be simplified without losing any behavior.
### 1. Reduce branching in `HandlerExceptionAsync`
The current logic:
```csharp
private bool? _errorDetails;
public async Task HandlerExceptionAsync(Exception ex, RenderFragment<Exception> errorContent)
{
_errorDetails ??= Configuration.GetValue("DetailedErrors", false);
if (_errorDetails is true)
{
await DialogService.ShowErrorHandlerDialog(errorContent(ex));
}
else if (ShowErrorLoggerToast)
{
await ToastService.Error(ToastTitle, ex.Message);
}
else
{
await DialogService.ShowErrorHandlerDialog(errorContent(ex));
}
}
```
The two dialog branches are identical, and `bool?` is only used for lazy loading. You can keep the lazy load and behavior but collapse it to a single decision:
```csharp
private bool _detailedErrorsLoaded;
private bool _showDetailedErrors;
public async Task HandlerExceptionAsync(Exception ex, RenderFragment<Exception> errorContent)
{
if (!_detailedErrorsLoaded)
{
_showDetailedErrors = Configuration.GetValue("DetailedErrors", false);
_detailedErrorsLoaded = true;
}
// Use dialog if detailed errors are on, or if toast is disabled
var useDialog = _showDetailedErrors || !ShowErrorLoggerToast;
if (useDialog)
{
await DialogService.ShowErrorHandlerDialog(errorContent(ex));
}
else
{
await ToastService.Error(ToastTitle, ex.Message);
}
}
```
This keeps all existing modes:
- `DetailedErrors == true` → dialog
- `DetailedErrors == false && ShowErrorLoggerToast == true` → toast
- `DetailedErrors == false && ShowErrorLoggerToast == false` → dialog
but with a single call site per UI path and no nullable state.
### 2. Centralize effective error-display configuration
You already have:
```csharp
private bool EnableErrorLogger => TabSet.EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger;
private bool EnableErrorLoggerILogger => TabSet.EnableErrorLoggerILogger ?? Options.CurrentValue.EnableErrorLoggerILogger;
private bool ShowErrorLoggerToast => TabSet.ShowErrorLoggerToast ?? Options.CurrentValue.ShowErrorLoggerToast;
private string ToastTitle => TabSet.ErrorLoggerToastTitle ?? "Error";
```
To reduce mental indirection, you can also encapsulate the “effective strategy” so `HandlerExceptionAsync` doesn’t need to know about `TabSet`/`Options` details:
```csharp
private bool UseToastForErrors => !GetDetailedErrorsFlag() && ShowErrorLoggerToast;
private bool GetDetailedErrorsFlag()
{
if (!_detailedErrorsLoaded)
{
_showDetailedErrors = Configuration.GetValue("DetailedErrors", false);
_detailedErrorsLoaded = true;
}
return _showDetailedErrors;
}
```
Then `HandlerExceptionAsync` becomes:
```csharp
public async Task HandlerExceptionAsync(Exception ex, RenderFragment<Exception> errorContent)
{
if (UseToastForErrors)
{
await ToastService.Error(ToastTitle, ex.Message);
}
else
{
await DialogService.ShowErrorHandlerDialog(errorContent(ex));
}
}
```
That keeps all functionality while making the error strategy easier to reason about in one place.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
# Conflicts: # src/BootstrapBlazor/Components/Tab/TabItemContent.cs
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses issue #7462, which reported that error logging toast notifications were not being displayed correctly in BootstrapBlazor version 10.2.0. The PR adds support for native HTML element exception handling and introduces new parameters to control error logging behavior.
Changes:
- Added new parameters (
EnableErrorLoggerILogger,ShowErrorLoggerToast,OnErrorHandleAsync) to Tab and Layout components for more granular error handling control - Modified error handling logic in TabItemContent to check
DetailedErrorsconfiguration and conditionally show toast or dialog based on settings - Changed ErrorLogger callback signature from
Func<ErrorLogger, Task>toFunc<IErrorLogger, Task>for better abstraction - Restructured BootstrapBlazorErrorBoundary's exception handling flow
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ErrorLogger.cs | Changed OnInitializedCallback signature to use IErrorLogger interface; modified GetLastOrDefaultHandler to use FirstOrDefault |
| BootstrapBlazorErrorBoundary.cs | Restructured BuildRenderTree exception handling logic; always renders ChildContent after exception handling |
| TabItemContent.cs | Added DetailedErrors configuration check and toast/dialog selection logic in HandlerExceptionAsync |
| Tab.razor.cs | Added EnableErrorLoggerILogger, ShowErrorLoggerToast, and OnErrorHandleAsync parameters |
| Layout.razor | Passed new error logger parameters to Tab component |
| Layout.razor.cs | Changed _errorLogger type from ErrorLogger to IErrorLogger |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7493 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 748 -1
Lines 32943 32978 +35
Branches 4577 4583 +6
=========================================
+ Hits 32943 32978 +35
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:
|
ArgoZhang
added
enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7462
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add configurable error logging and handling support for tab contents and native HTML element exceptions, integrating ErrorLogger more flexibly across layout and tab components.
New Features:
Enhancements: