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

Snake case protobuf enums fail to compile. #2776

Closed
juchem opened this issue Jun 29, 2022 · 13 comments · Fixed by #2826
Closed

Snake case protobuf enums fail to compile. #2776

juchem opened this issue Jun 29, 2022 · 13 comments · Fixed by #2826

Comments

@juchem
Copy link

juchem commented Jun 29, 2022

🐛 Bug Report

When declaring snake cased enums in a proto file, grpc-gateway fails to render the correct go type name for it, writing the type name as-is instead of converting to camel case.

GRPC correctly uses property GoIdent of protogen.Enum to produce the proper type name.

To Reproduce

  1. write a protobuf file with snake case enums
  2. write a request message with a member what typed with said enum
  3. add a service with an rpc that uses said request message
  4. annotate rpc so that the what member is part of the REST URL (e.g.: /api/{what})
  5. generate grpc-gateway sources using the protobuf plugin
  6. compile go code that uses the generated sources

A minimal repro is available as a docker image source in repro.tar.gz. To reproduce, run:

docker build -t bug-repro . && docker run -it --rm bug-repro

Contents of the attached archive:

-rw-r--r-- 1 mj mj  631 Jun 29 09:21 api.proto
-rw-r--r-- 1 mj mj 1703 Jun 29 09:18 Dockerfile
-rwxr-xr-x 1 mj mj  395 Jun 29 09:14 entrypoint.sh
-rw-r--r-- 1 mj mj   59 Jun 29 09:03 repro.go

Expected behavior

The source file generated by the grpc-gateway plugin should compile successfully.

Actual Behavior

Compilation fails:

# bug/report
../api/bug/report/api.pb.gw.go:109:29: undefined: snake_case_enum_value
../api/bug/report/api.pb.gw.go:114:18: undefined: snake_case_enum
../api/bug/report/api.pb.gw.go:138:29: undefined: snake_case_enum_value
../api/bug/report/api.pb.gw.go:143:18: undefined: snake_case_enum

Your Environment

An isolated Debian-based container with go 1.18.

@johanbrandhorst
Copy link
Collaborator

Hi, thanks for your issue! Is this a problem with lower cased enum values specifically? I've never seen anyone use anything but SCREAMING_SNAKE_CASE for enum values in protobuf, which I guess is why we haven't seen this issue before. Could you please share the example files inline in the issue instead of as a tar file? I'm not going to download and open a tar file from a stranger on the internet, sorry.

@juchem
Copy link
Author

juchem commented Jun 29, 2022

@johanbrandhorst that's undestandable.

The probelm is with the enum itself, not the values.

rpc_1 compiles fine since it references the camel cased enum, even though the values themselves aren't uppercase. It's easy to test it by removing rpc_2.

rpc_2, on the other hand, fails to compile because the enum type name is rendered using the proto type name, not the Go type name.

Let me include the proto file here.

api.proto

syntax = "proto3";

option go_package = "bug/report";

import "google/api/annotations.proto";

service identity {
  rpc rpc_1(request_1) returns (response_1) {
    option (google.api.http) = {
      get: "/api/v1/rpc_1/{what}"
    };
  }

  rpc rpc_2(request_2) returns (response_2) {
    option (google.api.http) = {
      get: "/api/v1/rpc_2/{what}"
    };
  }
}

enum CamelCaseEnum {
  value_1_0 = 0;
  value_1_1 = 1;
}

message request_1 {
  CamelCaseEnum what = 1;
}

message response_1 {
}

enum snake_case_enum {
  value_2_0 = 0;
  value_2_1 = 1;
}

message request_2 {
  snake_case_enum what = 1;
}

message response_2 {
}

@juchem
Copy link
Author

juchem commented Jun 29, 2022

I'll include the remaining text files just in case you decide to repro using the docker image, so you don't have to untar anything:

repro.go

package main

import (
  _ "bug/report"
)

func main() {
}

entrypoint.sh

#!/bin/bash -e

export PATH="${PATH}:$(go env GOPATH)/bin"

# https://github.com/googleapis/googleapis.git
protoc \
  -I /srv/googleapis \
  -I /srv/protobuf/src \
  -I /srv/src \
  --go_out=/srv/api \
  --go-grpc_out=/srv/api \
  --grpc-gateway_out=/srv/api \
  --grpc-gateway_opt="logtostderr=true" \
  /srv/src/api.proto

go build -modfile=/srv/src/go.mod -o /srv/out/main /srv/src/repro.go

Dockerfile

FROM bitnami/minideb:unstable

ENV DEBIAN_PRIORITY=critical
ENV DEBIAN_FRONTEND=noninteractive

RUN apt-get update && apt-get install -y --no-install-recommends \
  ca-certificates \
  curl git \
  protobuf-compiler \
  golang

RUN go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
RUN go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
RUN go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway@latest
RUN go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@latest

RUN git clone --depth=1 https://github.com/googleapis/googleapis.git /srv/googleapis
RUN git clone --depth=1 https://github.com/protocolbuffers/protobuf.git /srv/protobuf

RUN mkdir -p /srv/out /srv/src /srv/api/bug/report

RUN cd /srv/src && go mod init main && go mod edit \
  --require 'bug/report@v0.0.0' \
  --replace 'bug/report=/srv/api/bug/report'

RUN cd /srv/api/bug/report && go mod init bug/report

WORKDIR /srv/src

RUN go get google.golang.org/genproto@v0.0.0-20220628213854-d9e0b6570c03
RUN go get golang.org/x/net@v0.0.0-20220127200216-cd36cc0744dd
RUN go get golang.org/x/sys@v0.0.0-20211216021012-1d35b9e2eb4e
RUN go get github.com/golang/protobuf@v1.5.2
RUN go get github.com/grpc-ecosystem/grpc-gateway/v2@v2.10.3
RUN go get golang.org/x/net@v0.0.0-20220127200216-cd36cc0744dd
RUN go get golang.org/x/sys@v0.0.0-20211216021012-1d35b9e2eb4e
RUN go get golang.org/x/text@v0.3.7
RUN go get google.golang.org/genproto@v0.0.0-20220628213854-d9e0b6570c03
RUN go get google.golang.org/grpc@v1.47.0
RUN go get google.golang.org/protobuf@v1.28.0

ENTRYPOINT [ "/srv/entrypoint.sh" ]
COPY entrypoint.sh /srv

COPY api.proto /srv/src
COPY repro.go /srv/src

@johanbrandhorst
Copy link
Collaborator

Thanks for adding those files :). This should be pretty easy to reproduce in our own repository by adding an ugly looking enum to one of our example Protobuf files, e.g. https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/a_bit_of_everything.proto. Then rerunning generation should reproduce the error. From there, we'll have to fix it or our build won't succeed. Would you be interested in trying to fix this?

@juchem
Copy link
Author

juchem commented Jun 29, 2022 via email

@jbaxx
Copy link
Contributor

jbaxx commented Jul 28, 2022

I'm interested in trying to fix this.

TL; DR

I've been able to reproduce the issue, looks like identifiers for enums in the *.pb.gw.go file doesn't match those in the *.pb.go file when enums are snake-cased. Also, I'd appreciate some guidance on the path to test and fix as mentioned in the fix proposal below.

Reproducing the issue

I was able to reproduce the issue as well by:

  1. Adding snake-cased enums to a_bit_of_everything.proto
  2. Re-generating the .pb.* files with buf.
  3. Trying to go build or go run any main package that references the examplepb package generated from a_bit_of_everythingpb.proto like examples/internal/cmd/example-gateway-server/main.go

Enums and service added to a_bit_of_everything.proto:

service SnakeEnum {

    rpc SnakeEnum(SnakeEnumRequest) returns (SnakeEnumResponse) {
        option (google.api.http) = {
            get: "/v1/example/snake/{who}/{what}"
        };
    }
}

enum snake_case_enum {
    value_c = 0;
    value_d = 1;
}

enum snake_case_0_enum {
    value_e = 0;
    value_f = 1;
}

message SnakeEnumRequest {
    snake_case_enum what = 1;
    snake_case_0_enum who = 2;
}

message SnakeEnumResponse {}

go build and/or go run output:

# github.com/grpc-ecosystem/grpc-gateway/v2/examples/internal/proto/examplepb
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2639:29: undefined: snake_case_0_enum_value
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2644:17: undefined: snake_case_0_enum
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2651:29: undefined: snake_case_enum_value
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2656:18: undefined: snake_case_enum
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2680:29: undefined: snake_case_0_enum_value
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2685:17: undefined: snake_case_0_enum
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2692:29: undefined: snake_case_enum_value
examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go:2697:18: undefined: snake_case_enum

Investigation

So far, looks like the proto generator protoc-gen-go creates the types/variables's identifiers related to the enums Camel-cased in the examples/internal/proto/examplepb/a_bit_of_everything.pb.go file:

type SnakeCaseEnum int32

const (
	SnakeCaseEnum_value_c SnakeCaseEnum = 0
	SnakeCaseEnum_value_d SnakeCaseEnum = 1
)

// Enum value maps for SnakeCaseEnum.
var (
	SnakeCaseEnum_name = map[int32]string{
		0: "value_c",
		1: "value_d",
	}
	SnakeCaseEnum_value = map[string]int32{
		"value_c": 0,
		"value_d": 1,
	}
)

// ...

type SnakeCase_0Enum int32

const (
	SnakeCase_0Enum_value_e SnakeCase_0Enum = 0
	SnakeCase_0Enum_value_f SnakeCase_0Enum = 1
)

// Enum value maps for SnakeCase_0Enum.
var (
	SnakeCase_0Enum_name = map[int32]string{
		0: "value_e",
		1: "value_f",
	}
	SnakeCase_0Enum_value = map[string]int32{
		"value_e": 0,
		"value_f": 1,
	}
)

While the grpc-gateway proto generator protoc-gen-grpc-gateway generates the derived enum identifiers without Camel-casing (no modification to the identifier actually) within the following method in the examples/internal/proto/examplepb/a_bit_of_everything.pb.go file:

func local_request_SnakeEnum_SnakeEnum_0(ctx context.Context, marshaler runtime.Marshaler, server SnakeEnumServer, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) {
	var protoReq SnakeEnumRequest
	var metadata runtime.ServerMetadata

	var (
		val string
		e   int32
		ok  bool
		err error
		_   = err
	)

	val, ok = pathParams["who"]
	if !ok {
		return nil, metadata, status.Errorf(codes.InvalidArgument, "missing parameter %s", "who")
	}

	e, err = runtime.Enum(val, snake_case_0_enum_value)
	if err != nil {
		return nil, metadata, status.Errorf(codes.InvalidArgument, "type mismatch, parameter: %s, error: %v", "who", err)
	}

	protoReq.Who = snake_case_0_enum(e)

	val, ok = pathParams["what"]
	if !ok {
		return nil, metadata, status.Errorf(codes.InvalidArgument, "missing parameter %s", "what")
	}

	e, err = runtime.Enum(val, snake_case_enum_value)
	if err != nil {
		return nil, metadata, status.Errorf(codes.InvalidArgument, "type mismatch, parameter: %s, error: %v", "what", err)
	}

	protoReq.What = snake_case_enum(e)

	msg, err := server.SnakeEnum(ctx, &protoReq)
	return msg, metadata, err

}

Causing the derived enum identifiers in the .pb.gw.go to be undeclared, and a compilation issue.

Identifiers mismatch summary:

.pb.go identifiers .pb.gw.go identifiers
SnakeCase_0Enum snake_case_0_enum
SnakeCaseEnum_value snake_case_0_enum_value
SnakeCaseEnum snake_case_enum
SnakeCaseEnum_value snake_case_enum_value

Fix proposal

I suspect the fix is to apply the Camel-casing function to the enum names as is done for the service and method names in the applyTemplate function.

But in this case, the difference is that looping through the service and methods' names (+ casing) is done in the code, while looping through the enum's names is done within the template.

Could you point if I'm in the right direction and hint a path for the fix?
Also some guidance on testing this will be great, thanks!

@johanbrandhorst
Copy link
Collaborator

Hi @jbaxx, thanks for that thorough analysis! I think you're already well on the way there, adding a failing example to a_bit_of_everything.proto is as much of an integration test as we really need for the generator. In terms of fixing it, we could always add the CamelCase function into the template rendering so that we can call it within the template. How does that sound?

@juchem
Copy link
Author

juchem commented Aug 23, 2022

@johanbrandhorst are there any plans to put out a release with this bug fix in the short term?

@johanbrandhorst
Copy link
Collaborator

I've just tagged v2.11.3: https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.11.3

@juchem
Copy link
Author

juchem commented Aug 24, 2022

@johanbrandhorst go install doesn't seem to find the binaries. Is the publishing a batch job that will eventually fix itself or a manual process?

Output:

$ go install -v -x github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@v2.11.3
# get https://proxy.golang.org/github.com/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/@v/v2.11.3.info: 404 Not Found (0.060s)
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/@v/v2.11.3.info: 404 Not Found (0.060s)
# get https://proxy.golang.org/github.com/grpc-ecosystem/@v/v2.11.3.info: 404 Not Found (0.068s)
# get https://proxy.golang.org/github.com/@v/v2.11.3.info: 404 Not Found (0.103s)
$ go install -v -x github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway@v2.11.3
# get https://proxy.golang.org/github.com/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/@v/v2.11.3.info
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/@v/v2.11.3.info: 404 Not Found (0.065s)
# get https://proxy.golang.org/github.com/@v/v2.11.3.info: 404 Not Found (0.107s)
# get https://proxy.golang.org/github.com/grpc-ecosystem/@v/v2.11.3.info: 404 Not Found (0.106s)
# get https://proxy.golang.org/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway/@v/v2.11.3.info: 404 Not Found (0.108s)

@johanbrandhorst
Copy link
Collaborator

It can take a few hours for the Go proxy to pick up a new version, I just ran the command you pasted here locally and it worked fine for me! Let me know if it's still not working.

@juchem
Copy link
Author

juchem commented Aug 25, 2022

I just ran it and still the same 404 output. I double checked just in case and the binary was not downloaded

@juchem
Copy link
Author

juchem commented Aug 25, 2022

@johanbrandhorst FYI, for whatever reason, after cleaning the contents of GOPATH the install command finally worked. The problem was on my end, so please disregard my previous comment.

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

Successfully merging a pull request may close this issue.

3 participants