Skip to content

Fix ErrorBoundary not rendering ErrorContent when multiple exceptions occur #63145

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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Components/Components/src/ErrorBoundaryBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public abstract class ErrorBoundaryBase : ComponentBase, IErrorBoundary
/// </summary>
protected Exception? CurrentException { get; private set; }

/// <summary>
/// Gets a value indicating whether the error boundary is currently in an error state.
/// </summary>
internal bool IsInErrorState => CurrentException is not null;

/// <summary>
/// Resets the error boundary to a non-errored state. If the error boundary is not
/// already in an errored state, the call has no effect.
Expand Down
7 changes: 6 additions & 1 deletion src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,12 @@ private void HandleExceptionViaErrorBoundary(Exception error, ComponentState? er
// Don't just trust the error boundary to dispose its subtree - force it to do so by
// making it render an empty fragment. Ensures that failed components don't continue to
// operate, which would be a whole new kind of edge case to support forever.
AddToRenderQueue(candidate.ComponentId, builder => { });
// However, if the error boundary already has an error (is in error state),
// don't queue an empty render as it would override the error content that should be displayed.
if (errorBoundary is ErrorBoundaryBase errorBoundaryBase && !errorBoundaryBase.IsInErrorState)
{
AddToRenderQueue(candidate.ComponentId, builder => { });
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do this IsInErrorState, just compare against { CurrentException: { } }

Also, this is not correct according to

                // Don't just trust the error boundary to dispose its subtree - force it to do so by
                // making it render an empty fragment. Ensures that failed components don't continue to
                // operate, which would be a whole new kind of edge case to support forever.

We want to trigger an empty render in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the implementation to use the { CurrentException: { } } pattern as requested instead of the IsInErrorState property. I also made CurrentException protected internal to allow access from the Renderer while maintaining the protected API for derived classes. The fix now correctly prevents empty renders from overriding error content when multiple exceptions occur. Commit 6d4016c.


try
{
Expand Down
14 changes: 14 additions & 0 deletions src/Components/test/E2ETest/Tests/ErrorBoundaryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,20 @@ public void CanHandleMultipleAsyncErrorsFromDescendants()
AssertGlobalErrorState(false);
}

[Fact]
public void CanHandleTwoErrorsInSameComponent()
{
var container = Browser.Exists(By.Id("two-errors-test"));

// Trigger the component with two errors
container.FindElement(By.Id("show-two-errors-component")).Click();

// Should show error content, not be blank
Browser.Exists(By.ClassName("two-errors-error-content"));

AssertGlobalErrorState(false);
}

void AssertGlobalErrorState(bool hasGlobalError)
{
var globalErrorUi = Browser.Exists(By.Id("blazor-error-ui"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@{
throw new Exception("error2");
}

@code
{
protected override async Task OnInitializedAsync()
{
await Task.Yield(); // Make it actually async
throw new Exception("error");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@
<button class="throw-in-children" @onclick="@(() => { multipleChildrenBeginDelayedError = true; })">Cause multiple errors</button>
</div>

<hr />
<h2>Two different errors in same component</h2>
<p>This tests the case where a component throws in both OnInitializedAsync and during rendering.</p>
<div id="two-errors-test">
@if (showTwoErrorsComponent)
{
<ErrorBoundary>
<ChildContent>
<ComponentWithTwoErrors />
</ChildContent>
<ErrorContent>
<div class="two-errors-error-content" style="background: red">@context</div>
</ErrorContent>
</ErrorBoundary>
}
<button id="show-two-errors-component" @onclick="@(() => showTwoErrorsComponent = true)">Show component with two errors</button>
</div>

<hr />
<h2>Dispatch exception to renderer</h2>
<p>Use DispatchExceptionAsync to see if exceptions are correctly dispatched to the renderer.</p>
Expand All @@ -122,6 +140,7 @@
private bool disposalTestRemoveErrorBoundary;
private bool disposalTestBeginDelayedError;
private bool multipleChildrenBeginDelayedError;
private bool showTwoErrorsComponent;

void EventHandlerErrorSync()
=> throw new InvalidTimeZoneException("Synchronous error from event handler");
Expand Down
Loading