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

2 generated files with same message name makes deserialize response error #1462

Open
rizdaputra opened this issue Sep 6, 2024 · 11 comments
Open

Comments

@rizdaputra
Copy link

So i have different proto files, they have each a request that have same message response name but the value and structure are actually different. This cause problem whenever i declare both then one of them is rendered as that message even though i am calling the other function.

e.g.

ClientA has getStatus with response StatusResponse
ClientB has getStatus with response StatusResponse

but the StatusResponse definition in ClientA and ClientB is different but whenver i call each getStatus both are using the StatusResponse when doing assertion.

@sampajano
Copy link
Collaborator

Hi, thanks for the question :)

May I ask if you're using Typescript or Common JS, and can you show me how you're importing the client classes?


I'm wondering, if you could just use type aliases to get around this issue?

e.g.

import {StatusResponse as StatusResponseA} from './client1_pb';
import {StatusResponse as StatusResponseB} from './client2_pb';

When you're importing the message classes like here?

import {EchoRequest, EchoResponse, ServerStreamingEchoRequest, ServerStreamingEchoResponse} from './echo_pb';

@rizdaputra
Copy link
Author

i am using typescript. So you mean i need to edit the generated files manually? because the problem i am facing is that this is happening on the client generated files, not on my code, it is error when deserializing. I need a solution where i dont need to edit the generated code because i run the generation on every CI/CD to make sure it is updated if there are any proto changes

@sampajano
Copy link
Collaborator

Ohh i see.. no i wasn't suggesting you to edit the generated files manually..

I was thinking that if you have 2 different versions of StatusResponse, then you can import them using an alias (like the example i gave) to avoid collision in your client code.

Yeah it definitely doesn't make sense for you having to edit the generated files yourself :)


So, if it's an error during deserializing, i guess the generated code has confused the 2 StatusResponse classes, right?

If you can provide some more details on what the generated files look like it might help with understanding of this issue.

@rizdaputra
Copy link
Author

yes it confused the 2 statusresponse classes,

when i call the request in the first client it checks the statusresponse class of the second class instead

These are the classes generated, i omitted some item because i cannot disclose some of the item, but basically there are some differences:

export class RequestStatusResponse extends jspb.Message {
  getRobotstatus(): RobotConnectionStatus_t;
  setRobotstatus(value: RobotConnectionStatus_t): RequestStatusResponse;

  getStatustimestamp(): string;
  setStatustimestamp(value: string): RequestStatusResponse;

  serializeBinary(): Uint8Array;
  toObject(includeInstance?: boolean): RequestStatusResponse.AsObject;
  static toObject(includeInstance: boolean, msg: RequestStatusResponse): RequestStatusResponse.AsObject;
  static serializeBinaryToWriter(message: RequestStatusResponse, writer: jspb.BinaryWriter): void;
  static deserializeBinary(bytes: Uint8Array): RequestStatusResponse;
  static deserializeBinaryFromReader(message: RequestStatusResponse, reader: jspb.BinaryReader): RequestStatusResponse;
}

export namespace RequestStatusResponse {
  export type AsObject = {
    robotstatus: RobotConnectionStatus_t,
    statustimestamp: string,
  }
}

export enum RobotConnectionStatus_t { 
  CONNECTION_STATUS_INVALID = 0,
  CONNECTION_STATUS_CONNECTED = 1,
  CONNECTION_STATUS_ERROR = 2,
  CONNECTION_STATUS_TIMEOUT = 3,
  CONNECTION_STATUS_SERVER_ERROR = 4,
  CONNECTION_STATUS_MISSING_OBSERVER = 5,
}

and the other one is

export class RequestStatusResponse extends jspb.Message {
  getConnectionstatus(): RobotConnectionStatus_t;
  setConnectionstatus(value: RobotConnectionStatus_t): RequestStatusResponse;

  getConnectedtool(): Tool_t;
  setConnectedtool(value: Tool_t): RequestStatusResponse;

  serializeBinary(): Uint8Array;
  toObject(includeInstance?: boolean): RequestStatusResponse.AsObject;
  static toObject(
    includeInstance: boolean,
    msg: RequestStatusResponse,
  ): RequestStatusResponse.AsObject;
  static serializeBinaryToWriter(
    message: RequestStatusResponse,
    writer: jspb.BinaryWriter,
  ): void;
  static deserializeBinary(bytes: Uint8Array): RequestStatusResponse;
  static deserializeBinaryFromReader(
    message: RequestStatusResponse,
    reader: jspb.BinaryReader,
  ): RequestStatusResponse;
}

export namespace RequestStatusResponse {
  export type AsObject = {
    connectionstatus: RobotConnectionStatus_t;
    connectedtool: Tool_t;
  };
}

export enum RobotConnectionStatus_t {
  CONNECTION_STATUS_INVALID = 0,
  CONNECTION_STATUS_CONNECTED = 1,
  CONNECTION_STATUS_ERROR = 2,
  CONNECTION_STATUS_TIMEOUT = 3,
  CONNECTION_STATUS_SERVER_ERROR = 4,
}
export enum Tool_t {
  TOOL_INVALID = 0,
  TOOL_A= 1,
  TOOL_B= 2,
  TOOL_C= 3,
}

the main function

methodDescriptorRequestStatus = new grpcWeb.MethodDescriptor(
    '/Service/RequestStatus',
    grpcWeb.MethodType.UNARY,
    google_protobuf_empty_pb.Empty,
    generated_pb.RequestStatusResponse,
    (request: google_protobuf_empty_pb.Empty) => {
      return request.serializeBinary();
    },
    generated_pb.RequestStatusResponse.deserializeBinary,
  );

  requestStatus(
    request: google_protobuf_empty_pb.Empty,
    metadata?: grpcWeb.Metadata | null,
  ): Promise<generated_pb.RequestStatusResponse>;

  requestStatus(
    request: google_protobuf_empty_pb.Empty,
    metadata: grpcWeb.Metadata | null,
    callback: (
      err: grpcWeb.RpcError,
      response: generated_pb.RequestStatusResponse,
    ) => void,
  ): grpcWeb.ClientReadableStream<generated_pb.RequestStatusResponse>;

  requestStatus(
    request: google_protobuf_empty_pb.Empty,
    metadata?: grpcWeb.Metadata | null,
    callback?: (
      err: grpcWeb.RpcError,
      response: generated_pb.RequestStatusResponse,
    ) => void,
  ) {
    if (callback !== undefined) {
      return this.client_.rpcCall(
        this.hostname_ + '/Service/RequestStatus',
        request,
        metadata || {},
        this.methodDescriptorRequestStatus,
        callback,
      );
    }
    return this.client_.unaryCall(
      this.hostname_ + '/Service/RequestStatus',
      request,
      metadata || {},
      this.methodDescriptorRequestStatus,
    );
  }
proto.RequestStatusResponse.deserializeBinary = function(bytes) {
  var reader = new jspb.BinaryReader(bytes);
  var msg = new proto.RequestStatusResponse;
  return proto.RequestStatusResponse.deserializeBinaryFromReader(msg, reader);
};

@sampajano
Copy link
Collaborator

@rizdaputra Aha i see! Thanks for providing the file content!

I'm if you could could also share how "RequestStatusResponse" was imported in the 2 generated services?

What's the namespace that was used?

And would you be able to specify different namespaces so they can be imported in different ways (to avoid the confusions)?

@rizdaputra
Copy link
Author

i think in the auto generated files it is always generated imports like this

import * as proto_file_name from 'generated_pb'

so when calling it is doing proto_file_name.RequestStatusResponse

You mean i change in the proto so the name will be different? I am searching for a solution because the owner of the proto is not me and it is used in multiple application, changing it would break multiple applications that are not maintained by me. Wondering if there is a way to add prefix or suffix when generating the service/client or is there another solution for this

@sampajano
Copy link
Collaborator

Could you share the exact command you used for generating the proto & grpc files? I'll take a look at if that's possible! Thanks!

@rizdaputra
Copy link
Author

i used

protoc -I=./protocollection/FirstProtoFolder FirstProtoFile.proto --js_out=import_style=commonjs,binary:./src/grpcs --grpc-web_out=import_style=typescript,mode=grpcweb:./src/grpcs
protoc -I=./protocollection/SecondProtoFolder SecondProtoFile.proto --js_out=import_style=commonjs,binary:./src/grpcs --grpc-web_out=import_style=typescript,mode=grpcweb:./src/grpcs

@sampajano
Copy link
Collaborator

@rizdaputra Hihi! Thanks for providing the commands you used! And sorry for the delay!!

So I've tried creating 2 folders like below (under the echo folder):

./test1/echo.proto
./test2/echo2.proto

With echo.proto and echo2.proto having exactly the same content.

And when i run:

protoc -I=./test1 echo.proto --js_out=import_style=commonjs:./ts-example --grpc-web_out=import_style=typescript,mode=grpcwebtext:./ts-example

protoc -I=./test2 echo2.proto --js_out=import_style=commonjs:./ts-example --grpc-web_out=import_style=typescript,mode=grpcwebtext:./ts-example

I'm getting 2 files, and one of them would be using echo2_pb.EchoRequest and the other using echo2_pb.EchoRequest. Like below:

Screenshot 2024-10-17 at 5 30 36 PM

That seems like it might work without conflict to me?

Is this what you're seeing and would this potentially work for you?

Thanks!

@rizdaputra
Copy link
Author

@sampajano thank you for getting back and trying to replicate for me, in my case they are in a different folder. But yes code wise the generated code is correct, but when the code is compiled somehow the function calling is actually conflicting and the only one conflicting is the deserialize process of the response. i have another call that are exactly same and it is not conflicting. But when the same response type name with different structure, the type that is used for deserializing the response is wrong

@rizdaputra
Copy link
Author

It is generated like this, as you can see there are differences but the name is the same, is it because the class name is same then it is conflicting? I want to resolve this without changing the name in the proto file since that would need to change multiple parts of the system and might be breaking
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants