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

"Ambiguous match found" error during JSON serialization for Fable.Remoting.Json > 2.16.0 #268

Open
ArtemyB opened this issue Aug 20, 2021 · 13 comments

Comments

@ArtemyB
Copy link

ArtemyB commented Aug 20, 2021

Recently after updating my project dependencies to the latest Fable.Remoting.Giraffe v5.4.0 (which requires Fable.Remoting.Json >= 2.18.0) one of the API function in my Fable.Remoting handler started throwing an error "Ambiguous match found" in some cases. After a quick glance I haven't discovered any obvious pattern (had no time to look thoroughly). So more attentive search is required, because the error occurs only for some instances of the return type (i.e. in some cases the same API function succeeds and in other cases it fails) -- I'll explore this later. But judging by the stack trace I suspect it is related to unions serialization.

At the same time there is no error when I use Fable.Remoting.Json 2.16.0 (requires at least Fable.Remoting.Giraffe 5.7.0-rc-6 and Fable.Remoting.Server 5.21.0).

The exception stack trace:

   at System.DefaultBinder.FindMostDerivedNewSlotMeth(MethodBase[] match, Int32 cMatches)
   at System.RuntimeType.GetMethodImplCommon(String name, Int32 genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConv, Type[] types, ParameterModifier[] modifiers)
   at System.RuntimeType.GetMethodImpl(String name, BindingFlags bindingAttr, Binder binder, CallingConventions callConv, Type[] types, ParameterModifier[] modifiers)
   at System.Type.GetMethod(String name, BindingFlags bindingAttr)
   at Microsoft.FSharp.Reflection.Impl.getUnionCaseConstructorMethod(Type typ, Int32 tag, BindingFlags bindingFlags) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\reflect.fs:line 408
   at Microsoft.FSharp.Reflection.Impl.getUnionCaseConstructorCompiled(Type typ, Int32 tag, BindingFlags bindingFlags) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\reflect.fs:line 418
   at Microsoft.FSharp.Reflection.FSharpValue.PreComputeUnionConstructor(UnionCaseInfo unionCase, FSharpOption`1 bindingFlags) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\reflect.fs:line 1005
   at Fable.Remoting.Json.ReflectionHelpers.mapping@1(UnionCaseInfo uci)
   at Fable.Remoting.Json.ReflectionHelpers.getUnionInfo@105.Invoke(Type t)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Fable.Remoting.Json.FableJsonConverter.WriteJson(JsonWriter writer, Object value, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeConvertable(JsonWriter writer, JsonConverter converter, Object value, JsonContract contract, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
   at Fable.Remoting.Json.FableJsonConverter.WriteJson(JsonWriter writer, Object value, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeConvertable(JsonWriter writer, JsonConverter converter, Object value, JsonContract contract, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
   at Fable.Remoting.Json.FableJsonConverter.WriteJson(JsonWriter writer, Object value, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeConvertable(JsonWriter writer, JsonConverter converter, Object value, JsonContract contract, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
   at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)
   at Fable.Remoting.Server.Proxy.jsonSerialize[a](a o, Stream stream)
   at Fable.Remoting.Server.Proxy.makeEndpointProxy@69-8.Invoke(result _arg1)
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, FSharpFunc`2 userCode, b result1) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 405
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 105
@Zaid-Ajaj
Copy link
Owner

Hi @ArtemyB I haven't seen this error before 🤔 maybe if you can share the return types that are failing, I could also take a look there?

@Zaid-Ajaj
Copy link
Owner

Just to make sure, can you check if using the latest v2.19.0 of Fable.Remoting.Json solves this issue?

@ArtemyB
Copy link
Author

ArtemyB commented Aug 21, 2021

@Zaid-Ajaj sorry for the answer delay: posted the issue and went away from the PC :) But now I'm back and already have tried the recent Fable.Remoting.Json v2.19.0 -- no success, still get the error. However I've looked through the data being serialized in my error-prone Remoting-function and have noticed a pattern: all error-returning calls contain at least one value that looks like Some <DU Case> whereas all successful calls contain value None in these places (in that piece of data I have a record one of the fields of which has type like SomeDU option, where SomeDU: type SomeDU = | Case1 | Case2 | Case3). So maybe this is the problem source -- either it's just Option with DU or DUs with nested DUs in general.

@ArtemyB
Copy link
Author

ArtemyB commented Aug 21, 2021

Hmm, there is also a value of type like Some { R = 102uy; G = 51uy; B = 255uy; A = 255uy } in data serialized during error calls whereas in successful cases there is value None in the similar place (it is a value of another record's field). So type signature of this field is Color option, where Color: type Color = { R: byte; G: byte; B: byte; A: byte }.

@ArtemyB
Copy link
Author

ArtemyB commented Aug 21, 2021

#268 (comment)

Probably Option with DU is not the reason -- one other field of the same record also has type AnotherDU option and its value is Some ... in some of the successful remoting function calls.

@Zaid-Ajaj
Copy link
Owner

Hi @ArtemyB I am looking at the issue again and I am able to reproduce the problem :/ any chance you could post an exact type of the API (and the all the types used in it) so that I can debug it? Creating a full sample repository to demonstrate the problem would be really helpful for me as well

@ArtemyB
Copy link
Author

ArtemyB commented Oct 29, 2021

@Zaid-Ajaj hi. Understood, I'll revisit the project on this weekend and extract problematic fragment to a separate sample.

@ArtemyB ArtemyB closed this as completed Oct 29, 2021
@ArtemyB ArtemyB reopened this Oct 29, 2021
@ArtemyB
Copy link
Author

ArtemyB commented Oct 29, 2021

P.S.: Only a guess: my problem could be connected with PR #254 (don't know how exactly -- simply it looks like the major update between Fable.Remoting.Json v2.16.0 and v2.17.0; + it affects unions and other types serialization)

@Zaid-Ajaj
Copy link
Owner

Hi @ArtemyB is this still an issue for you from the latest versions? If so, please consider posting a sample project that I can test with

@brud
Copy link

brud commented Nov 18, 2022

Hi @Zaid-Ajaj error still there, I have a repro https://github.com/brud/safe-stack-remoting-issue-repro
I also deleted the build project - readme contains the actual commands to start the projects.

@brud
Copy link

brud commented Nov 20, 2022

@Zaid-Ajaj should I create a new issue for that?

@ArtemyB
Copy link
Author

ArtemyB commented Nov 25, 2022

@Zaid-Ajaj I apologize fo such a huge delay, but I simply haven't touched that troubled project, leaving it on the old version of Fable.Remoting. And extracting repro from it required quite a lot of work (the project was kind of a mess), so I simply couldn't get a time and energy to do that.

However now, thanks to @brud and his repro, error's reason is finally revealed: both @brud's and my data has a DU with a case named Tag. And whenever a value of this DU occurs (it could also be any other case from this DU, no only the Tag), the "Ambiguous match found" error arises. As I understand it happens on the reflection level because DU's system Tag property somehow conflicts with a user-defined Tag case during Fable.Remoting.Json's FableConverter logic. Once I renamed my Tag case, the error disappeared.

Note that I just tested on the recent Fable.Remoting.Json v2.21.0, so I suspect the key change was introduced in v2.16.0 together with the DU serialization optimizations. Thus, the error is still present, however luckily knowing its origin, there is workaround 😌

@Zaid-Ajaj
Copy link
Owner

Hi there @brud and thanks a lot for the repro. It is now clear what the problem with the diagnosis from @ArtemyB and how to fix it (not using Tag as a DU case) I will reopen the issue and investigate it 😄

@Zaid-Ajaj Zaid-Ajaj reopened this Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants