-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
--swagger_out plugin translates proto type int64 to string in Swagger specification #219
Comments
Hi, |
@tartale, gomodules@f7d82dd fixes (reverses?) this behavior. We generate a json schema by post processing swagger spec. So, we had to revert this to generate proper json schema. |
Will this be addressed? |
Sorry, I thought I'd commented on this before. This is working as intended. gRPC gateway follows the serialization rules of proto3 and the proto3 spec says that int64, fixed64, and uint64 are all strings. This is the case because of IEEE 754 Double precision which JS implements using a 52 bit mantissa (1 for sign, and 11 for exponent). This means the largest number that JS can represent without losing precision is 2^52. Try it out: > (Math.pow(2,53) + 0) == (Math.pow(2,53) + 1)
true So, rather than pretending that Javascript can handle it (and lose data) proto3 prefers to have the client interact with strings. Fewer footguns == better libraries. If you want to interact with int64 as numbers, there are a bunch of JS bigint libraries out there that you can use to parse the string, perform whatever operations you want and get back a string. That would probably be a better way to handle this. Note that int32 doesn't suffer from the same issue. If you don't have the pressure to operate on numbers > 2^32 that might be a good fix. |
Thanks for the clarity! |
@achew22 , Thanks for your explanation. We generate a json schema that is used for frontend validation. In that scenario, the data format is more important than actual data. So, in that case, we should be validating it against an "integer". Am I right? |
Unfortunately, that is not the case. Let's walk through a simple example and see what happens. Here is a non comprehensive list of my assumptions:
Okay, let's make a proto like this:
Now let's send some data through grpc-gateway using that proto. [Code left as an exercise for the reader] You should get out on the other side assuming you transmit TestProto.newBuilder().setA(1L).setB(2L).build() (using Java syntax but any language would be fine). {"a":"1","b":"2"} And when you transmit stuff back to the server {"a":"1","b":"9007199254740993"} On your server you should get a response that is an int64 and contains all the bits of precision without losing anything. If you get something else, can you please reopen this so I can take a look at it because it is a bug. |
@achew22 , Will grpc-gateway fail if my frontend sends Edit: After reading the spec, the answer is it will work ok. For json -> protoc, string or int both are ok. For protoc->json, protoc will only product strings. |
I don't know. It's not specified behavior. Wanna try it out and see what happens? It might work, but I can't promise even if it does that it'll work forever. |
@achew22 , it seems to be specified int the notes from your link above. |
Cool! Then go for it. If it works that's wonderful, but don't forget the aforementioned imprecision issues. |
If you really want to have the type as number and you're 100% sure the data isn't ever going to be > 2^32, why not use an int32? |
The main source of int64 is timestamp. The other places we used int64 could probably be changed to int32. We will probably do that now that we are aware of this issue. |
Ah, have you considered using a google.protobuf.Timestamp? That will get detected by the swagger generator and you'll be able to interact with it directly as a Date object. What do you think about that? |
@achew22 , How does google.protobuf.Timestamp convert into Json? My guess is that it becomes 2 fields (sec, nano)? If that is the case, it is unacceptable for us. We use PHP, Java, Javascript and GO. Using epoch sec as int64 is the easiest way to keep our sanity. |
Timestamp is special and very nice. It's actually specified on that same page we keep sending back and forth to each other 😉. Timestamp string "1972-01-01T10:00:20.021Z" Uses RFC 3339, where generated output will always be Z-normalized and uses 0, 3, 6 or 9 fractional digits. I just switched my thing to use it a couple of days ago. Works like a charm! |
hi, anyone could help to explain why swagger translates proto type uint32 to int64 |
Could you join the #grpc-gateway channel on Gophers slack? https://invite.slack.golangbridge.org/. This isn't the right place for this sort of debugging. |
The --swagger_out plugin yields a swagger definition that translates an int64 protobuf type to a string, even though the Swagger 2.0 OpenAPI specification allows for a signed 64-bit type (type
integer
; formatint64
). This might be an oversight and/or copy/paste error, as this code section seems to indicate that this was done intentionally foruint64
values.Example proto file:
Running
protoc
with thegrpc-gateway
plugin and the--swagger_out
output currently yields this swagger definition:The text was updated successfully, but these errors were encountered: