Aspnetcore: Messages sent on a non-Blazor SignalR Hub can't be serialized when Blazor is enabled

[anurse]

Description

When SignalR is configured with a scale-out provider, either Redis or Azure SignalR, it has to pre-serialize every message in every hub protocol to cover cases where there are clients connected to other servers using different protocols. We also have to ensure all the user's custom serialization logic (Json converters, etc.) runs during serialization, which is why we serialize for all protocols before sending any messages.

This causes a problem when a standard SignalR Hub co-exists with Server-Side Blazor. Server-Side Blazor registers a custom Hub Protocol: BlazorPackHubProtocol which can only serialize a limited set of types (the ones Blazor uses). If the user attempts to send a message containing custom objects to clients via the standard SignalR Hub, they get the following error:

System.FormatException: Unsupported argument type BlazorApp1.MyMessage
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.SerializeArgument(MessagePackWriter& writer, Object argument)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.WriteInvocationMessage(InvocationMessage message, MessagePackWriter& writer)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.WriteMessageCore(HubMessage message, IBufferWriter`1 bufferWriter)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.GetMessageBytes(HubMessage message)
   at Microsoft.AspNetCore.SignalR.SerializedHubMessage.GetSerializedMessage(IHubProtocol protocol)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteSerializedHubMessage(Stream stream, SerializedHubMessage message)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteInvocation(String methodName, Object[] args, IReadOnlyList`1 excludedConnectionIds)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteInvocation(String methodName, Object[] args)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager`1.SendAllAsync(String methodName, Object[] args, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.SignalR.Internal.AllClientProxy`1.SendCoreAsync(String method, Object[] args, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.SignalR.ClientProxyExtensions.SendAsync(IClientProxy clientProxy, String method, Object arg1, CancellationToken cancellationToken)
   at BlazorApp1.Pages.Index.Button_Click(MouseEventArgs e) in C:\Users\anurse\source\repos\BlazorApp1\BlazorApp1\Pages\Index.razor:line 24

Note the RedisHubLifetimeManager on the call stack. The same would be true for Azure SignalR as their hub lifetime manager does the same thing.

The core problem here is that this pre-serialization occurs for all protocols registered in DI regardless of what is enabled on the Hub. However, even if we filtered the list of protocols by what's enabled, the other issue is that BlazorPack is enabled by default for all hubs unless explicitly removed (because all protocols are enabled unless explicitly removed).

Repro Steps

Repro App: https://github.com/anurse/BlazorRedisSerializationRepro

This can be reproduced using either Redis scale-out for SignalR or Azure SignalR

Redis Repro Steps:

1. Start redis via docker docker run -it --rm -p 6379:6379 redis
2. Launch the app
3. Click "Send a message"

Azure SignalR Repro Steps:

1. Change the app to use Azure SignalR (remove AddStackExchangeRedis from Startup.ConfigureServices, add in Azure SignalR config)
2. Launch the app
3. Click "Send a message"

Expected Behavior

No error is reported

Actual Behavior

The following error is reported:

System.FormatException: Unsupported argument type BlazorApp1.MyMessage
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.SerializeArgument(MessagePackWriter& writer, Object argument)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.WriteInvocationMessage(InvocationMessage message, MessagePackWriter& writer)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.WriteMessageCore(HubMessage message, IBufferWriter`1 bufferWriter)
   at Microsoft.AspNetCore.Components.Server.BlazorPack.BlazorPackHubProtocol.GetMessageBytes(HubMessage message)
   at Microsoft.AspNetCore.SignalR.SerializedHubMessage.GetSerializedMessage(IHubProtocol protocol)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteSerializedHubMessage(Stream stream, SerializedHubMessage message)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteInvocation(String methodName, Object[] args, IReadOnlyList`1 excludedConnectionIds)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol.WriteInvocation(String methodName, Object[] args)
   at Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager`1.SendAllAsync(String methodName, Object[] args, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.SignalR.Internal.AllClientProxy`1.SendCoreAsync(String method, Object[] args, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.SignalR.ClientProxyExtensions.SendAsync(IClientProxy clientProxy, String method, Object arg1, CancellationToken cancellationToken)
   at BlazorApp1.Pages.Index.Button_Click(MouseEventArgs e) in C:\Users\anurse\source\repos\BlazorApp1\BlazorApp1\Pages\Index.razor:line 24

cc @davidfowl @rynowak @BrennanConroy @halter73 @bradygaster

[anurse]

Proposed Fix

1. Add a new option to HubOptions and HubOptions<T>:

public class HubOptions
{
    // Name to be bikeshed
    public IList<IHubProtocol> AdditionalHubProtocols { get; }
}

1. Change Blazor to add BlazorPackHubProtocol to this list instead of registering it in DI and putting it in the SupportedProtocols list (which is a list of names). Blazor would still empty out the SupportedProtocols list for it's hub options.

2. Add a new public interface: IHubMessageSerializer<THub>:

public interface IHubMessageSerializer<THub>
{
    // Takes a "HubMessage" and pre-serializes it into all the protocols supported
    // by this Hub. Returns an instance of the (existing) SerializedHubMessage carrier
    // type which is already designed to hold these "pre-serialized" messages.
    SerializedHubMessage SerializeMessage(HubMessage message);
}

1. Make SerializedHubMessage.GetAllSerializations public (and probably change to IEnumerable instead of IReadOnlyList for perf). It returns a list of SerializedMessage instances, which are basically just (ProtocolName, SerializedData) tuples.

2. Add an implementation of IHubMessageSerializer<THub> to DI that does the "right thing":

   • Grab the global list of IHubProtocol instances
   • Resolve all the items in the HubOptions.SupportedProtocols list from names to IHubProtocol instances
   • Union with HubOptions.AdditionalHubProtocols
   • Implements SerializeMessage by iterating over the resulting list of protocols and serializing them.

3. Change all existing lifetime managers to use this type to pre-serialize messages instead of using custom logic. When calling IHubMessageSerializer<THub>.SerializeMessage, the returned SerializedHubMessage will effectively expose an IDictionary<string, ReadOnlyMemory<byte>> (which is what both Redis and Azure SignalR need to send their payload up to the backplane).

Another option is to change all the lifetime managers to properly select appropriate protocols (i.e. do what item 5 above does), but having SignalR itself provide a service to do this means all lifetime managers use the same logic and the individual providers (Redis, Azure SignalR, Orleans, etc.) don't have to.

cc @chenkennt @vicancy for Azure SignalR impact. This will be something the Azure SignalR SDK will need to update to support when we have an updated version of the SignalR server.

[IEvangelist]

Thanks for digging into this earlier today @anurse and @bradygaster, much appreciated. I still wish we would have used live share though - need to find an excuse to use that 🤘

[vicancy]

Thanks @anurse . The proposal sounds good to me.

[BrennanConroy]

We've decided on a more tactical change in 3.1. We're adding an internal attribute Microsoft.AspNetCore.SignalR.Internal.NonDefaultHubProtocolAttribute that will be placed on BlazorPackHubProtocol. And when populating the "SupportedProtocols" list in HubOptions we check for the attribute by name + namespace and exclude it.

We're also making the RedisHubLifetimeManager look at the "SupportedProtocols" list and only serializing the HubMessage using the protocols from that list.

[anurse]

We're also making the RedisHubLifetimeManager look at the "SupportedProtocols" list and only serializing the HubMessage using the protocols from that list.

FYI: This is the change we'll need to port to the Azure SignalR SDK. @BrennanConroy will also be working on a plan for that and we'll work with @vicancy and @chenkennt to get that in. (https://github.com/Azure/azure-signalr/issues/693)

[BrennanConroy]

Fixed via https://github.com/aspnet/AspNetCore/pull/15177