protobuf-net / protobuf-net.Grpc

GRPC bindings for protobuf-net and grpc-dotnet
Other
846 stars 106 forks source link

Question: Way to validate that the service interface contract match beween server and client? #284

Open Dunge opened 1 year ago

Dunge commented 1 year ago

Using the "code first" approach with [ProtoContract] and [ProtoMember] attributes on model classes (and not a .proto file).

My application is distributed externally, and when I update the server I also provide a new client release, but my auto-updater mechanism need the application to relaunch to execute. When users don't, I want to pop an error message.

Up to now I have an interceptor that checks for RpcException with this rule:

catch (RpcException ex)
{
    if (ex.StatusCode == StatusCode.Internal && ex.Status.Detail != null && (ex.Status.Detail.Contains("Invalid wire-type") || ex.Status.Detail.Contains("Unexpected wire-type")))
        throw new Exception("Your version does not match the server. Please restart software to update.");
    throw;
}

Now I just ended up with a new error message "Error starting gRPC call. OverflowException: Arithmetic operation resulted in an overflow."

I somehow doubt catching a list of different error message is a good way to go.

So is there a way on the connection establishment itself from the lib to determine if the contract matches?

Or maybe I should create my own "GetVersion()" method contract and update the number every new release and check this?

mgravell commented 1 year ago

This is a great question. Nothing integrated here today, but it would be pretty easy for me to add something, that works via the headers metadata. For example imagine [RequiredVersion("7.2")] on a service/method, which at the client would automatically add this header to all calls, and at the server would automatically test this header, giving a suitable error message. Probably via the "interceptor" API.

Thoughts?

(probably enforcing client >= server by default, with client === server as an option)

mgravell commented 1 year ago

Btw you could implement this manually today, simply by including metadata in the API via CallContext as a parameter.

mgravell commented 1 year ago

I'll have to look into the server bindings, but another interesting possibility (may not be viable): concurrent server mappings:

[Service("blah.whatever"), ApiVersion(3)]
interface IFooV3 {...}

[Service("blah.whatever"), ApiVersion(1, 2)]
interface IFooV1 {...}

(and then either one service that implements both, or two services version-specific)

I'll have to investigate, but ... interesting possibility. Probably not possible, though, if I recall the specifics of the server binding API

Dunge commented 1 year ago

Thank you. Knowing nothing is integrated today answers my question, I'll probably implement some protection on my end. I'll leave it up to you if you want to keep this ticket open as an idea for future development, because yes I believe it could be profitable for other users.

You are right including the version in the header as metadata is a good way, it protects all calls unlike my original idea of a GetVersion() call that the application needs to know when to call at a specific moment.

Your last idea, if I understand correctly, of having multiple implementation of the same method that gets called depending on the client version is even better, it would allow to keep compatibility if the client is not updated the server can still respond using the previous version. But as you mention, I don't know how feasible this is, since you would have to interpret the version metadata before deserializing the proper gRPC message to know which format it uses.

Note that we already can do something similar if you just have different name on the methods (MethodV1(), MethodV2()) on both side of the wire, but having only one Method() from the client point of view and multiple implementation on the server point of view is interesting.

In any case, for my personal use case it's not required. It would requires the server to keep a copy of all different model classes variations, which is not really something I personally wish to maintain in my app, it changes too often. Programmers would often add or delete a property deep under multiple layers of composition in the model and wouldn't be capable of knowing they need to implement a new version of all method calls that could contain that object. As long as the "current latest" version is available, I'm happy.