[blazor] fix multiple calls to OnConnectionDownAsync

[pavelsavara]

• don't call CircuitHost.OnConnectionDownAsync() from CircuitHost.DisposeAsync() if it's not connected
• don't call _circuitConnectedCounter.Add from CircuitMetrics.OnCircuitDown
• fix unit test

Fixes #62219

[javiercn]

I don't think this is the right change to fix the metric issue. It's changing an existing behavior of the system that users might be relying on.

[pavelsavara]

this event is called twice. That's a bug not a feature

[ilonatommy]

What is causing the 2nd call?

[pavelsavara]

The proper event is Hub.OnDisconnect()

CircuitHost.OnConnectionDownAsync() is called from CircuitHost.DisposeAsync() regardless if it's connected or not, creating dis-balance.

[pavelsavara]

@javiercn does this make sense or you have another proposal for the fix ?

[javiercn]

I think it makes sense, but I think it might not be potentially right to just check Client.Connected.

I might be wrong, but I suspect there are two paths that we care about.

• Hub.OnDisconnect which I imagine calls client.SetDisconnected and then eventually calls dispose?
• The other one is when we terminate the circuit (client navigates away or closes the tab) gracefully.
  • https://github.com/dotnet/aspnetcore/blob/main/src/Components/Server/src/CircuitDisconnectMiddleware.cs

So, I think that if we check on this, we run the risk of not running the handlers during termination. We might need to mark the host as terminating or something similar to differentiate between the two cases and choose when to run the handlers.

[pavelsavara]

I just tested the "close tab" and "navigate to non-interactive page". Both behave as expected, that is, they call the event once.

[javiercn]

Terminate it's not being fired in those cases.

• Close tab will likely don't cause unload to fire.
• navigate to non-interactive page
  • It takes a bit of time for the client to start the circuit disposal.
  • If for some reason it's not happening, it's also a bug.

https://github.com/dotnet/aspnetcore/blob/main/src/Components/Server/src/CircuitDisconnectMiddleware.cs#L81-L87

https://github.com/dotnet/aspnetcore/blob/main/src/Components/Server/src/Circuits/CircuitRegistry.cs#L391

If we check for the circuit to be connected it won't fire when we want to eagerly terminate the circuit.

[pavelsavara]

Maybe we need to call that event from places that call circuitHost.Client.SetDisconnected() rather than relying that circuitHost.DisposeAsync() would always call it.

[pavelsavara]

Also I'm not sure that the new PauseAndDisposeCircuitHost is calling SetDisconnected()