grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.51k stars 766 forks source link

Generated JS files use number for int64 fields, causing wrong values to be sent due to precision loss. #1229

Open Kapps opened 2 years ago

Kapps commented 2 years ago

When generating a JS file for a proto file using the protoc-gen-grpc-web plugin, 64 bit integers are treated as the JavaScript number type. As numbers in JS are limited precision floating point, they can not support large 64 bit integers without precision loss. As such, it is not possible to send a number such as 9024037547368883040 over GRPC.

Example (named foo.proto):

syntax = "proto3";

message foo {
        int64 bar = 1;
}

Compiled via: protoc -I=. foo.proto --js_out=import_style=commonjs:. --grpc-web_out=import_style=typescript,mode=grpcweb:.

The generated output for the TypeScript bindings includes the following. Attempting to pass in the original number will give a Typescript error due to loss of precision.

  getBar(): number;
  setBar(value: number): foo;

If the type system is circumvented, such as by passing a string cast to any, the remote server appears to receive the wrong value due to improper loss of precision (9024037547368883200 instead of 9024037547368883040).

I haven't been able to find any workarounds to use to avoid this issue yet.

Kapps commented 2 years ago

Ah, looks like one can change the declaration to int64 bar = 1 [jstype = JS_STRING]; to generate a string mapping, preventing loss of precision.

I would argue that the default should be a string or custom data type, as the current default will result in data corruption if one is using numbers larger than int32. Which one probably is, if they're explicitly declaring an int64.

sampajano commented 2 years ago

Thanks for the report!

Yeah it's certainly unexpected to have precision loss here.. But probably needs a bit more digging to see if it's a client v.s. server bug, or a limitation of the current wire protocol (would doubt that's the case).

jasonclg commented 2 years ago

I guess the following code produced the number type in JS file https://github.com/grpc/grpc-web/blob/master/javascript/net/grpc/web/generator/grpc_generator.cc#L314

Is there any plan to use BigInt instead to replace the number type and preserve the precision?

sampajano commented 2 years ago

Ahh! That's a good pointer! Thanks!

Although actually, i believe that code might be only related wrt generating the .d.ts typescript API, but not related to how the number is actually encoded -- which afaiu is handled by the google-protobuf javascript support itself.

And i looked up some old code in there (the JS impl is recently deleted due to migration), and it seems that the code does have the intention to preserve precision:

https://github.com/protocolbuffers/protobuf/blob/19275202783b1f6e73130b2d1edfaf66a26ec0e4/js/binary/writer.js#L501-L506

https://github.com/protocolbuffers/protobuf/blob/19275202783b1f6e73130b2d1edfaf66a26ec0e4/js/binary/encoder.js#L83-L99

fengpeng commented 2 years ago

HI~ The problem exists.

My proto file

message Response {
    repeated int64 orgs = 101;
}

Compiled via:

--js_out=import_style=commonjs,binary:. --grpc-web_out=import_style=commonjs+dts,mode=grpcwebtext

The genrated output

getOrgsList(): Array<number>;
setOrgsList(value: Array<number>): Response;

When call getOrgsList method, value loses precision. (-_-)ゞ゛

kimbrowne commented 1 year ago

Is there any resolution to this problem. We rely on int64 values and they are rounding for large numbers.

For example 1234552132131231239 is returned as 1234552132131231200.
I have tested with a golang client to a grpc server and this does not happen. I have tried both Envoy and Improbable proxies and it makes no difference. This suggests that the problem is purely a grpc-web issue.

If it helps I can provide a grpc-web example together with the golang server and client and even the improbable proxy with the settings.

The comments from the pb files shows // Code generated by protoc-gen-grpc-web. DO NOT EDIT. // versions: // protoc-gen-grpc-web v1.4.2 // protoc v3.20.3