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

F# discriminated union custom converter isn't used. #51481

Closed
1 task done
mvejca opened this issue Oct 18, 2023 · 3 comments
Closed
1 task done

F# discriminated union custom converter isn't used. #51481

mvejca opened this issue Oct 18, 2023 · 3 comments
Labels
External This is an issue in a component not contained in this repository. It is open for tracking purposes. ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Status: Resolved

Comments

@mvejca
Copy link

mvejca commented Oct 18, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have an ASP.NET .net 6 minimal API in F#. I'm trying to use a discriminated union in the API using a custom converter. It seems like when ASP.NET tries to serialize the type that it doesn't pick up the attribute on the type. I get the normal error when trying to serialize the discriminated union with the default converter. Just calling JsonSerializer.Serialize with the type works without issue, the issue is only when ASP.NET tries to serialize the type

"Hello world!"
info: Microsoft.Hosting.Lifetime[14]
      Now listening on: http://localhost:5211
info: Microsoft.Hosting.Lifetime[0]
      Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
      Hosting environment: Development
info: Microsoft.Hosting.Lifetime[0]
      Content root path: C:\src\project
fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.NotSupportedException: F# discriminated union serialization is not supported. Consider authoring a custom converter for the type.
         at System.Text.Json.Serialization.Converters.FSharpTypeConverterFactory.CreateConverter(Type typeToConvert, JsonSerializerOptions options)
         at System.Text.Json.Serialization.JsonConverterFactory.GetConverterInternal(Type typeToConvert, JsonSerializerOptions options)
         at System.Text.Json.JsonSerializerOptions.GetConverterInternal(Type typeToConvert)
         at System.Text.Json.JsonSerializerOptions.DetermineConverter(Type parentClassType, Type runtimePropertyType, MemberInfo memberInfo)
         at System.Text.Json.Serialization.Metadata.JsonTypeInfo.GetConverter(Type type, Type parentClassType, MemberInfo memberInfo, Type& runtimeType, JsonSerializerOptions options)
         at System.Text.Json.JsonSerializerOptions.<InitializeForReflectionSerializer>g__CreateJsonTypeInfo|112_0(Type type, JsonSerializerOptions options)
         at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
         at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
         at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions options, Type runtimeType)
         at System.Text.Json.JsonSerializer.SerializeAsync[TValue](Stream utf8Json, TValue value, JsonSerializerOptions options, CancellationToken cancellationToken)
         at Microsoft.AspNetCore.Http.HttpResponseJsonExtensions.WriteAsJsonAsyncSlow[TValue](Stream body, TValue value, JsonSerializerOptions options, CancellationToken cancellationToken)
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

Expected Behavior

It should use the custom converter I defined

Steps To Reproduce

Used the starting dotnet new web -lang=F# project and replaced the Program.fs with below

open System
open System.Text.Json
open Microsoft.AspNetCore.Builder
open Microsoft.AspNetCore.Http.Json
open Microsoft.Extensions.DependencyInjection
open System.Text.Json.Serialization

[<JsonConverter(typeof<IDConverter>)>]
type ID =
    | String of string
    | Number of int
and IDConverter() =
    inherit JsonConverter<ID>()
    with
        override _.Write(writer, value, options) =
            match value with
            | String s -> JsonSerializer.Serialize(writer, s, options)
            | Number n -> JsonSerializer.Serialize(writer, n, options)
        override _.Read(reader, _, _) =
            match reader.TokenType with
            | JsonTokenType.String ->Sting (reader.GetString())
            | JsonTokenType.Number -> Number(reader.GetInt32())
            | _ -> raise (JsonException "Expected String or Number ID")
  
[<EntryPoint>]
let main args =
    let builder =
        WebApplication.CreateBuilder(args)

    builder.Services.Configure<JsonOptions> (fun (opts: JsonOptions) ->
        opts.SerializerOptions.Converters.Add(IDConverter())) |> ignore
    let app = builder.Build()

    //Works
    printfn $"""{JsonSerializer.Serialize (ID.String "Hello world!")}"""
    // Doesn't work
    app.MapGet("/", Func<ID>(fun () -> ID.String "Hello world!"))
    |> ignore

    app.Run()

    0 // Exit code

Exceptions (if any)

No response

.NET Version

6

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Oct 18, 2023
@mvejca
Copy link
Author

mvejca commented Oct 20, 2023

It seems like this is an issue with how in F# when you get the type of the value at runtime it doesn't actually match the type at compile time and the default CanConvert method for the custom converter doesn't recognize them at the same type and skips using the custom converter.
This is shown by running this:

    let a = (ID.String "A").GetType()
    let b = typeof<ID>

a is Program+ID+String but b is Program+ID

Changing the custom converter to implement the CanConvert method and compare the types a bit differently does work, but this does seem a bit hacky

        override this.CanConvert(t) =
            if FSharpType.IsUnion(t) then
                let unionCaseInfo = FSharpType.GetUnionCases(t).[0]
                unionCaseInfo.DeclaringType = typeof<ID>
            else false

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@captainsafia
Copy link
Member

Catching up on triage (yes, this is a major catch-up...)

It looks like this issue might arise from the a problem in the System.Text.Json serializer. dotnet/runtime#55744 provides some guidance on alternatives until the feature is supported in STJ. This alternatives might have a better story for working around the compile-time vs. runtime type discrepancy you're seeing in the custom converter.

As far as triage goes, I'm going to mark this as external for now. It doesn't seem like there's something directly actionable for minimal APIs to do.

@captainsafia captainsafia added External This is an issue in a component not contained in this repository. It is open for tracking purposes. ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. labels Aug 14, 2024
Copy link
Contributor

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External This is an issue in a component not contained in this repository. It is open for tracking purposes. ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Status: Resolved
Projects
None yet
Development

No branches or pull requests

3 participants