aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

[SignalR] Support union in the js client

Open sharok opened this issue 6 years ago • 11 comments

MessagePack for C# supports Union type and I can use it for interfaces. For example, I have the following model that I send to js client:

[MessagePackObject]
public class Prices
{
        [Key("last")]
        public decimal Last { get; set; }

        [Key("sell")]
        public decimal Sell { get; set; }

        [Key("buy")]
        public decimal Buy { get; set; }

        [Key("product")]
        public IProductModel Product { get; set; }
}

[MessagePack.Union(0, typeof(ProductEx0))]
[MessagePack.Union(1, typeof(ProductEx1))]
public interface IProductModel 
{
   string Name{ get; set; }
}

I can create different IProductModel and send to the client. I handle creating the corresponding objects manually on the client and all works fine. But, it does't work when I want to deserialize message to a class that contain union. I have the following class on the server side:

public RegisterProduct 
{
   [Key("product")]
   public IProductModel Product { get; set; }
}

I send the following object from the client:

{
      product: new ProductEx0()     
}

MessagePack on server side throws error:

System.InvalidOperationException: code is invalid. code:129 format:fixmap

I understand that Union types are encoded in different way, but from the client I send simple model. therefore messagepack cannot handle it.

sharok avatar Feb 06 '19 05:02 sharok

cc @muratg to come up with a repro and potential path forward. assigning to @BrennanConroy to come up with a test case so we can investigate. thanks for the detailed repro above!

bradygaster avatar Feb 06 '19 19:02 bradygaster

Until this is resolved, I'd highly recommend adding mention of this in the SignalR documentation. Right now it is recommended to use MessagePack for serialisation of messages, but with limitations like this (which you can hit late into development) not listed from the outset it's a bit of a frustrating situation.

Also a bit weird that this is titled as a javascript client issue, but also applies to the c# client.

peppy avatar Aug 02 '21 04:08 peppy

Will there ever be a fix for it?

@peppy I heard you have some kind of workaround?

Sonorpearl avatar Mar 04 '24 11:03 Sonorpearl

Mentioning this issue, so it's also linked: https://github.com/MessagePack-CSharp/MessagePack-CSharp/issues/1171

Sonorpearl avatar Mar 04 '24 11:03 Sonorpearl

Also throwing it in, but it seems like MemoryPack (MessagePack Alternative), has solved this issue: https://github.com/Cysharp/MemoryPack/issues/146 Haven't done an investigation yet.

Sonorpearl avatar Mar 04 '24 11:03 Sonorpearl

The standard hub Json protocol is much easier to use and supports polymorphism. In our real life complex solution, we couldn't use MessagePack everywhere, and MemoryPack seems to have even more constraints. Unless there is a real performance issue related to message size, better stay away from those.

GaijinFizz avatar Mar 05 '24 06:03 GaijinFizz

Guys its been years now, any chance you could actually fix this ? @Sonorpearl As a solution I have just copied their protocol implementation from the aspnet source and made the required changes so my models serialized by desired type, its literally 2 lines of code you need to change.

image

olegsavelos avatar Mar 11 '24 18:03 olegsavelos

Thanks @olegsavelos, I will look into that tomorrow. My plan was to also look at the implementation of osu!, which was done here by @peppy: https://github.com/ppy/osu/pull/14389

BFS-JWesseler avatar Mar 12 '24 16:03 BFS-JWesseler

any chance you could actually fix this ?

its literally 2 lines of code you need to change

We would gladly accept a PR to fix this. This is an open-source project 😄

Tests would likely go in https://github.com/dotnet/aspnetcore/blob/main/src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTests.cs

BrennanConroy avatar Apr 02 '24 02:04 BrennanConroy