search

Azure / azure-signalr

Azure SignalR Service SDK for .NET
https://aka.ms/signalr-service
MIT License
427 stars 101 forks source link

AddToGroupAsync throws a TimeoutException exception when MapHub is called twice for the same hub #2003

Closed sasha-khadasevich closed 3 months ago

sasha-khadasevich commented 4 months ago

Describe the bug

Groups.AddToGroupAsync() throws a TimeoutException approximately half the time when MapHub is called twice for the same hub. This issue is caused by changes introduced in #1779. It can be reproduced with Microsoft.Azure.SignalR version 1.21.3 or later works fine with version 1.21.2.

I'm unsure if calling MapHub twice for the same hub is allowed. If it isn't, this behavior should be documented.

To Reproduce

Update the ChatRoom sample project as follows:

Program.cs

app.MapHub<ChatSampleHub>("chat"); 
app.MapHub<ChatSampleHub>("chat/v2");

ChatSampleHub.OnConnectedAsync await Groups.AddToGroupAsync(Context.ConnectionId, $"{Guid.NewGuid()}");

Exceptions (if any)

fail: Microsoft.AspNetCore.SignalR.HubConnectionHandler[1] Error when dispatching 'OnConnectedAsync' on hub. System.TimeoutException: Ack-able message Microsoft.Azure.SignalR.Protocol.JoinGroupWithAckMessage(ackId: 1) timed out. at Microsoft.Azure.SignalR.AckHandler.HandleAckStatus(IAckableMessage message, AckStatus status) at Microsoft.Azure.SignalR.ServiceConnectionContainerBase.WriteAckableMessageAsync(ServiceMessage serviceMessage, CancellationToken cancellationToken) at Microsoft.Azure.SignalR.MultiEndpointMessageWriter.<>c__DisplayClass10_0.<b__0>d.MoveNext()

Further technical details

### Tasks
vicancy commented 3 months ago

Thanks for reporting the issue, yes I can consistently repro the issue, please allow me to take a further look.

vicancy commented 3 months ago

To double confirm, the behavior, as the same as self-host SignalR, is that although they map to different endpoints chat and chat/v2, these clients are actually sharing the same Hub, a sendToAll message send to all these clients despite they connect to different endpoint.

Curious about the usage, is it for rolling update the client logic?

sasha-khadasevich commented 3 months ago

In our project, we used something like chat/v1 and chat/v1.0 to follow different conventions for clients. We faced issues after updating from version 1.21.2 to the latest one and spent a lot of time figuring out what was wrong (cause error message and stack trace are not precise). Right now, we definitely don't need to register the hub for two URLs. We removed it and are happy with the update, but I believe it would be nice to fix or document this limitation, as someone else might face the same issue in the future.

vicancy commented 3 months ago

Thanks for sharing Sasha, the next release will fix the issue.