AspNetCore.Docs icon indicating copy to clipboard operation
AspNetCore.Docs copied to clipboard

Misleading and broken code example for "Manually reconnect" after SignalR connection closed

Open oskarb opened this issue 2 years ago • 7 comments

Description

The section "Manually reconnect" contains this example:

connection.Closed += async (error) =>
{
    await Task.Delay(new Random().Next(0,5) * 1000);
    await connection.StartAsync();
};

However, I could not get this to work and I've spent an annoying amount of time double-checking I had followed the various steps in the setup.

As far as I can tell, this example code is broken and not reliable, because if the network or server is not back and accepting connections within a second or 2, the call to StartAsync() will throw an exception, the connection attempt will be abandoned, and there will be no new call to the Closed handler, so no additional attempts will be made:

System.Net.Http.HttpRequestException: Connection refused (localhost:7321)
 ---> System.Net.Sockets.SocketException (111): Connection refused
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 token)
   at System.Net.Sockets.Socket.<ConnectAsync>g__WaitForConnectWithCancellation|277_0(AwaitableSocketAsyncEventArgs saea, ValueTask connectTask, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String host, Int32 port, HttpRequestMessage initialRequest, Boolean async, CancellationToken cancellationToken)

The code example does not show this and the surrounding text doesn't even hint about it. Quite the opposite, the current text in that section gives a clear impression that it is all you need to do.

I think the section should at least point this out.

For what it's worth, it seems calling the ConnectWithRetryAsync() method, provided earlier in the article, from the Closed event handler instead of directly calling StartAsync() makes it more robust.

Page URL

https://learn.microsoft.com/en-us/aspnet/core/signalr/dotnet-client?view=aspnetcore-8.0&tabs=visual-studio

Content source URL

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/signalr/dotnet-client.md

Document ID

36052173-7061-6d01-8aed-915685cd7cf6

Article author

@bradygaster

oskarb avatar Feb 01 '24 20:02 oskarb

@oskarb, sorry to hear about your experience with this doc. I'll take a look at it. I could also be very out of date. Thanks for relating the issue and for going the extra mile of making suggestions for corrections as well. Very appreciated.

wadepickett avatar Feb 01 '24 21:02 wadepickett

@BrennanConroy, should I change the client code example to use in combination with the ConnectWithRetryAsync() example so start failures are handled, instead of StartAsync()? It is not clear to me if that is too much for the example or if adjusting the delay setting before reconnecting is the first place to start.

wadepickett avatar Feb 01 '24 22:02 wadepickett

Calling ConnectWithRetryAsync is fine, the manual reconnect section is kind of old, and was written before WithAutomaticReconnect existed.

BrennanConroy avatar Feb 01 '24 22:02 BrennanConroy

Thanks! One more question: ConnectWithRetryAsync(HubConnection connection, CancellationToken token) takes a CancellationToken. Where exactly in the entire process is the CancellationToken expected to have been created in this case?

wadepickett avatar Feb 01 '24 23:02 wadepickett

Probably when they call the method

BrennanConroy avatar Feb 01 '24 23:02 BrennanConroy

My case is a service process that should use SignalR to talk to another service process. So there is no UI or UI-thread involved.

Maybe I missed something about the automatic reconnect feature.... in all of this I found various articles online that seem to say that if the server did an intentional and graceful shutdown, then the client will not automatically reconnect. And I got the impression that I would then anyway need to handle it in the Closed event. But perhaps that is not true, I so far did not try it.

Currently I'm using something similar to the code below. The real version of this does seem to work and reconnect every time I stop the server temporarily, but I'm not sure I have fully grasped the finer points regarding the cancellation tokens and Task usage yet (in general).



public class MyService : IHostedService
{
    private HubConnection connection;
    private CancellationTokenSource stopSignalRToken = new();

    public Task StartAsync(CancellationToken _)
    {
        connection = new HubConnectionBuilder()
                            .WithUrl("http://localhost:53353/ChatHub")
                            .Build();

        // Awaiting will hold here until the first successful connection.
        // Modules expecting to use the connection must anyway be prepared
        // for the connection to be down at any time, so we discard instead,
        // to proceed with other startup tasks.
        _ = ActivateContinuousConnection(connection, stopSignalRToken.Token)
    
        // ... other startup work
        
        return Task.CompletedTask;
    }
    
    public async Task StopAsync(CancellationToken cancellationToken)
    {
        // ... shut down various modules
        
        // Close connection and stop reconnection attempts.
        stopSignalRToken.Cancel();
        await connection.StopAsync();
    }

    public static async Task<bool> ActivateContinuousConnection(HubConnection connection, CancellationToken token)
    {
        connection.Closed += async (error) =>
        {
            await Task.Delay(new Random().Next(0,5) * 1000);
            await ConnectWithRetryAsync(connection, token);
        };
    
        return await ConnectWithRetryAsync(connection, token);
    }


    public static async Task<bool> ConnectWithRetryAsync(HubConnection connection, CancellationToken token)
    {
        // Keep trying to until we can start or the token is canceled.
        while (true)
        {
            try
            {
                await connection.StartAsync(token);
                Debug.Assert(connection.State == HubConnectionState.Connected);
                return true;
            }
            catch when (token.IsCancellationRequested)
            {
                return false;
            }
            catch
            {
                // Failed to connect, trying again in 5000 ms.
                Debug.Assert(connection.State == HubConnectionState.Disconnected);
                await Task.Delay(5000);
            }
        }
    }
}

oskarb avatar Feb 02 '24 09:02 oskarb

Probably when they call the method

The client JavaScript would not create and pass a CancellationToken, so I was confused about where it was coming from, unless it was expected the developer would add a controller on the server side that created a CancellationToken and then passed that on to the Hub method that required one.

Here is what I think I was missing: Are there condidtions under which the signalR framework automatically creates a CancellationToken and passes it in the case that the Hub method requires it as a parameter? So the JavaScript client does not send a CancellationToken, and yet the Hub method gets one automatically supplied by the framework if it requires one as a parameter?

wadepickett avatar Feb 02 '24 21:02 wadepickett