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

Cannot use stdtime and stdduration extensions in query parameters in gRPC-gateway #420

Closed
johanbrandhorst opened this issue Jul 1, 2018 · 6 comments

Comments

@johanbrandhorst
Copy link
Member

Hi!

We ran into this recently at work, but I'm aware there's not much GoGo Protobuf can do about it, so I'm just raising it as a reference point for others who may run into this problem. Essentially:

IF you use the gRPC-Gateway
and
IF you use gogoproto.stdtime or gogoproto.stdduration
and
IF you use either of the aforementioned types in a GET request with no body

then you will see the following error in reply to your request:

unsupported field type reflect.Value

This, rather cryptically, points to a problem in the gRPC-Gateway query parameter resolution. The error comes from https://github.com/grpc-ecosystem/grpc-gateway/blob/fa90cfb8fa521cb2df47e090ea21ad310a188d07/runtime/query.go#L294, but the issue is really that that method is not equipped to handle a target field of type *time.Time or *time.Duration.

I will attempt to fix this in the gRPC-Gateway, but I'm not terribly hopeful that it will be merged, since it only affects users of GoGo Protobuf.

The workaround, of course, is to not use gogoproto.stdtime or gogoproto.stdduration for fields that are parsed from the query parameters in your gRPC-Gateway requests.

@johanbrandhorst johanbrandhorst changed the title Cannot use stdtime and stdduration extensions in query parameters in grpc-gateway Cannot use stdtime and stdduration extensions in query parameters in gRPC-gateway Jul 1, 2018
@johanbrandhorst
Copy link
Member Author

Fix submitted to grpc-gateway

@awalterschulze
Copy link
Member

Wow sorry I've been a little sick, so I haven't caught up with everything yet. This is really awesome work. The pull request to grpc-gateway looks great. I really hope they accept it.
Keeping the gogoprotobuf dependency out was key to it having a chance, great call :)

@tmc
Copy link

tmc commented Jul 5, 2018

Happily merged. :)

@awalterschulze
Copy link
Member

Wow this is awesome work thank you so much @johanbrandhorst and @tmc
I really appreciate this :D

@awalterschulze
Copy link
Member

I guess we can close this issue now?

@johanbrandhorst
Copy link
Member Author

Yes, agreed

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