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

Allow missing extension ranges since not all messages are extended #497

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kvarnefalk
Copy link

Background & Problem

I'm working on a java grpc service that have a field of type google.protobuf.DescriptorProto from descriptor.proto in the response message.

When running grpcurl, I'm hit with the following fatal error message:

Error invoking method "MyDescriptorMethod": error resolving server extensions for message MyDescriptorMethodResponse: failed to query for extensions of type google.protobuf.FeatureSet: Message not found: google.protobuf.FeatureSet

If I allow the grpcurl to continue past that error, I'm also getting errors on google.protobuf.ExtensionRangeOptions.

For context, here's the snippet where FeatureSet is reserving extensions:

message FeatureSet {
  ...
  extensions 1000;  // for Protobuf C++
  extensions 1001;  // for Protobuf Java
  extensions 1002;  // for Protobuf Go

  extensions 9990;  // for deprecated Java Proto1

  extensions 9995 to 9999;  // For internal testing
  extensions 10000;         // for https://github.com/bufbuild/protobuf-es
}

Difference in grpcurl and java-grpc

grpcurl calls desc.MessageDescriptor.GetExtensionRanges(), which seems to return a list of the extension ranges above. However, java-grpc is calling FileDescriptor.getExtensions() ref. From what I can tell, getExtensions() is only returning a value if the message in question is extended by another message.

For example, there are multiple other messages in descriptor.proto that also declare extension ranges in the same way, that are not failing. However, for those, there are other messages extending the message. See envoy/annotations/deprecation.proto:

extend google.protobuf.FieldOptions {
  bool disallowed_by_default = [18]
  string deprecated_at_minor_version = 157299826;
}

Solution?

So it seems like, for fetchAllExtensions to properly work, all messages with an extension range declared must have another message extending that message? That feels like a thing that should not necessarily always be true. Rather we would allow no extensions to be found on the message, since that is expected if there are only extension ranges declared (and no other messages extending it).

Please advise if this solution makes sense, or if you think it's rather grpc-java that should update their reflection service to account for non-extended messages.

Thanks for a great tool!

@jhump
Copy link
Contributor

jhump commented Dec 11, 2024

@Kvarnefalk, this is probably better off as an issue, to discuss.

What was the actual command-line used? Were you using server reflection or providing proto sources or descriptor sets? If using server reflection, what was the actual RPC response from the server? I guess that isn't clear from the error message, which is unfortunate. But I think it would be better to handle this elsewhere, like in the source's implementation of AllExtensionsForType (which should likely ignore "not found" errors and just return back an empty list of extensions).

@jhump
Copy link
Contributor

jhump commented Dec 11, 2024

Also, what version of grpcurl are you using and, if using server reflection, what version of grpc-java are you using in the server?

@Kvarnefalk
Copy link
Author

Happy to reopen this as an issue if it makes more sense, or rewrite it to make AllExtensionsForType ignore the not-found error. We are using server reflection. For context, this is the reflection service in grpc-java

This is the command I'm using:

grpcurl -plaintext -max-time 1000 -d ''  localhost:5990 MyService/MyMethod

And here's the rpc error response (looks like it's coming from here in grpc-java)

ErrorCode = 5
ErrorMessage = "Type not found."

Using grpcurl 1.9.2 and io-grpc:grpc-services 1.67.1

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

Successfully merging this pull request may close these issues.

2 participants