ErrorBoundary does not work

garrettlondon1 commented 3 weeks ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

When creating a new Blazor Web App template and using InteractiveServer components without global interactivity.. I cannot get the error boundary to show

In a static component with an interactive island, it does not show Error content.. in a interactive page, it doesnt show error content either..

Confused what I am doing wrong

Expected Behavior

Error boundary shows child content or error content when an unhandled exception happens in a static page or interactive page/component

https://github.com/garrettlondon1/errorboundarytest

Created from Blazor Web app

Steps To Reproduce

Go to the home page, and click the button to increase the count

Exceptions (if any)

warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100] Unhandled exception rendering component: Exception of type 'System.Exception' was thrown. System.Exception: Exception of type 'System.Exception' was thrown. at StaticErrorBoundary.Components.Pages.Counter.IncrementCount() in /Users/garrettlondon/Github/StaticErrorBoundary/Components/Pages/Counter.razor:line 29 at Microsoft.AspNetCore.Components.EventCallbackWorkItem.InvokeAsync[T](MulticastDelegate delegate, T arg) at Microsoft.AspNetCore.Components.ComponentBase.Microsoft.AspNetCore.Components.IHandleEvent.HandleEventAsync(EventCallbackWorkItem callback, Object arg) at Microsoft.AspNetCore.Components.RenderTree.Renderer.DispatchEventAsync(UInt64 eventHandlerId, EventFieldInfo fieldInfo, EventArgs eventArgs, Boolean waitForQuiescence) fail: Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111]

Unhandled exception in circuit 'XOj9HyZGUCrv2y5bZvv5Mswe399H9w8ayWexTB5H--w'. System.Exception: Exception of type 'System.Exception' was thrown. at StaticErrorBoundary.Components.Pages.Counter.IncrementCount() in /Users/garrettlondon/Github/StaticErrorBoundary/Components/Pages/Counter.razor:line 29 at Microsoft.AspNetCore.Components.EventCallbackWorkItem.InvokeAsync[T](MulticastDelegate delegate, T arg) at Microsoft.AspNetCore.Components.ComponentBase.Microsoft.AspNetCore.Components.IHandleEvent.HandleEventAsync(EventCallbackWorkItem callback, Object arg) at Microsoft.AspNetCore.Components.RenderTree.Renderer.DispatchEventAsync(UInt64 eventHandlerId, EventFieldInfo fieldInfo, EventArgs eventArgs, Boolean waitForQuiescence)

.NET Version

8.0.302

Anything else?

No response

MackinnonBuck commented 3 weeks ago

Thanks for reaching out, @garrettlondon1.

It looks like the reason the error boundary doesn't show is because the error boundary is being rendered as a child component of the component throwing the exception. Since exceptions only bubble up the component hierarchy, a child error boundary component won't catch that exception.

This behavior could be changed, but we're going to backlog this for now to collect more feedback.

garrettlondon1 commented 3 weeks ago

It looks like the reason the error boundary doesn't show is because the error boundary is being rendered as a child component of the component throwing the exception.

Thanks @MackinnonBuck, but I'm confused.

Counter.razor code block has the method IncrementCount Counter.razor markup has the onclick handler that calls IncrementCount

This interactivity lives within the same exact component, no other components are throwing an exception. Why would an error boundary not work on the Counter.razor page?

What am I missing here?

garrettlondon1 commented 3 weeks ago

And what is the solution to catch these unhandled exceptions without putting a try catch block in every single point of interactivity (which is crazy at scale)

IevgeniiKunshchykov commented 3 weeks ago

Hello guys,

I've got two examples: 1) This example contains case when error is caught. There are 2 components: 1st rises error and seconds wraps 1st with ErrorBoundary

ComponentWithError.razor

WrappingComponent.razor

2) This example contains case when error is not caugh and unhandled exception happens.

I found the reson why errorboundary does not work when error happens just inside content but not in child component There is such a class .Microsoft.AspNetCore.Components.RenderTree.Renderer.cs which contains HandleExceptionViaErrorBoundary method that recursively tries to find ErrorBoundary to handle exception. But search goes in tree by components.

That's why my first case works. Because WrappingComponent.razor contains ErrorBoundary But second case has no wrapping component or that component just does not have ErrorBoundary . ErrorBoundary is located just inside that component. And that case is not covered by that loop in Renderer.cs

Seems like we are obliged always to wrap components with ErrorBoundary but cannot just wrap some content.

Any reason for that ??? Why whole tree cannot be checked ??

garrettlondon1 commented 3 weeks ago

@MackinnonBuck @danroth27 I see three options that Blazor forces developers into:

Use global interactivity with Error Boundary > not an option, cannot take advantage of SSR and per-page interactivity
Wrap every interactive component inside another page entirely so that Renderer.cs can see ErrorBoundary > terrible design
Use a try catch block to handle exceptions in every single interactive method > not scalable at all

Thank you @IevgeniiKunshchykov

danroth27 commented 3 weeks ago

I believe @IevgeniiKunshchykov's assessment matches are our understanding of the issue. We could make a change to make it possible for error boundaries to handle exceptions thrown by HTML event handlers. This would be a behavior change that could potentially be breaking, but it seems likely that the impact could be negligible. But before we make such a change, we want to make sure we have sufficient justification and user demand for the change. We don't want to risk breaking existing users if the beneficial impact is unclear.

@garrettlondon1 If we were to make this change, are you saying that error boundaries would then meet your error handling needs? Or are you saying in your last comment that you think Blazor needs an entirely different error handling solution?

garrettlondon1 commented 3 weeks ago

@danroth27 thanks for the quick reply..

ErrorBoundary is definitely the proper solution for Interactive blazor error handling

The problem is that you have to wrap every single interactive component in it because App.razor/Router is now static

This is fairly manageable and im fine with it..

All this story is asking for, is to make every interactive action inside of the ErrorBoundary able to bubble up the component tree correctly, and hit the ErrorBoundary properly: not throw an unhandled exception

Without the hack that @IevgeniiKunshchykov did with WrappingComponent

guardrex commented 1 week ago

BTW ... I think we already cover that the error boundary should be around the portion of the component hierarchy that can throw an exception. We don't say anywhere that it can directly wrap content. Mackinnon and I spent some time working on the coverage, and we're taking feedback, but I think what's being discussed here is covered.

https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/handle-errors?view=aspnetcore-8.0#error-boundaries

UPDATE (7/18): After discussion with @TimMurphy and @MackinnonBuck, I made improvements to our coverage to at least help clear up confusion. The updated section is live now at ...

https://learn.microsoft.com/aspnet/core/blazor/fundamentals/handle-errors#error-boundaries

dotnet / aspnetcore