Closed Zaid-Ajaj closed 6 months ago
Zaid-Ajaj
commented
6 months ago The IBinaryServer.stringEnum test is failing where the data sent to the server is
["secondString"]and the response:
{
"error": "Message: The given key 'secondString' was not present in the dictionary., request body: Some \"[\"secondString\"]\"",
"ignored": false,
"handled": true
}
kerams
commented
6 months ago I'll find some time this week.
If I remember right Fable.Remoting binary tests are Client-Server roundtrips where we do deserialization in JS. On the other hand, Message Pack serialization tests probably tests (de)serialization in JS.
Zaid-Ajaj
commented
6 months ago No rush 😄 no one seems to have complained about this yet since most things are working as they should (client-server wise) only the client-side roundtripping is failing
kerams
commented
6 months ago Could IBinaryServer.stringEnum indicate a problem in Fable.SimpleJson on the server?
The write failures are a symptom of a regression in Fable around the (un)currying of function parameters. I see this in 4.5.0, which would incidentally explain why I couldn't get MessagePack to work in Fable.SignalR some time ago.
When I change
let private serializerCache = System.Collections.Generic.Dictionary<string, obj -> ResizeArray<byte> -> unit> ()(and modify writeObject accordingly) to
let private serializerCache = System.Collections.Generic.Dictionary<string, Action<obj, ResizeArray<byte>>> ()all Message Pack serialization tests with the exception of Enum start passing.
cc @MangelMaxime
MangelMaxime
commented
6 months ago @kerams I am no expert regarding the curry/uncurry stuff.
You are problem better to open an issue on Fable repo so others can have a look at it. The lines which seems to cause problem is outArg = curry2(v);.
kerams
commented
6 months ago IBinaryServer.stringEnum string enum is failing because the string representation of a DU case is now camel-cased, and so no match is found on the server.
with the exception of Enum start passing
because of native bigint support, writeInt64 (box x :?> int64) had to be changed to writeInt64 (box x :?> int |> int64) - int to int64 conversions are no longer a no-op.
kerams
commented
6 months ago a DU case is now camel-cased
Fable 4 now does this automatically. I suggest we only support StringEnum with CaseRules.None - they are rare in Client-Server communication anyway.
Zaid-Ajaj
commented
6 months ago Fable 4 now does this automatically. I suggest we only support StringEnum with CaseRules.None - they are rare in Client-Server communication anyway.
That sounds reasonable, I tend to think about [<StringEnum>] in Fable intended for interop purposes, not client-server comms
Hm, why is this Fable only? If the server doesn't know it's dealing with a string enum, it will serialize enums as numbers.
I tried to simplify the setup but not having the shared project reference Fable.Core but I see that you managed to work around it 🎉 really appreciate it @kerams ❤️
Zaid-Ajaj
commented
6 months ago Merged and published a batch of packages based on changes to the MsgPack package, should be usable with latest Fable 🚀
@kerams I've updated the tests to use latest Fable, currently at v4.9.0 but the results aren't very good
Test Results
``` ========== SUMMARY ========== Total test count 216 Passed tests 164 Failed tests 52 Skipped tests 0 ========== TESTS ========== √ Fable.Remoting / Proxy.combineWithBaseUrlWorks √ Fable.Remoting / IServer.simulateLongComputation cancellation √ Fable.Remoting / IServer.getLength √ Fable.Remoting / IServer.echoTupleMap √ Fable.Remoting / IServer.echoTupleSet √ Fable.Remoting / IServer.returnUnit √ Fable.Remoting / IServer.intToUnit √ Fable.Remoting / IServer.tupleToUnit √ Fable.Remoting / IServer.tupleToTuple √ Fable.Remoting / IServer.echoAnonymousRecord √ Fable.Remoting / IServer.echoNestedAnonRecord √ Fable.Remoting / IServer.binaryContent √ Fable.Remoting / IServer.echoTestCommand √ Fable.Remoting / IServer.privateConstructor √ Fable.Remoting / IServer.echoRemoteWorkEntity √ Fable.Remoting / IServer.binaryContent √ Fable.Remoting / IServer.echoToken √ Fable.Remoting / ISever.echoInteger √ Fable.Remoting / IServer.simpleUnit √ Fable.Remoting / IServer.echoString √ Fable.Remoting / IServer.echoRecordWithChar √ Fable.Remoting / IServer.echoRecordWithChar using characters with accents √ Fable.Remoting / IServer.echoUnionOfOtherUnions √ Fable.Remoting / IServer.echoBool √ Fable.Remoting / IServer.mapRecordAsKey √ Fable.Remoting / IServer.mapDateTimeOffsetAsKey √ Fable.Remoting / IServer.echoIntKeyMap √ Fable.Remoting / IServer.echoBigIntKeyMap √ Fable.Remoting / IServer.echoLongKeyMap √ Fable.Remoting / IServer.echoDecimalKeyMap √ Fable.Remoting / IServer.setRecordAsValue √ Fable.Remoting / IServer.echoIntOption √ Fable.Remoting / IServer.echoStringOption √ Fable.Remoting / IServer.echoPrimitiveLong √ Fable.Remoting / IServer.echoPrimitiveLong with large values √ Fable.Remoting / IServer.echoComplexLong √ Fable.Remoting / IServer.echoOptionalLong √ Fable.Remoting / IServer.echoSingleDULong √ Fable.Remoting / IServer.echoLongInGenericUnion √ Fable.Remoting / IServer.echoSimpleUnionType √ Fable.Remoting / IServer.echoGenericUnionInt √ Fable.Remoting / IServer.echoGenericUnionString √ Fable.Remoting / IServer.echoRecord √ Fable.Remoting / IServer.echoHighScores √ Fable.Remoting / IServer.echoHighScores without do √ Fable.Remoting / IServer.echoHighScores √ Fable.Remoting / IServer.echoHighScores without do √ Fable.Remoting / IServer.echoNestedGeneric √ Fable.Remoting / IServer.echoOtherDataC √ Fable.Remoting / IServer.echoIntList √ Fable.Remoting / IServer.echoSingleCase √ Fable.Remoting / IServer.echoStringList √ Fable.Remoting / IServer.echoBoolList √ Fable.Remoting / IServer.echoListOfListsOfStrings √ Fable.Remoting / IServer.echoResult for ResultIt's funny because from the binary server tests only
IBinaryServer.stringEnumis failing but then almost all msg pack serialization tests are failing (except for a few).Any chance you could have a look?