search

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
34.93k stars 9.87k forks source link

Stateful reconnect doesn't work as expected when using Redis backplane #55575

Open ruben-ivre opened 2 months ago

ruben-ivre commented 2 months ago

Is there an existing issue for this?

Describe the bug

Stateful reconnect requires that both client and server calculate the same sequence Id so when the reconnect happen they can resume on the right message, but when using Redis as backplane a different code path is used, which ends up causing some messages not being accounted for in the server side and creating a discrepancy that leads to lost messages on reconnect. The whole sequence Id calculation happens in MessageBuffer class, which increases the Id every time an invocation message is sent (in WriteAsyncCore), skipping the count otherwise.

The problem is that when using Redis, the message comes as a SerializedHubMessage using a constructor that doesn't set the Message property, so when MessageBuffer tries to determine if it's an invocation message, is operator returns false as the message is null, and thus this message doesn't increment the sequence Id. On the client side is read correctly, identified as an Invocation, and increments the sequence Id, create a discrepancy that will essentially break stateful reconnect.

Expected Behavior

Sequence Id is kept in sync in both client side and server side. This is critical for stateful reconnect to work properly.

Steps To Reproduce

  1. Create a new SignalR core project, with Redis backplane.
  2. Enable stateful reconnect on both client and server.
  3. Send a message to all users (or a group), so it's sent through the backplane.
  4. Observe how _totalMessageCount in MessageBuffer in server side is not incremented with SerializedHubMessage coming from Redis backplane.

Exceptions (if any)

No response

.NET Version

8.0.3

Anything else?

The issue is pretty easy to spot once you know where to look.

Observe here how MessageBuffer.cs:124 expects Message property to be not null. image

Observe here the constructor in SerializedHubMessage.cs:27 that is used by Redis backplane. No assignment of Message property. image

Observe here how MessageBuffer.cs:161 checks message using is operator that will yield false as it is null, not incrementing the sequence Id as it should have. image

ruben-ivre commented 2 months ago

So far I have a workaround it that is to change that WriteAsync function to create an empty invocation message if hubMessage.Message is null, which works for my use case, but it's not a very good solution. image

Probably a better option would be to add some info to SerializedHubMessage to be able to reconstruct the proper message type. If you can point me to a proper solution, I can work on that.