Server fails to start with a grpc service for a proto2 proto that contains an enum.

[chris-ryan-square]

When starting, the server crashes with a java.lang.NoSuchMethodException

To replicate, use the grpc example in armeria-exemples and modify the hello.proto to:

syntax = "proto2";

package example.grpc.hello;
option java_package = "example.armeria.grpc";

service HelloService {
  rpc Hello (HelloRequest) returns (HelloReply) {}
  rpc LazyHello (HelloRequest) returns (HelloReply) {}
  rpc BlockingHello (HelloRequest) returns (HelloReply) {}
  rpc LotsOfReplies (HelloRequest) returns (stream HelloReply) {}
  rpc LotsOfGreetings (stream HelloRequest) returns (HelloReply) {}
  rpc BidiHello (stream HelloRequest) returns (stream HelloReply) {}
}

message HelloRequest {
  required string name = 1;
}

enum Foo {
  A = 1;
  B = 2;
}

message HelloReply {
  required string message = 1;
  optional Foo foo = 2;
}

Run the application and you should get an error message like this:

Exception in thread "main" java.lang.IllegalStateException: Could not find setter.
	at org.curioswitch.common.protobuf.json.ProtoFieldInfo.setValueMethod(ProtoFieldInfo.java:230)
	at org.curioswitch.common.protobuf.json.DoParse.setFieldValue(DoParse.java:452)
	at org.curioswitch.common.protobuf.json.DoParse.apply(DoParse.java:410)
	at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod$WithBody.applyCode(TypeWriter.java:708)
	at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod$WithBody.applyBody(TypeWriter.java:693)
	at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod.apply(TypeWriter.java:600)
	at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForCreation.create(TypeWriter.java:5751)
	at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:2166)
	at net.bytebuddy.dynamic.scaffold.subclass.SubclassDynamicTypeBuilder.make(SubclassDynamicTypeBuilder.java:232)
	at net.bytebuddy.dynamic.scaffold.subclass.SubclassDynamicTypeBuilder.make(SubclassDynamicTypeBuilder.java:204)
	at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase.make(DynamicType.java:3659)
	at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase$Delegator.make(DynamicType.java:3897)
	at org.curioswitch.common.protobuf.json.TypeSpecificMarshaller.buildOrFindMarshaller(TypeSpecificMarshaller.java:275)
	at org.curioswitch.common.protobuf.json.TypeSpecificMarshaller.buildAndAdd(TypeSpecificMarshaller.java:158)
	at org.curioswitch.common.protobuf.json.MessageMarshaller$Builder.build(MessageMarshaller.java:425)
	at com.linecorp.armeria.common.grpc.GrpcJsonUtil.jsonMarshaller(GrpcJsonUtil.java:62)
	at com.linecorp.armeria.common.grpc.GrpcJsonMarshallerBuilder.build(GrpcJsonMarshallerBuilder.java:70)
	at com.linecorp.armeria.common.grpc.GrpcJsonMarshaller.of(GrpcJsonMarshaller.java:45)
	at com.linecorp.armeria.internal.shaded.guava.collect.CollectCollectors.lambda$toImmutableMap$6(CollectCollectors.java:184)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
	at java.base/java.util.stream.DistinctOps$1$2.accept(DistinctOps.java:174)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at com.linecorp.armeria.server.grpc.FramedGrpcService.<init>(FramedGrpcService.java:135)
	at com.linecorp.armeria.server.grpc.GrpcServiceBuilder.build(GrpcServiceBuilder.java:746)
	at example.armeria.grpc.Main.configureServices(Main.java:57)
	at example.armeria.grpc.Main.newServer(Main.java:41)
	at example.armeria.grpc.Main.main(Main.java:23)
Caused by: java.lang.NoSuchMethodException: example.armeria.grpc.Hello$HelloReply$Builder.setFooValue(int)
	at java.base/java.lang.Class.getDeclaredMethod(Class.java:2675)
	at org.curioswitch.common.protobuf.json.ProtoFieldInfo.setValueMethod(ProtoFieldInfo.java:228)
Caused by: java.lang.NoSuchMethodException: example.armeria.grpc.Hello$HelloReply$Builder.setFooValue(int)

The handling of enums seems to be incorrect. There should be no error and the service should work.

[jrhee17]

First off, I believe this is a regression and we probably want armeria to support both proto2 and proto3 if possible.

Cause
Analyzing the stacktrace, it seems like the server fails to start up when trying to initialize a json marshaller for the service.

armeria/grpc/src/main/java/com/linecorp/armeria/server/grpc/FramedGrpcService.java

Lines 128 to 136 in 21be2a4

	if (supportedSerializationFormats.stream().noneMatch(GrpcSerializationFormats::isJson)) {
	jsonMarshallers = ImmutableMap.of();
	} else {
	jsonMarshallers =
	registry.services().stream()
	.map(ServerServiceDefinition::getServiceDescriptor)
	.distinct()
	.collect(toImmutableMap(ServiceDescriptor::getName, jsonMarshallerFactory));
	}

Workaround

If we don't initialize the server with serialization formats of json type, other functionality should work fine.

armeria/examples/grpc/src/main/java/example/armeria/grpc/Main.java

Line 52 in 21be2a4

.supportedSerializationFormats(GrpcSerializationFormats.values())

.supportedSerializationFormats(GrpcSerializationFormats.PROTO, GrpcSerializationFormats.PROTO_WEB, GrpcSerializationFormats.PROTO_WEB_TEXT) // or just the serialization formats needed

For users who don't need json marshalling support, this may be a valid workaround (verified by going through the code + integration tests)

Additional notes
We probably want to add integration tests to ensure this type of regression doesn't occur for proto2 as well.

Additionally, we may look into protobuf-jackson and look for the root cause (and hopefully add support for proto2)

[jrhee17]

curioswitch/protobuf-jackson#8
Not sure how trivial the fix is, but created an issue.

Independent of above, we might want to find a way to conditionally ignore proto2 for json marshalling so that server startup isn't blocked