Skip to content

Commit 1dac45a

Browse files
Redesign around ErrorBoundaryBase abstract base class.
1 parent 157ca4e commit 1dac45a

File tree

9 files changed

+196
-135
lines changed

9 files changed

+196
-135
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Runtime.ExceptionServices;
6+
using System.Threading.Tasks;
7+
using Microsoft.AspNetCore.Components.Rendering;
8+
9+
namespace Microsoft.AspNetCore.Components
10+
{
11+
/// <summary>
12+
/// A base class for error boundary components.
13+
/// </summary>
14+
public abstract class ErrorBoundaryBase : IComponent
15+
{
16+
// This deliberately doesn't inherit from ComponentBase because it's not intended to be
17+
// subclassable using a .razor file. ErrorBoundaryBase shouldn't be used as a base class
18+
// for all components indiscriminately, because that will lead to undesirable error-ignoring
19+
// patterns. We expect that subclassing ErrorBoundaryBase to be done mainly by platform
20+
// authors, providing just a default error UI for their rendering technology and not
21+
// customizing other aspects of the semantics, such as whether to re-render after an error.
22+
23+
private RenderHandle _renderHandle;
24+
private Exception? _currentException;
25+
26+
/// <summary>
27+
/// The content to be displayed when there is no error.
28+
/// </summary>
29+
[Parameter] public RenderFragment? ChildContent { get; set; }
30+
31+
/// <summary>
32+
/// The content to be displayed when there is an error.
33+
/// </summary>
34+
[Parameter] public RenderFragment<Exception>? ErrorContent { get; set; }
35+
36+
/// <summary>
37+
/// Specifies whether to reset the error state each time this component instance is rendered
38+
/// by its parent. This allows the child content to be recreated in an attempt to recover from the error.
39+
/// </summary>
40+
[Parameter] public bool AutoReset { get; set; }
41+
42+
/// <inheritdoc />
43+
public void Attach(RenderHandle renderHandle)
44+
{
45+
_renderHandle = renderHandle;
46+
}
47+
48+
/// <inheritdoc />
49+
public Task SetParametersAsync(ParameterView parameters)
50+
{
51+
foreach (var parameter in parameters)
52+
{
53+
if (parameter.Name.Equals(nameof(ChildContent), StringComparison.OrdinalIgnoreCase))
54+
{
55+
ChildContent = (RenderFragment)parameter.Value;
56+
}
57+
else if (parameter.Name.Equals(nameof(ErrorContent), StringComparison.OrdinalIgnoreCase))
58+
{
59+
ErrorContent = (RenderFragment<Exception>)parameter.Value;
60+
}
61+
else if (parameter.Name.Equals(nameof(AutoReset), StringComparison.OrdinalIgnoreCase))
62+
{
63+
AutoReset = (bool)parameter.Value;
64+
}
65+
else
66+
{
67+
throw new ArgumentException($"The component '{GetType().FullName}' does not accept a parameter with the name '{parameter.Name}'.");
68+
}
69+
}
70+
71+
if (AutoReset)
72+
{
73+
_currentException = null;
74+
}
75+
76+
_renderHandle.Render(Render);
77+
return Task.CompletedTask;
78+
}
79+
80+
/// <summary>
81+
/// Logs the exception.
82+
/// </summary>
83+
/// <param name="exception">The <see cref="Exception"/> being handled.</param>
84+
protected abstract ValueTask LogExceptionAsync(Exception exception);
85+
86+
/// <summary>
87+
/// Renders the default error content. This will only be used when <see cref="ErrorContent"/>
88+
/// was not supplied.
89+
/// </summary>
90+
/// <param name="builder">The <see cref="RenderTreeBuilder"/></param>
91+
/// <param name="exception">The current exception.</param>
92+
protected abstract void RenderDefaultErrorContent(RenderTreeBuilder builder, Exception exception);
93+
94+
internal void HandleException(Exception exception)
95+
{
96+
if (_currentException is not null)
97+
{
98+
// If there's an error while we're already displaying error content, then it's the
99+
// error content that's failing. Avoid the risk of an infinite error rendering loop.
100+
ExceptionDispatchInfo.Capture(exception).Throw();
101+
}
102+
103+
// TODO: If there's an async exception here, do something with it (can be fatal)
104+
_ = LogExceptionAsync(exception);
105+
106+
_currentException = exception;
107+
_renderHandle.Render(Render);
108+
}
109+
110+
private void Render(RenderTreeBuilder builder)
111+
{
112+
if (_currentException is null)
113+
{
114+
builder.AddContent(0, ChildContent);
115+
}
116+
else if (ErrorContent is not null)
117+
{
118+
builder.AddContent(1, ErrorContent(_currentException));
119+
}
120+
else
121+
{
122+
// Even if the subclass tries to re-render the same ChildContent as its default error content,
123+
// we still won't reuse the subtree components because they are in a different region. So we
124+
// can be sure the old tree will be correctly disposed.
125+
builder.OpenRegion(2);
126+
RenderDefaultErrorContent(builder, _currentException);
127+
builder.CloseRegion();
128+
}
129+
}
130+
}
131+
}

src/Components/Components/src/IErrorBoundary.cs

-29
This file was deleted.

src/Components/Components/src/PublicAPI.Unshipped.txt

+12-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,16 @@ Microsoft.AspNetCore.Components.ComponentApplicationState.PersistAsJson<TValue>(
88
Microsoft.AspNetCore.Components.ComponentApplicationState.PersistState(string! key, byte[]! value) -> void
99
Microsoft.AspNetCore.Components.ComponentApplicationState.TryTakeAsJson<TValue>(string! key, out TValue? instance) -> bool
1010
Microsoft.AspNetCore.Components.ComponentApplicationState.TryTakePersistedState(string! key, out byte[]? value) -> bool
11-
Microsoft.AspNetCore.Components.IErrorBoundary
12-
Microsoft.AspNetCore.Components.IErrorBoundary.HandleException(System.Exception! exception) -> void
11+
Microsoft.AspNetCore.Components.ErrorBoundaryBase
12+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.Attach(Microsoft.AspNetCore.Components.RenderHandle renderHandle) -> void
13+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.AutoReset.get -> bool
14+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.AutoReset.set -> void
15+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.ChildContent.get -> Microsoft.AspNetCore.Components.RenderFragment?
16+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.ChildContent.set -> void
17+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.ErrorBoundaryBase() -> void
18+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.ErrorContent.get -> Microsoft.AspNetCore.Components.RenderFragment<System.Exception!>?
19+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.ErrorContent.set -> void
20+
Microsoft.AspNetCore.Components.ErrorBoundaryBase.SetParametersAsync(Microsoft.AspNetCore.Components.ParameterView parameters) -> System.Threading.Tasks.Task!
1321
Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime
1422
Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime.ComponentApplicationLifetime(Microsoft.Extensions.Logging.ILogger<Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime!>! logger) -> void
1523
Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime.PersistStateAsync(Microsoft.AspNetCore.Components.Lifetime.IComponentApplicationStateStore! store, Microsoft.AspNetCore.Components.RenderTree.Renderer! renderer) -> System.Threading.Tasks.Task!
@@ -33,6 +41,8 @@ Microsoft.AspNetCore.Components.CascadingTypeParameterAttribute
3341
Microsoft.AspNetCore.Components.CascadingTypeParameterAttribute.CascadingTypeParameterAttribute(string! name) -> void
3442
Microsoft.AspNetCore.Components.CascadingTypeParameterAttribute.Name.get -> string!
3543
Microsoft.AspNetCore.Components.RenderTree.Renderer.GetEventArgsType(ulong eventHandlerId) -> System.Type!
44+
abstract Microsoft.AspNetCore.Components.ErrorBoundaryBase.LogExceptionAsync(System.Exception! exception) -> System.Threading.Tasks.ValueTask
45+
abstract Microsoft.AspNetCore.Components.ErrorBoundaryBase.RenderDefaultErrorContent(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder! builder, System.Exception! exception) -> void
3646
override Microsoft.AspNetCore.Components.LayoutComponentBase.SetParametersAsync(Microsoft.AspNetCore.Components.ParameterView parameters) -> System.Threading.Tasks.Task!
3747
static Microsoft.AspNetCore.Components.ParameterView.FromDictionary(System.Collections.Generic.IDictionary<string!, object?>! parameters) -> Microsoft.AspNetCore.Components.ParameterView
3848
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.RenderTree.EventFieldInfo? fieldInfo, System.EventArgs! eventArgs) -> System.Threading.Tasks.Task!

src/Components/Components/src/RenderTree/Renderer.cs

+8-8
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,10 @@ private bool TrySendExceptionToErrorBoundary(IComponent? errorSource, Exception
327327
// If it becomes problematic, we can maintain a lookup from component instance to ID.
328328
var componentState = _componentStateById.FirstOrDefault(kvp => kvp.Value.Component == errorSource).Value;
329329

330-
// Find the closest IErrorBoundary, if any
330+
// Find the closest error boundary, if any
331331
while (componentState is not null)
332332
{
333-
if (componentState.Component is IErrorBoundary errorBoundary)
333+
if (componentState.Component is ErrorBoundaryBase errorBoundary)
334334
{
335335
// Force the IErrorBoundary component to clear its output, regardless of any logic inside
336336
// that component. This ensures that all descendants are cleaned up. We don't strictly have
@@ -342,17 +342,19 @@ private bool TrySendExceptionToErrorBoundary(IComponent? errorSource, Exception
342342

343343
try
344344
{
345-
// TODO: Should we log the error here? Currently it's up to the IErrorBoundary to do what
346-
// it wants, including hiding the error. Maybe we should force it to get logged regardless
347-
// of what the IErrorBoundary want to do. Whichever way we go on this is permanent, as
348-
// switching in either direction would be a breaking change.
345+
// We don't log the error here, and instead leave it up to the IErrorBoundary to do
346+
// what it wants (which could be suppressing the error entirely). Note that the default
347+
// logging behavior has to vary between hosting models.
348+
// TODO: Are we happy with letting IErrorBoundary suppress errors if it wants?
349349
errorBoundary.HandleException(error);
350350
}
351351
catch (Exception errorBoundaryException)
352352
{
353353
// If *notifying* about an exception fails, it's OK for that to be fatal. This is
354354
// different from an IErrorBoundary component throwing during its own rendering or
355355
// lifecycle methods.
356+
// TODO: Should we support rethrowing from inside HandleException? I prefer not to
357+
// unless we get a better justification. See design doc.
356358
HandleException(errorBoundaryException);
357359
}
358360

@@ -763,8 +765,6 @@ private void RenderInExistingBatch(RenderQueueEntry renderQueueEntry)
763765
}
764766
else
765767
{
766-
// TODO: Should we use disposeComponentState.Component as the owningComponent
767-
// so this is recoverable via IErrorBoundary?
768768
AddToPendingTasks(GetHandledAsynchronousDisposalErrorsTask(result), null);
769769

770770
async Task GetHandledAsynchronousDisposalErrorsTask(Task result)

src/Components/Components/src/Rendering/ComponentState.cs

+2-5
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,9 @@ public void RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment rend
7676
// If an exception occurs in the render fragment delegate, we won't process the diff in any way, so child components,
7777
// event handlers, etc., will all be left untouched as if this component didn't re-render at all. We also are careful
7878
// to leave ComponentState in an internally-consistent state so that technically this component could continue to be
79-
// used.
79+
// used. However, the Renderer will not allow the component to continue to be used, except if it's the IErrorBoundary,
80+
// because it forcibly clears the descendants of the IErrorBoundary before notifying it.
8081
// TODO: Verify that having a try/catch here doesn't degrade perf noticeably on WebAssembly. It might do.
81-
// TODO: Should we actually terminate this component in some way to guarantee it doesn't re-render? That's tricky because
82-
// we need it to be disposed in order that descendants, event handlers, etc. get cleaned up too. Unless we rely on it
83-
// getting removed from its parent, we would have to duplicate quite a bit of internal state-keeping to ensure all the
84-
// descendant state gets cleared up.
8582
renderFragmentException = ex;
8683
return;
8784
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using Microsoft.AspNetCore.Components;
4+
using Microsoft.AspNetCore.Components.Rendering;
5+
6+
namespace BlazorServerApp.Shared
7+
{
8+
// A badly-behaved error boundary that tries to keep using its same ChildContent even after an error
9+
// This is to check we still don't retain the descendant component instances
10+
public class ErrorIgnorer : ErrorBoundaryBase
11+
{
12+
protected override ValueTask LogExceptionAsync(Exception exception)
13+
{
14+
Console.WriteLine($"There was an error, but we'll try to ignore it. La la la la, can't year you. [{exception}]");
15+
return ValueTask.CompletedTask;
16+
}
17+
18+
protected override void RenderDefaultErrorContent(RenderTreeBuilder builder, Exception exception)
19+
{
20+
ChildContent!(builder);
21+
}
22+
}
23+
}

src/Components/Samples/BlazorServerApp/Shared/ErrorIgnorer.razor

-18
This file was deleted.

src/Components/Web/src/PublicAPI.Unshipped.txt

-7
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,7 @@ Microsoft.AspNetCore.Components.Forms.InputTextArea.Element.get -> Microsoft.Asp
1515
Microsoft.AspNetCore.Components.Forms.InputTextArea.Element.set -> void
1616
*REMOVED*static Microsoft.AspNetCore.Components.Forms.BrowserFileExtensions.RequestImageFileAsync(this Microsoft.AspNetCore.Components.Forms.IBrowserFile! browserFile, string! format, int maxWith, int maxHeight) -> System.Threading.Tasks.ValueTask<Microsoft.AspNetCore.Components.Forms.IBrowserFile!>
1717
Microsoft.AspNetCore.Components.Web.ErrorBoundary
18-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.AutoReset.get -> bool
19-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.AutoReset.set -> void
20-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.ChildContent.get -> Microsoft.AspNetCore.Components.RenderFragment?
21-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.ChildContent.set -> void
2218
Microsoft.AspNetCore.Components.Web.ErrorBoundary.ErrorBoundary() -> void
23-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.ErrorContent.get -> Microsoft.AspNetCore.Components.RenderFragment<System.Exception!>?
24-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.ErrorContent.set -> void
25-
Microsoft.AspNetCore.Components.Web.ErrorBoundary.HandleException(System.Exception! exception) -> void
2619
Microsoft.AspNetCore.Components.Web.IErrorBoundaryLogger
2720
Microsoft.AspNetCore.Components.Web.IErrorBoundaryLogger.LogErrorAsync(System.Exception! exception, bool clientOnly) -> System.Threading.Tasks.ValueTask
2821
static Microsoft.AspNetCore.Components.ElementReferenceExtensions.FocusAsync(this Microsoft.AspNetCore.Components.ElementReference elementReference, bool preventScroll) -> System.Threading.Tasks.ValueTask

0 commit comments

Comments
 (0)