Seeming error with generated code when using Goole API Annotations

Question

Seeming error with generated code when using Goole API Annotations

brian-scb opened this issue 3 years ago · 15 comments

When using a proto file with Goole API Annotations for a gateway I get an error about a function that cannot be applied:

.../Grpc/Health.fs(28,25): error FS0003: This value is not a function and cannot be applied. .../Product/Product.fsproj]

snippet of the generated code:

global.System.Lazy<_>(
            (fun () ->
                global.Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode(
                    descriptorData,
                    [|
                        global.Google.Api.AnnotationsReflection.Descriptor() // this is like 28, the line that is generating the error
                    |],
                    new global.Google.Protobuf.Reflection.GeneratedClrTypeInfo(
                        null,
                        null,
                        [|
                            new global.Google.Protobuf.Reflection.GeneratedClrTypeInfo(typeof<global.Grpc.Health.V1.HealthCheckRequest>, global.Grpc.Health.V1.HealthCheckRequest.Parser, [| "Service" |], null, null, null, null)
                            new global.Google.Protobuf.Reflection.GeneratedClrTypeInfo(typeof<global.Grpc.Health.V1.HealthCheckResponse>, global.Grpc.Health.V1.HealthCheckResponse.Parser, [| "Status" |], null, [| typeof<global.Grpc.Health.V1.HealthCheckResponse.Types.ServingStatus> |], null, null)
                        |]
                    )
                )
            ),
            true
        )

Seems if I remove the () from global.Google.Api.AnnotationsReflection.Descriptor() that particular error goes away, not sure if it's generating any other problems as I'm not using the dotnet version of the gateway at this time.

I used the following proto file for this example:

syntax = "proto3";

package grpc.health.v1;

import "google/api/annotations.proto";

option csharp_namespace = "Grpc.Health.V1";
option go_package = "google.golang.org/grpc/health/grpc_health_v1";

message HealthCheckRequest {
  string service = 1;
}

message HealthCheckResponse {
  enum ServingStatus {
    UNKNOWN = 0;
    SERVING = 1;
    NOT_SERVING = 2;
    SERVICE_UNKNOWN = 3;  // Used only by the Watch method.
  }
  ServingStatus status = 1;
}

service Health {
  rpc Check(HealthCheckRequest) returns (HealthCheckResponse) {
    option (google.api.http).post = "/health/status";
  };
}

Answer 1 · 2021-06-29T14:10:25.000Z

I just encountered the same issue. There are IMO 2 solutions to this problem, I'm fine with either if @Arshia001 is fine with them.

Change the

let Descriptor(): global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value

let Descriptor: global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value

This makes the generator compatible with the Google.Api.CommonProtos NuGet package and other packages that are generated by the C# generator

The user of the plugin just compiles the google/apis protos them selves and then depend on them.
I tried this approach but since the annotations.proto is using extensions the plugin failed to generate correct code.

I'm currently working on fixing the extensions support in the plugin, will link a PR once I have something working.

@Arshia001 What do you think?

Answer 2 · 2021-06-29T16:47:53.000Z

It it proving to be a bit tricky to implement the extension support. I'll be going route 1. to unblock my work. I'll open a PR tomorrow with the changes.

@Arshia001 Do you have any ideas on how best to implement extension support? Extension are not that widely used so it's not a huge problem if route 1. is acceptable since it allows users to use the googleapis protos.

Answer 3 · 2021-06-29T20:01:15.000Z

The code generator wasn't really meant to be compatible with the C# code generator. It's meant to be its own thing, in much the same way the code generator for any other language (Java, Go, etc.) is incompatible with the C# code generator. This is, in fact, the reason why there's a separate Protobuf.FSharp package; we could have used Google's (MS's?) official package if the code generator was indeed compatible.

It may be possible to make the codegen compatible with the C# codegen to some degree (though it will be hard at least, if not impossible), but there'll always be the fact that code generated by the C# codegen is fundamentally different. The C# codegen generates classes and properties with presence APIs and whatnot, while the F# codegen generates records of ValueOption fields. If we were OK with using the C# codegen from F#, we wouldn't need this repo in the first place. One can always simply put the .protos in a C# library and reference that from F#. We're already doing this with EF (when we use it, which is rare) and the WCF client libraries.

To fix this situation, I believe we need to fix the edge cases where the F# codegen doesn't work, and use it to generate code from referenced .protos. If those .protos come with supporting logic inside their library, the logic has to be reimplemented in F#. This is actually the case for the built-in timestamp type.

Now, for this particular case, I don't know how extensions work. I'm sure we can work together to figure something out though. Can you give me a few pointers on extensions and the API's you're having trouble with? If extensions are really rare (haven't seen them used myself, yet) and this isn't likely to be a problem with many other API's, we could even try to fix the generated code by hand, though that's the very last option.

Answer 4 · 2021-06-29T20:27:56.000Z

I'm not 100% following. I was using the C# code gen and I'm trying to get away from that to keep it F# to take advantage of using record types instead of the class based interface from the C# version. Is there something I should have done different that made you think I was trying use C# for something.

This extension I'm using is for using the gRPC gateway. I believe this is also supported by the C# code (might be wrong about this though)

edit: adding references to the protos for annotations and http

Answer 5 · 2021-06-29T20:36:04.000Z

You're not using the C# codegen, but the Google library you're probably using is. Code generated by the two code generators is incompatible which is why you're getting an error.

Answer 6 · 2021-06-29T20:37:47.000Z

I was trying to add those as links in the fsproj it found the annotations file, but ended up throwing an error finding the http file.

[edit]: I see now that code gen for the Annotations file is looking a little odd with a module at the end that doesn't seem complete and some other errors. Can see in a gist here

Answer 7 · 2021-06-30T05:12:03.000Z

Well, something is clearly missing there, both on line 27 Api..google and at the end. I'll have to take a look.

Answer 8 · 2021-06-30T10:18:51.000Z

I'm trying to fix the extension generation but I'm having some problems with ValueOptions at the moment:

Currently the generated code in my branch is:

// <auto-generated>
//     Generated by the F# GRPC code generator. DO NOT EDIT!
//     source: google/api/annotations.proto
// </auto-generated>
namespace rec Google.Api
#nowarn "40"
module AnnotationsReflection =
    let private descriptorBackingField: global.System.Lazy<global.Google.Protobuf.Reflection.FileDescriptor> =
        let descriptorData = global.System.Convert.FromBase64String("\
            Chxnb29nbGUvYXBpL2Fubm90YXRpb25zLnByb3RvEgpnb29nbGUuYXBpGhVnb29nbGUvYXBpL2h0dHAu\
            cHJvdG8aIGdvb2dsZS9wcm90b2J1Zi9kZXNjcmlwdG9yLnByb3RvOksKBGh0dHASHi5nb29nbGUucHJv\
            dG9idWYuTWV0aG9kT3B0aW9ucxiwyrwiIAEoCzIULmdvb2dsZS5hcGkuSHR0cFJ1bGVSBGh0dHBCbgoO\
            Y29tLmdvb2dsZS5hcGlCEEFubm90YXRpb25zUHJvdG9QAVpBZ29vZ2xlLmdvbGFuZy5vcmcvZ2VucHJv\
            dG8vZ29vZ2xlYXBpcy9hcGkvYW5ub3RhdGlvbnM7YW5ub3RhdGlvbnOiAgRHQVBJYgZwcm90bzM=")
        global.System.Lazy<_>(
            (fun () ->
                global.Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode(
                    descriptorData,
                    [|
                        global.Google.Api.HttpReflection.Descriptor()
                        global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
                    |],
                    new global.Google.Protobuf.Reflection.GeneratedClrTypeInfo(
                        null,
                        [|
                            global.Google.Api.AnnotationsExtensions.http
                        |],
                        null
                    )
                )
            ),
            true
        )
    let Descriptor(): global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value
module AnnotationsExtensions =
    let http = global.Google.Protobuf.Extension<global.Google.Protobuf.Reflection.MethodOptions,ValueOption<global.Google.Api.HttpRule>>(72295728, global.Google.Protobuf.FieldCodec.ForMessage(578365826u, global.Google.Api.HttpRule.Parser))
// End of auto-generated code

The error I'm getting is:

error FS0001: A generic construct requires that the type 'ValueOption<HttpRule>' have reference semantics, but it does not, i.e. it is a struct

Answer 9 · 2021-06-30T10:23:58.000Z

The WIP is here: #8

Answer 10 · 2021-06-30T10:34:26.000Z

@purkhusid the error indicates that Google.Protobuf.Extension expects a reference type (i.e. class in C#, types not annotated with [<Struct>] in F#). ValueOption is a value type, hence the name value option.

Are you sure the generic argument needs to be an option? I think it needs to be the type itself, e.g. global.Google.Protobuf.Extension<global.Google.Protobuf.Reflection.MethodOptions,global.Google.Api.HttpRule>.

Answer 11 · 2021-06-30T13:09:14.000Z

@Arshia001 Yeah, I wasn't sure if the extensions should be ValueOption or not, but it seems like it's not possible.

I updated the PR and now everything compiles. I'm not familiar enough with Extensions to know if the generated code is doing anything wrong though. It looks correct if I compare it to the C# code.

Answer 12 · 2021-07-02T10:43:39.000Z

@Arshia001 The error is a bit more complex than anticipated. After testing out the code in my PR I got a runtime exception related to the descriptor.proto:

Unhandled exception. System.TypeInitializationException: The type initializer for '<StartupCode$ingestion_api-service>.$Service' threw an exception.
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at Google.Protobuf.Reflection.SingleFieldAccessor..ctor(PropertyInfo property, FieldDescriptor descriptor)
   at Google.Protobuf.Reflection.FieldDescriptor.CreateAccessor()
   at Google.Protobuf.Reflection.FieldDescriptor.CrossLink()
   at Google.Protobuf.Reflection.MessageDescriptor.CrossLink()
   at Google.Protobuf.Reflection.FileDescriptor.CrossLink()
   at Google.Protobuf.Reflection.FileDescriptor.BuildFrom(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, Boolean allowUnknownDependencies, GeneratedClrTypeInfo generatedCodeInfo)
   at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode(Byte[] descriptorData, FileDescriptor[] dependencies, GeneratedClrTypeInfo generatedCodeInfo)
   at <StartupCode$Protobuf-FSharp>.$Descriptor.clo@192-7.Invoke()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
   at <StartupCode$google-api-protos>.$Annotations.clo@26-27.Invoke() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/google/api/fsharp_annotations_proto.dll_pb/Annotations.fs:line 27
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at Google.Api.AnnotationsReflection.Descriptor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/google/api/fsharp_annotations_proto.dll_pb/Annotations.fs:line 48
   at <StartupCode$ingestion_api-service>.$Service.clo@19.Invoke() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 20
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at Src.Protobuf.IngestionApi.Service.ServiceReflection.Descriptor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 35
   at <StartupCode$ingestion_api-service>.$Service.Descriptor@47-1() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
   at <StartupCode$ingestion_api-service>.$Service.clo@47-10.Invoke(Unit arg00@) in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at Microsoft.FSharp.Control.LazyExtensions.Force[T](Lazy`1 ) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\prim-types.fs:line 6629
   at <StartupCode$ingestion_api-service>.$Service..cctor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
   --- End of inner exception stack trace ---
   at <StartupCode$ingestion_api-service>.$Service.get___Method_Bootstrap@46()
   at Src.Protobuf.IngestionApi.Service.BootstrapService.BootstrapServiceMethodBinder.BindService(BootstrapServiceBase serviceImpl) in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 50
   at Main.main(String[] argv) in /home/danielp/dannidev/monorepo/src/dotnet/apis/ingestion_api/Main.fs:line 60

I debugged the issue and it comes down the following:

The annotations.proto depends on the descriptor.proto and the generator resolves the descriptor for descriptor.proto to global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()

Since we depend on the Google.Protobuf package to parse the descriptors we get an null reference exception here: https://github.com/protocolbuffers/protobuf/blob/6b0ff74ecf63e26c7315f6745de36aff66deb59d/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs#L90-L90

The descriptor.proto is proto2 and the Google.Protobuf package assumes that there is an Has<property name> property on all fields on a proto2 message(Also applies for optional in proto3). The global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor() in Protobuf.FSharp does not have these properties and thus we get a null reference exception.

I tried editing the generated code manually to use global.Google.Protobuf.Reflection.DescriptorReflection.Descriptor instead of global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor() and the runtime errors were no longer present.

Since this is also an error when using the optional keyword in proto3 I think the correct approach is to fix the generator so that if the proto file is proto2 or the field is optional we add the Has<field name> and Clear<field name> properties to generated code.

Answer 13 · 2021-07-06T14:25:07.000Z

I have a fix ready for review in #8 along with some other generation fixes

Answer 14 · 2021-07-06T14:26:37.000Z

That's amazing! I'll check it out as soon as I can. Thanks!

Answer 15 · 2022-07-17T13:11:06.000Z

Fixed in #16.