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

Failed to generate pb file when using proto3 version. #1717

Closed
zhushenhang opened this issue Mar 15, 2021 · 22 comments · Fixed by #1718 or #1725
Closed

Failed to generate pb file when using proto3 version. #1717

zhushenhang opened this issue Mar 15, 2021 · 22 comments · Fixed by #1718 or #1725

Comments

@zhushenhang
Copy link

Problem description

Failed to generate pb file when using proto3 version.
UHDInterface.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-grpc hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--grpc_out

Reproduction steps

  1. I yarn add grpc-tools
  2. cmd command: protoc --js_out=import_style=commonjs,binary:./ --plugin=protoc-gen-grpc=./grpc_node_plugin.exe --grpc_out=./ UHDInterface.proto
  3. Finally reported an error

Environment

  • OS name, version and architecture: Windows 10
  • Node version v14.15.1
  • Node installation method msi
  • If applicable, compiler version [e.g. clang 3.8.0-2ubuntu4]
  • Package name and version grpc-tools ^1.10.0

Additional context

I used grpc to build the client and server for the first time and learned the official example. I see the following two files: helloworld_pb and helloworld_grpc_pb.I did not find an official example of generating these two files. I found other information. I would like to ask, how do we generally generate these two files now?

@zhushenhang
Copy link
Author

In addition, I saw that there are two official methods: dynamic and static. Is there any specific difference between the two? My colleague thinks static is better

@murgatroid99
Copy link
Member

The description of how those files are generated in the example can be found here: https://github.com/grpc/grpc/tree/master/examples/node/static_codegen.

In this context, "static" code generation refers to generating files ahead of time using grpc-tools and using them with the google-protobuf library. "Dynamic" code generation refers to loading .proto files and generating objects at runtime using the @grpc/proto-loader library. Those two libraries provide different interfaces for the same services.

@zhushenhang
Copy link
Author

@murgatroid99 Thank you so much for your guidance.
It is normal for protoc to generate the pb file of the proto3 version, but with the addition of grpc_node_plugin to generate the pb file, an error is still reported that the proto3 version is not supported. Where can I download the latest version of grpc_node_plugin?

@zhushenhang
Copy link
Author

@murgatroid99 Does the grpc_node_plugin tool do not support the optional in proto3? My colleague's server is C++, and he said that the latest version can support it. Does grpc_node_plugin plan to update to support optional in the future?

@murgatroid99
Copy link
Member

That is not working for you because the version of protoc distributed in grpc-tools is not up to date, so it does not have support for optional.

@zhushenhang
Copy link
Author

@murgatroid99 Hello, first of all thank you for helping me solve the problem. I don't use github very much. What is the link you sent? What should I do now, wait for npm to update the grpc-tool package?

@huan
Copy link
Contributor

huan commented Mar 17, 2021

Thanks for the upgrade to support `optional in protoc v3.15!

I saw the grpc-tools version has been bumped to v1.11, cloud you please publish this new version to NPM so that we can use it now?

@murgatroid99
Copy link
Member

I have published grpc-tools version 1.11.0 to npm.

@huan
Copy link
Contributor

huan commented Mar 17, 2021

Thank you so much, I appreciate it!

@zhushenhang
Copy link
Author

@murgatroid99 @huan Thank you for your help.Maybe I trouble you, I read the official document, optional and singular mean the same. I should just remove the optional.

@huan
Copy link
Contributor

huan commented Mar 18, 2021

@zhushenhang Thanks for the notice!

Could you please share the link from the official document which is talking about "optional and singular mean the same"?

From my understanding, the singular can not be checked for presence.

According to the How To Implement Field Presence for Proto3:

Presence in proto3 uses exactly the same syntax and semantics as in proto2. Proto3 Fields marked optional will track presence like proto2, while fields without any label (known as “singular fields”), will continue to omit presence information.

Proto3 descriptors already use LABEL_OPTIONAL for proto3 singular fields, which do not track presence.

So it seems that the singular can not be checked for the absence in proto 3?

@zhushenhang
Copy link
Author

@huan
I read the definitions of optional and singular, so my understanding is that they may mean the same thing.
singular: a well-formed message can have zero or one of this field (but not more than one). And this is the default field rule for proto3 syntax.

optional: a well-formed message can have zero or one of this field (but not more than one).

Is my understanding wrong? I use proto not for long. A lot of things I do not really understand, thank you for your guidance

@huan
Copy link
Contributor

huan commented Mar 19, 2021

I don't think they are same.

As my understanding, what is same is that they can all be absence.

The difference is that you can not check presence/absence for singular, which can be only archived by using optional.

@zhushenhang
Copy link
Author

Now I get it! thank you @huan

@zhushenhang
Copy link
Author

May I ask you another question, how to set the value of map type in static code?

message HelloRequest {
  Maps   maps = 1;
}
message Maps {
  map<string, AudioChannelCountMapBitrateOptions> formatMapChannelCount = 1;
}

message AudioChannelCountMapBitrateOptions{
  map<string, StringVec> bitrateMap = 1;
}

message StringVec{
  repeated string strings = 1;
}

I have a map structure like this.
After I generate the pb file, how do I use it?

const strVec = new messages.StringVec();
strVec.setStringsList(['1', '2']);
console.log(strVec.getStringsList());

StringVec provides a set method and a get method, so I can use it like this.But Maps and AudioChannelCountMapBitrateOptions only provide the get method, like getFormatmapchannelcountMap,getBitratemapMap.
How do I set the value of this map structure so that I can get a complete map data structure.
I think the structure like

mp2 : {
  2 : ['1' , '2']
}

But currently I only got ['1','2']

@huan
Copy link
Contributor

huan commented Mar 19, 2021

I have not used the map before, but I believe you can get the map out, and then set something on it?

@huan
Copy link
Contributor

huan commented Mar 19, 2021

@murgatroid99 Hi, it seems that the --grpc_out still has the protoc-gen-grpc hasn't been updated to support optional fields in proto3 problem with grpc-tools@1.11.0. (the --js_out works now)

Log messages:

$ node_modules/.bin/grpc_tools_node_protoc \
  -I proto/ \
  -I third-party/ \
  --js_out=import_style=commonjs,binary:./static_codegen/ \
  --grpc_out=grpc_js:./static_codegen \
  proto/wechaty/puppet/friendship.proto

wechaty/puppet/friendship.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-grpc hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--grpc_out: 
/home/huan/wechaty/grpc/node_modules/grpc-tools/bin/protoc.js:41

Reproduce repo:

https://github.com/wechaty/grpc/blob/1eb02009e8895acce60ef0e8cd37648dcf792b2e/scripts/generate-stub.sh#L39-L41

@zhushenhang
Copy link
Author

@huan yes,I tested it and the problem still exists.

@huan
Copy link
Contributor

huan commented Mar 22, 2021

@zhushenhang Thank you very much for the confirmation!

@murgatroid99 Could you please confirm this problem and re-open this issue if it does? or should we create a new issue for tracking the future fix?

@murgatroid99
Copy link
Member

I have published another fix in version 1.11.1 that addresses that error.

@zhushenhang
Copy link
Author

@murgatroid99 thank you

@huan
Copy link
Contributor

huan commented Mar 24, 2021

Thank you so much for the quick fix!

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