dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.37k stars 9.99k forks source link

Change IJSObjectReference.Dispose specifically so that it deals with catching and discarding JSDisconnectedException internally #49418

Open danroth27 opened 1 year ago

danroth27 commented 1 year ago

I'm getting a warning and exception when trying to dispose a JavaScript module reference in a component's DisposeAsync method when the component is using the server interactive render mode.

Repro steps:

Expected result: No exception Actual result: JSDisconnectedException

warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100]
      Unhandled exception rendering component: JavaScript interop calls cannot be issued at this time. This is because the circuit has disconnected and is being disposed.
      Microsoft.JSInterop.JSDisconnectedException: JavaScript interop calls cannot be issued at this time. This is because the circuit has disconnected and is being disposed.
         at Microsoft.AspNetCore.Components.Server.Circuits.RemoteJSRuntime.BeginInvokeJS(Int64 asyncHandle, String identifier, String argsJson, JSCallResultType resultType, Int64 targetInstanceId)
         at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, CancellationToken cancellationToken, Object[] args)
         at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, Object[] args)
         at Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(IJSRuntime jsRuntime, String identifier, Object[] args)
         at Microsoft.JSInterop.Implementation.JSObjectReference.DisposeAsync()
         at BestForYouRecipes.IngredientsListEditor.DisposeAsync() in C:\Users\daroth\Documents\GitHub\danroth27\BestForYouRecipes\BestForYouRecipes.Client\IngredientsListEditor.razor:line 136
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass83_0.<<Dispose>g__HandleAsyncExceptions|1>d.MoveNext()

fail: Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111]
      Unhandled exception in circuit 'yAE5LTi2-XXk6e9JbmT_P1RmxksmTriTYTGkrzPgH2I'.
      Microsoft.JSInterop.JSDisconnectedException: JavaScript interop calls cannot be issued at this time. This is because the circuit has disconnected and is being disposed.
         at Microsoft.AspNetCore.Components.Server.Circuits.RemoteJSRuntime.BeginInvokeJS(Int64 asyncHandle, String identifier, String argsJson, JSCallResultType resultType, Int64 targetInstanceId)
         at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, CancellationToken cancellationToken, Object[] args)
         at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, Object[] args)
         at Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(IJSRuntime jsRuntime, String identifier, Object[] args)
         at Microsoft.JSInterop.Implementation.JSObjectReference.DisposeAsync()
         at BestForYouRecipes.IngredientsListEditor.DisposeAsync() in C:\Users\daroth\Documents\GitHub\danroth27\BestForYouRecipes\BestForYouRecipes.Client\IngredientsListEditor.razor:line 136
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass83_0.<<Dispose>g__HandleAsyncExceptions|1>d.MoveNext()
SteveSandersonMS commented 1 year ago

Expected result: No exception Actual result: JSDisconnectedException

Why do you expect no exception? The component is trying to do a JS interop call after the browser connection has been closed, so it should throw this exception. You do need to catch it. This isn't specific to .NET 8 - the same thing would happen on .NET 7 if a user closes their browser tab or navigates to an external site.

I see that the usability here is quite poor though. One thing we could do is change IJSObjectReference.Dispose specifically so that it deals with catching and discarding JSDisconnectedException internally. It would be safe to do since if JS is disconnected, then there's no reason why the server should be concerned about disposing IJSObjectReference instances.

danroth27 commented 1 year ago

The component is trying to do a JS interop call after the browser connection has been closed

I didn't close the browser or any tab. I just navigated back to the home page.

SteveSandersonMS commented 1 year ago

I didn't close the browser or any tab. I just navigated back to the home page.

It's a full-page load, so it is the same as closing the browser tab. Even if that were changed (which it will be in the next preview), the user could close their browser tab, so this exception still has to be dealt with.

danroth27 commented 1 year ago

One thing we could do is change IJSObjectReference.Dispose specifically so that it deals with catching and discarding JSDisconnectedException internally. It would be safe to do since if JS is disconnected, then there's no reason why the server should be concerned about disposing IJSObjectReference instances.

Sounds reasonable.

danroth27 commented 1 year ago

@SteveSandersonMS In the meantime, I added the exception handling code as suggested: https://github.com/danroth27/BestForYouRecipes/commit/8a54651e8d57337ed8e76576dd18a3efd962e215

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 10 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.