CommonProtos is not compatible with Grpc 2.32 / protobuf 3.13

[alec-anikin]

CommonProtos 2.1.0
Grpc/Grpc.Tools 2.32.0
.net core 3.1.8

Google.Protobuf.InvalidProtocolBufferException: Message Money doesn't provide the generated method that enables WriteContext-based serialization. You might need to regenerate the generated protobuf code.

[jskeet]

Please could you clarify exactly what you're doing and what fails?

While I would definitely expect to regenerate the code over time, if gRPC and/or Google.Protobuf have made changes that mean code generated against older versions is invalid, that's an issue that should be highlighted in the appropriate project. (Updating to a new version of Google.Protobuf or gRPC shouldn't require every dependency to be regenerated.)

[alec-anikin]

Grpc service proto:

syntax = "proto3";
import "google/type/money.proto";
package my.services.grpc;

service SomeProvider {
    rpc GetPrices(GetPricesRequest) returns (GetPricesResponse);
}
message GetPricesRequest {
    repeated int64 item_ids = 1;
}
message GetPricesResponse {
    repeated Price competitor_prices = 1;
}
message Price {
    int64 item_id = 1;
    google.type.Money price = 2;
}

Exception stack trace:

Google.Protobuf.InvalidProtocolBufferException: Message Money doesn't provide the generated method that enables WriteContext-based serialization. You might need to regenerate the generated protobuf code.
   at My.Services.Grpc.Price.pb::Google.Protobuf.IBufferMessage.InternalWriteTo(WriteContext& output) in /.../obj/Release/netcoreapp3.1/Prices.cs:line 511
   at Google.Protobuf.Collections.RepeatedField`1.WriteTo(WriteContext& ctx, FieldCodec`1 codec)
   at My.Services.Prices.Grpc.GetPricesResponse.pb::Google.Protobuf.IBufferMessage.InternalWriteTo(WriteContext& output) in /../obj/Release/netcoreapp3.1/Prices.cs:line 318
   at Google.Protobuf.MessageExtensions.WriteTo(IMessage message, IBufferWriter`1 output)
   at My.Services.Grpc.SomeProvider.__Helper_SerializeMessage(IMessage message, SerializationContext context) in /.../obj/Release/netcoreapp3.1/pricesGrpc.cs:line 22
   at Grpc.AspNetCore.Server.Internal.PipeExtensions.WriteMessageAsync[TResponse](PipeWriter pipeWriter, TResponse response, HttpContextServerCallContext serverCallContext, Action`2 serializer, Boolean canFlush)
   at Grpc.AspNetCore.Server.Internal.CallHandlers.UnaryServerCallHandler`3.HandleCallAsyncCore(HttpContext httpContext, HttpContextServerCallContext serverCallContext)
   at Grpc.AspNetCore.Server.Internal.CallHandlers.ServerCallHandlerBase`3.<HandleCallAsync>g__AwaitHandleCall|8_0(HttpContextServerCallContext serverCallContext, Method`2 method, Task handleCall)

Worked well until upgrade to grpc 2.32. I think this issue relates to this features
grpc/grpc#23485
protocolbuffers/protobuf#7576

[jskeet]

I was just adding a comment on the same gRPC issue.

I've just validated that I can run existing tests for an "entirely old" package even when using Grpc.Core 2.32.0, so I suspect it's a matter of the gRPC 2.32.0 tooling expecting more than is available here.

I'll change this from a bug to a feature request - I think gRPC should do more to address the compatibility here (as I don't think it's unreasonable to expect older library versions to still work) but there's no harm in regenerating.

[JamesNK]

gRPC will work with older generated code, if the older generated code is the base message, e.g. GreeterReply was generated using old tooling like Grpc.Tools.1.31.0. There is a type check to see if the message implements the new interface, and if it doesn't then the old path is used.

Where it doesn't work is if you have generated a base message with newer tooling, but that messages use types generated with older tooling. For example GreeterReply is generated using Grpc.Tools.1.32.0 but references google.type.Money generated by Grpc.Tools.1.31.0. The base message type check passes and there is no way to know the a nested type won't also pass until runtime.

CommonProtos needs to be regenerated.

[jskeet]

@JamesNK: "CommonProtos needs to be regenerated."

And as I say, I'm happy to do that, but this feels like a significant backward compatibility issue that should have been considered more broadly.

As an analogy, can you imagine the .NET team deciding "If you use .NET 5 tooling to compile a new console app that depends on a library, that library must have been built with .NET 5 as well"? There's no way that would be allowed to fly. This feels like "efficiency at any price, including breaking reasonable use cases". I hope more emphasis is put on backward compatibility in the future.

[JamesNK]

Almost every scenario is supported: new base message + new nested message, old base + old nested, old base + new nested.

The one scenario that doesn't automatically work out of the box is this one: new base + old nested. It was technically impossible. In this situation a user can work around it by setting the GRPC_DISABLE_PROTOBUF_BUFFER_SERIALIZATION compiler flag to force gRPC to use the old serialization code path.

[jskeet]

That "one scenario" has the potential to be a pretty common one though - and exactly the same could have been said about my example with .NET tooling. I think this would have been better as opt in rather than opt out.

I'll try to get to regenerating this today, but of course this isn't the only library that could be affected by this :(

[jskeet]

I've now published Google.Api.CommonProtos 2.2.0, generated with Google.Protobuf.Tools 3.13.0