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

0 feedback on wrong request fields when using .proto #105

Closed
bwplotka opened this issue Jun 2, 2019 · 6 comments
Closed

0 feedback on wrong request fields when using .proto #105

bwplotka opened this issue Jun 2, 2019 · 6 comments

Comments

@bwplotka
Copy link

bwplotka commented Jun 2, 2019

Hi and thanks for awesome project! 👋

I guess it is thanks of forward and backward protobuf compatibility as you never know what server actually is implementing, but I just spent hour of crafting request in JSON only to get frustrated and coded Golang client in 2 minutes to find out that I was doing fields in snake_case instead of camelCase 🤦‍♂️

The request was coming through and even funnier -> It was succesful in some way due to default values.

Any way we can improve this? Either by validation or some tooling on how to craft the request? I guess you are parsing against given proto so it's clear that user uses field that does not exists? (:

@jhump
Copy link
Contributor

jhump commented Jun 3, 2019

@bwplotka, grpcurl does accept snake case in addition to camel case. Or, more precisely, if you give it a field name that does not match any of the message's field names in JSON format, it will assume you are using "original names" (e.g. the name of the field in the proto, which might be snake case) and check to see if the name matches that way.

Are you saying you were getting an error because the snake-case names were not being parsed? Perhaps you could provide some more info to demonstrate the problem, like a simple proto file and example invocation that do not behave as expected?

BTW, if you simply give a name that is not in the message, you will get an error message:

$ grpcurl -d '{"foo":"bar"}' api.grpc.me:443 listennotes.v1.Podcasts.GetEpisodes

Error invoking method "listennotes.v1.Podcasts.GetEpisodes": error getting request data: 
message type listennotes.v1.EpisodesRequest has no known field named foo

@w3irdrobot
Copy link

Where is this translation done? I can't seem to be able to get grpcurl to automatically transform my snake_case'd keys to the attributes in the protobuf message.

@jhump
Copy link
Contributor

jhump commented Jul 24, 2019

@searsaw, I don't really understand the question. Are you looking for where in the code this is done? I believe this is happening inside the JSON unmarshaling in github.com/jhump/protoreflect/dynamic.

Do you have a simple proto file and invocation of grpcurl that demonstrates the issue you are seeing? If I can repro the issue, I can probably provide more help getting it to work (or, if it's a bug, work on a patch).

@w3irdrobot
Copy link

user.proto

syntax = "proto3";
package main;

service Users {
  rpc Create(User) returns (User) {}
}

message User {
  string ID = 1;
  string FirstName = 2;
  string LastName = 3;
  string Email = 4;
}

main.go

package main

import (
	"context"
	"log"
	"net"

	"github.com/segmentio/ksuid"

	"google.golang.org/grpc"
)

func main() {
	lis, err := net.Listen("tcp", ":3001")
	if err != nil {
		log.Fatal("Error occurred getting tcp listener", err)
	}

	var options []grpc.ServerOption
	gServer := grpc.NewServer(options...)
	service := &Server{}
	RegisterUsersServer(gServer, service)

	log.Println("gRPC service started on :3001")
	log.Fatal(gServer.Serve(lis))
}

type Server struct{}

func (s *Server) Create(ctx context.Context, user *User) (*User, error) {
	user.ID = ksuid.New().String()
	return user, nil
}

deploy.sh

#!/bin/bash

grpcurl \
  -plaintext \
  -proto user.proto \
  -d '{"first_name": "George", "last_name": "Foreman", "email": "george.foreman@ilikegrills.com"}' \
  localhost:3001 main.Users.Create

When build the proto file into Golang code, run the server, and run the deploy.sh script, i get the following message.

Error invoking method "main.Users.Create": error getting request data: message type main.User has no known field named first_name

@jhump
Copy link
Contributor

jhump commented Jul 27, 2019

@searsaw, I think there is some confusion around the JSON format for protobufs as well as the usual naming conventions for identifiers in your .proto source files.

When I said that snake case is supported, I meant that the actual proto name is supported. And the typical convention for field names in proto is that they are snake case:
https://developers.google.com/protocol-buffers/docs/style#message-and-field-names

In your example, the original name is not snake case -- it is upper-camel-case. So the correct field name to use is FirstName.

If you do use snake-case in your proto source (e.g. first_name), it will still generate Go code using upper-camel-case (e.g. the field in the generated struct is still FirstName). You can then use either the JSON name of the field or the original name (which is snake case). The JSON name of the field comes from its json_name option; or, for fields that have no such option, it is the camel-case version of the name (e.g. firstName).

Here is more info on the JSON format for protocol buffers. This is what grpcurl expects as input:
https://developers.google.com/protocol-buffers/docs/proto3#json

@w3irdrobot
Copy link

Thanks for this. Yeah it was just a misunderstanding with how I wrote my proto files. Once I started using the snake_case attribute names in my proto file, it all started working correctly. Thank you for the help!

@jhump jhump closed this as completed Sep 27, 2019
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

No branches or pull requests

3 participants