search

radzenhq / radzen-blazor

Radzen Blazor is a set of 90+ free native Blazor UI components packed with DataGrid, Scheduler, Charts and robust theming including Material design and FluentUI.
https://www.radzen.com
MIT License
3.52k stars 785 forks source link

JSDisconnectedException in RadzenDataGrid Dispose #373

Closed dxrdxr closed 2 years ago

dxrdxr commented 2 years ago

Describe the bug I added a NotificationService.Notify to my server side Blazor app and it is now repeatedly throwing JSDisconnectedException when I refresh the page.

The call stack:

Microsoft.AspNetCore.Components.Server.dll!Microsoft.AspNetCore.Components.Server.Circuits.RemoteJSRuntime.BeginInvokeJS(long asyncHandle, string identifier, string argsJson, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) Line 116
    at /_/src/Components/Server/src/Circuits/RemoteJSRuntime.cs(116)
Microsoft.JSInterop.dll!Microsoft.JSInterop.JSRuntime.InvokeAsync<Microsoft.JSInterop.Infrastructure.IJSVoidResult>(long targetInstanceId, string identifier, System.Threading.CancellationToken cancellationToken, object[] args) Line 132
    at /_/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs(132)
Microsoft.JSInterop.dll!Microsoft.JSInterop.JSRuntime.InvokeAsync<Microsoft.JSInterop.Infrastructure.IJSVoidResult>(long targetInstanceId, string identifier, object[] args) Line 91
    at /_/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs(91)
Microsoft.JSInterop.dll!Microsoft.JSInterop.JSRuntime.InvokeAsync<Microsoft.JSInterop.Infrastructure.IJSVoidResult>(string identifier, object[] args) Line 69
    at /_/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs(69)
Microsoft.JSInterop.dll!Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(Microsoft.JSInterop.IJSRuntime jsRuntime, string identifier, object[] args) Line 32
    at /_/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeExtensions.cs(32)
[Resuming Async Method]
System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.JSInterop.JSRuntimeExtensions.<InvokeVoidAsync>d__0>(ref Microsoft.JSInterop.JSRuntimeExtensions.<InvokeVoidAsync>d__0 stateMachine) Line 38
    at /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs(38)
Radzen.Blazor.dll!Radzen.Blazor.RadzenDataGrid<DiveShop.ViewModels.CertificationVM>.Dispose()
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.Rendering.ComponentState.Dispose() Line 252
    at /_/src/Components/Components/src/Rendering/ComponentState.cs(252)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose(bool disposing) Line 1030
    at /_/src/Components/Components/src/RenderTree/Renderer.cs(1030)
Microsoft.AspNetCore.Components.Web.dll!Microsoft.AspNetCore.Components.RenderTree.WebRenderer.Dispose(bool disposing) Line 87
    at /_/src/Components/Web/src/WebRenderer.cs(87)
Microsoft.AspNetCore.Components.Server.dll!Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer.Dispose(bool disposing) Line 121
    at /_/src/Components/Server/src/Circuits/RemoteRenderer.cs(121)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose() Line 1089
    at /_/src/Components/Components/src/RenderTree/Renderer.cs(1089)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContextDispatcher.InvokeAsync(System.Action workItem) Line 30
    at /_/src/Components/Components/src/Rendering/RendererSynchronizationContextDispatcher.cs(30)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.RenderTree.Renderer.DisposeAsync() Line 1105
    at /_/src/Components/Components/src/RenderTree/Renderer.cs(1105)
Microsoft.AspNetCore.Components.Server.dll!Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost.DisposeAsync.AnonymousMethod__36_0() Line 188
    at /_/src/Components/Server/src/Circuits/CircuitHost.cs(188)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext.InvokeAsync.AnonymousMethod__9_0(object state) Line 74
    at /_/src/Components/Components/src/Rendering/RendererSynchronizationContext.cs(74)
Microsoft.AspNetCore.Components.dll!Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext.ExecuteSynchronously(System.Threading.Tasks.TaskCompletionSource<object> completion, System.Threading.SendOrPostCallback d, object state) Line 237
[...] uninteresting stacks elided

Environment

Related issues See the following Blazor issues:

Comments Based on my reading of the related issues, there is a change in behavior in .Net 6 where calling JSRuntime inside a Dispose method can trigger this exception as the runtime may have disconnected from its circuit. This exception does not exist in .Net 5 so it is likely not caught and handled in the current Radzen library.

Looking at RadzenDataGrid.razor.cs#L1948 this is the likely code that needs to handle this exception.

In looking at the RemoteJSRuntime.cs code where the exception is thrown, one possible way to help avoid this is to check RemoteJSRuntime.IsInitialized which checks the validity of _clientProxy which must not be null. The JSDisconnectedException can only be thrown if _clientProxy is null, therefore if IsInitialized is true then it is safe to call JSRuntime.InvokeVoidAsync.

There may be other places where this same pattern occurs.

enchev commented 2 years ago

I'm unable to reproduce such exception on our demos: https://blazor.radzen.com/notification

dxrdxr commented 2 years ago

It appears that pointing a finger at Notifications was a red herring. I removed notifications and it still throws.

I have distilled my repo down to a very minimal test case that only involves a DataGrid and I can consistently see the exceptions being throw when the Page is refreshed.

See this test repro.

enchev commented 2 years ago

I'm afraid that we cannot do anything else to prevent this. There is already check if the JSRuntime is available. And the StateHasChanged is in try/catch block.

dxrdxr commented 2 years ago

A little more debugging on this.

The code is failing in the RadzenDataGrid.Dispose method, the critical code is where it checks IsJSRuntimeAvailable to be true, if so then in calls JSRuntime.InvokeVoidAsync which throws JSDisconnectedException.

If you look at JSRuntime.IsInitialized it is false when the Invoke is called, so it is not unexpected that this fails.

Tracing through my testcase for when IsJSRuntimeAvailable is set and when it is get, you can observe that on initial access to the page it is set to true in OnAfterRenderAsync and a few more times when it is already true. In each case JSRuntime.IsInitialized is true.

The I hit Refresh (^R) I see a call to RadzenDataGrid.Dispose with a different UniqueID than the previous cases and IsJSRuntimeAvailable is false and JSRuntime.IsInitialized is false .

There is then another call to RadzenDataGrid.Dispose with the same UniqueID as the initial access, now IsJSRuntimeAvailable is still true but JSRuntime.IsInitialized is false ! The exception is thus thrown.

My working theory is that when the browser does a Refresh of the page, the JSRuntime is reset (new instance?). But all of the existing RadzenComponents have not had their Dispose methods called. This appears to fit with the discussions in the Issues linked above, that the Dispose method may take up to 60 seconds to be called in some cases.

I don't understand the history of the RadzenComponent code and why it is independently tracking the state of the JSRuntime, and notably that it never tracks the case where the runtime goes away (never set to false). So this appears to be a bad assumption that the Dispose method will be called before the JSRuntime goes away, and with .Net 6.0 things have changed where it does go away first (race?).

Without knowing the internal details, I would opine that the property be removed and the checks such as this:

if (IsJSRuntimeAvailable)
{
...
}

Be replaced by:

if (JSRuntime?.IsInitialized)
{
...
}

However, IsInitialized may be Public but it is not documented, so it may not be stable to rely on. The only documented way to know if the runtime is initialized is if OnAfterRenderAsync has been called, but I don't see how to detect if it has been destroyed.

An alternative approach that might work is to grab a reference to a IJSObjectReference in OnAfterRenderAsync and hold on to it, then in the RadzenComponent.Dispose actually dispose of the reference. This will guarantee that it still exists, but it may still not be in a usable state.

Or maybe the only stable thing to do is to simply catch JSDisconnectedException and swallow it? But note that this can only be done in a .Net 6.0 version of Radzen!