Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SignalR with Messagepack: JavaScript Number to C# Double Precision Loss #20333

Closed
thelgevold opened this issue Mar 30, 2020 · 5 comments
Closed
Labels
area-signalr Includes: SignalR clients and servers

Comments

@thelgevold
Copy link
Contributor

thelgevold commented Mar 30, 2020

Describe the bug

When serializing and deserializing values between JavaScript and C# using SignalR with MessagePack I am seeing a bit of precision loss in C# on the receiving end.

As an example I am sending the value 0.005 from JavaScript to C#. When the deserialized value appears on the C# side I am getting the value 0.004999999888241291, which is close, but not 0.005 exactly. The value on the JavaScript side is Number and on the C# side I am using double.

I am wondering if there is a way using binary serialization to have matching values on both sides.

If not, does this mean that there is no way to have 100% accurate binary conversions between JavaScript and C#?

SO post for reference: https://stackoverflow.com/questions/60892286/javascript-to-c-sharp-numeric-precision-loss/60921624?noredirect=1#comment107818772_60921624

To Reproduce

What steps can we follow to reproduce the issue?

Followed the steps in this tutorial: https://docs.microsoft.com/en-us/aspnet/core/signalr/messagepackhubprotocol?view=aspnetcore-3.1

Only difference is that I am using ContractlessStandardResolver.Instance

Further technical details

  • ASP.NET Core version 3.1
  • Include the output of dotnet --info
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
@mkArtakMSFT mkArtakMSFT added the area-signalr Includes: SignalR clients and servers label Mar 30, 2020
@weichch
Copy link
Contributor

weichch commented Apr 1, 2020

I'm giving a bit more information here:

msgpack5 by default doesn't force Number to be serialized as double floating point.

// Code from msgpack5
function msgpack (options) {
    options = options || {
        forceFloat64: false,
        ...
    }
}

There's an option to force that to happen, but @microsoft/signalr-protocol-msgpack doesn't allow access to it.

In browser console:

// As per MDN: Number type is double-precision 64-bit binary format IEEE 754 value
msgpack5().encode(Number(0.005))

// Output - 32bit single point
Uint8Array(5) [202, 59, 163, 215, 10]

This works:

msgpack5({forceFloat64:true}).encode(Number(0.005))

// Output
Uint8Array(9) [203, 63, 116, 122, 225, 71, 174, 20, 123]

@analogrelay
Copy link
Contributor

This sounds like something that can't really be avoided to some degree. Serializing floating point numbers is a tricky thing and JavaScript and C# don't necessary have the same encoding rules. You could serialize it as a Uint8Array directly, or even (though it's a bit ugly) a string and parse it back later. It depends on the kind of accuracy you're looking for.

There's an option to force that to happen, but @microsoft/signalr-protocol-msgpack doesn't allow access to it.

This seems like the relevant thing to fix here. We should provide access to the msgpack configuration options. Let's track that work here and leave this open.

@analogrelay analogrelay added this to the Next sprint planning milestone Apr 1, 2020
@thelgevold
Copy link
Contributor Author

thelgevold commented Apr 1, 2020

As an experiment I hacked {forceFloat64:true} into the distributed @microsoft/signalr-protocol-msgpack .js file and things were looking much better. Seems like this would be a very helpful fix unless it has other side effects that I can't see at the moment.

@thelgevold
Copy link
Contributor Author

I have created a PR here with a proposed solution for exposing the underlying msgpack configuration options: #20438

@BrennanConroy
Copy link
Member

Done via #20438

@dotnet dotnet locked as resolved and limited conversation to collaborators May 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

No branches or pull requests

5 participants