-
Notifications
You must be signed in to change notification settings - Fork 806
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
Comments
Fix submitted to grpc-gateway |
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. |
Happily merged. :) |
Wow this is awesome work thank you so much @johanbrandhorst and @tmc |
I guess we can close this issue now? |
Yes, agreed |
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
orgogoproto.stdduration
and
IF you use either of the aforementioned types in a
GET
request with no bodythen you will see the following error in reply to your request:
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
orgogoproto.stdduration
for fields that are parsed from the query parameters in your gRPC-Gateway requests.The text was updated successfully, but these errors were encountered: