Blazor `CircuitHandler` (Client-Side JS) not Triggering `onCircuitClosed` in Case of Server-Side Exceptions

AeonSake commented 8 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

When the server runs into an unhandled exception, the client's circuit is forcefully closed by the client via a message sent to the client. However, the onCircuitClosed function of the CircuitHandler handles attached with the Blazor.start() options is not triggered in this case. From what I can tell, there is no actual way to get informed that the circuit was forcefully closed in this way via code (other than maybe observing style changes to the error UI element).

Cause

Unhandled server-side exceptions are handled by notifying the client via a custom event (JS.Error) which then closes the connection from the client side.

Event is handled here: https://github.com/dotnet/aspnetcore/blob/63c8031b6a6af5009b3c5bb4291fcc4c32b06b10/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts#L166-L170

Which calls unhandledError(): https://github.com/dotnet/aspnetcore/blob/63c8031b6a6af5009b3c5bb4291fcc4c32b06b10/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts#L292-L298

Expected Behavior

The onCircuitClosed function is invoked, informing all handles that the circuit was closed. Ideally I would even like to have the close reason as argument (enum? or the error object/message) so the client can react to these cases (e.g., by reloading the page). The CircuitHandler interface could also be extended with something like an onCircuitFaulted function.

Steps To Reproduce

Server Setup

TestPage.razor:

@page "/test"
@implements IDisposable

<a href="/">Home</a>

@code {
    public void Dispose() => throw new Exception();
}

_Host.cshtml:

<!-- ... -->
<body>
    <!-- ... -->

    <script src="_framework/blazor.server.js" autostart="false"></script>
    <script>
        Blazor.start({
            circuitHandlers: [{
                 onCircuitOpened: () => console.log('circuit opened'),
                 onCircuitClosed: () => console.log('circuit closed')
            }]
        });
    </script>
</body>
</html>

Procedure

Navigate to /test and then press the Home link.

Result

The onCircuitClosed function is never invoked and the console message is not printed. The Page gets stuck in its last render state before the circuit was closed. However, the error UI element with the ID blazor-error-ui will receive the appropriate style changes (style set to display:block).

Notes

The component does not have to be a page (just for demonstration), also works with normal components that are disposed at some point. Also works with Exceptions thrown inside IAsyncDisposable implementations. In theory, all unhandled exceptions will cause this issue. The reason for why I selected the dispose pattern as source for the exception is because not even an ErrorBoundary component will catch those exceptions and because I had some occurrences where async disposing of JS module references failed and caused the circuit to fault. While I understand that the later can be circumvented via a try-catch block, I still need a way to automatically reloading the page in case of errors since in my case, the client is in a stand-alone kiosk-mode and I cannot rely on a user clicking the reload button in the error UI.

Exceptions (if any)

No response

.NET Version

8.0.202

Anything else?

No response

javiercn commented 8 months ago

@AeonSake thanks for contacting us.

I think we would be ok to call onCircuitClosed in that scenario. Its currently closed only when the server renderer gets disposed. I think we would be ok calling the handlers on this type of error.

If you are interested in sending a PR to add support for it, I think we would need to refactor the code from disposeCore and call it at that point.

I don't think we want to extend this interface further unless we see other scenarios where its useful. We don't typically want to disclose these types of errors to the client as they might contain sensitive information and are already logged into the server logs.

dotnet-policy-service[bot] commented 8 months ago

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

garrettlondon1 commented 5 months ago

This will be very helpful @AeonSake thank you

dotnet / aspnetcore