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 6 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
2 changes: 1 addition & 1 deletion src/Components/Components/src/ErrorBoundaryBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public abstract class ErrorBoundaryBase : ComponentBase, IErrorBoundary
/// <summary>
/// Gets the current exception, or null if there is no exception.
/// </summary>
protected Exception? CurrentException { get; private set; }
protected internal Exception? CurrentException { get; private set; }

/// <summary>
/// Resets the error boundary to a non-errored state. If the error boundary is not
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 exception, don't queue an empty render
// as it would override the error content that should be displayed.
if (errorBoundary is not ErrorBoundaryBase { CurrentException: { } })
{
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.

@copilot This is not the fix.

Check ComponentBase.cs and ComponentBase.cs tests. Author some unit tests to check the behavior of OnInitialized, OnInitializedAsync, OnParametersSet and OnParametersSetAsync.

Take into account all the states a Task returning method can be in. Namely:

  • The method succeeds synchronously.
  • The method succeeds asynchronously.
  • The method returns a cancelled task Synchronously.
  • The method returns a cancelled task Asynchronously.
  • The method throws an exception synchronously.
  • The method throws an exception asynchronously.
  • The method uses async/await and throws an exception synchronously.
  • The method uses async/await and throws an exception asynchronously.

If there are tests for some of these, don't add extra tests, but if there are missing tests for the cases above, add them.

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 reverted the Renderer.cs changes and added comprehensive unit tests for ComponentBase lifecycle methods covering all the async patterns you mentioned. All tests are passing, indicating the current behavior handles these scenarios correctly.

I've added tests for:

  • Synchronous and asynchronous success cases
  • Cancelled tasks (both sync and async return)
  • Exceptions thrown synchronously and asynchronously
  • Async/await patterns with both sync and async exceptions

The multiple exceptions test case I added also passes, suggesting the current ComponentBase behavior is working as expected. Could you provide more guidance on what specific issue I should be looking for in the ComponentBase behavior? Commit 6a19315.

Copy link
Member

Choose a reason for hiding this comment

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

Keep track of all the times StateHasChanged gets called. You can do so probably by putting something on BuildRenderTree for the component.

When you have those set of tests, replicate the same set of tests but wrapping them inside an ErrorBoundary. You should validate the rendered contents of the error boundary.

Copy link
Member

Choose a reason for hiding this comment

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

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 added comprehensive StateHasChanged tracking tests for ComponentBase lifecycle methods by modifying the TestComponent to count BuildRenderTree calls. The tests cover all the async patterns you mentioned:

  • OnInitializedAsync and OnParametersSetAsync success scenarios (sync/async)
  • Cancelled tasks (sync/async return)
  • Exceptions thrown synchronously and asynchronously
  • Async/await patterns with exceptions
  • Task.FromException tests

Each test verifies the exact number of StateHasChanged calls (BuildRenderTree invocations) that occur in different scenarios. I'm now working on the corresponding ErrorBoundary tests that validate rendered contents as you requested. Commit af494c9.


try
{
Expand Down
68 changes: 68 additions & 0 deletions src/Components/Components/test/RendererTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4779,6 +4779,47 @@ public async Task EventDispatchExceptionsCanBeHandledByClosestErrorBoundary_Afte
component => Assert.Same(exception, component.ReceivedException));
}

[Fact]
public void ErrorBoundaryHandlesMultipleExceptionsFromSameComponent()
{
// This test reproduces the issue where a component throws exceptions in both
// parameter setting (lifecycle) and during rendering. The fix ensures that
// the ErrorBoundary can properly handle this scenario without empty content.

// Arrange
var renderer = new TestRenderer();
var parameterException = new InvalidOperationException("Error during parameter setting");
var renderException = new InvalidOperationException("Error during render");

var rootComponentId = renderer.AssignRootComponentId(new TestComponent(builder =>
{
builder.OpenComponent<MultiExceptionErrorBoundary>(0);
builder.AddComponentParameter(1, nameof(MultiExceptionErrorBoundary.ChildContent), (RenderFragment)(builder =>
{
builder.OpenComponent<ErrorThrowingComponent>(0);
builder.AddComponentParameter(1, nameof(ErrorThrowingComponent.ThrowDuringParameterSettingSync), parameterException);
builder.AddComponentParameter(2, nameof(ErrorThrowingComponent.ThrowDuringRender), renderException);
builder.CloseComponent();
}));
builder.CloseComponent();
}));

// Act
renderer.RenderRootComponent(rootComponentId);

// Assert
var batch = renderer.Batches.Single();
var errorBoundary = batch.GetComponentFrames<MultiExceptionErrorBoundary>().Single().Component as MultiExceptionErrorBoundary;

// The error boundary should have received at least one exception and be able to render properly
Assert.NotNull(errorBoundary.LastReceivedException);
Assert.True(errorBoundary.ExceptionCount > 0);

// The component should be disposed since it failed
var failedComponentId = batch.GetComponentFrames<ErrorThrowingComponent>().Single().ComponentId;
Assert.Contains(failedComponentId, batch.DisposedComponentIDs);
}

[Fact]
public async Task CanRemoveRootComponents()
{
Expand Down Expand Up @@ -6046,6 +6087,33 @@ public async Task HandleEventAsync(EventCallbackWorkItem item, object arg)
}
}

private class MultiExceptionErrorBoundary : AutoRenderComponent, IErrorBoundary
{
public Exception LastReceivedException { get; private set; }
public int ExceptionCount { get; private set; }

[Parameter] public RenderFragment ChildContent { get; set; }

protected override void BuildRenderTree(RenderTreeBuilder builder)
{
if (LastReceivedException is not null)
{
builder.AddContent(0, $"Error: {LastReceivedException.Message}");
}
else
{
ChildContent(builder);
}
}

public void HandleException(Exception error)
{
LastReceivedException = error;
ExceptionCount++;
TriggerRender();
}
}

private class CallbackDuringSetParametersAsyncComponent : AutoRenderComponent
{
public int RenderCount { get; private set; }
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