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

Support parsing numbers representable by int64_t but not by number as BigInts in returned messages of StructuredIrStreams. #116

Open
junhaoliao opened this issue Nov 11, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@junhaoliao
Copy link
Member

Bug

  1. The nlohmann/json library, which clp-ffi uses to serialize kv-pair log events into JSON, acknowledges there are implementations whose number types can store up only to 2^53-1 : https://json.nlohmann.me/features/types/number_handling/#number-interoperability; however, it does serialize numbers out of that range: https://github.com/nlohmann/json/blob/4a602df34e71afca2242b34c743d4cc2486cd071/tests/src/unit-serialization.cpp#L168-L183
  2. When numbers in such ranges are parsed by JSON.parse(), loss of precision is expected. Below sample code illustrates the problem:
    const myNumStr = "9007199254740993"; // 2^53 + 1
    console.log(JSON.parse(myNumStr)); // 9007199254740992
  3. Therefore, it is expected that numbers in that range, when returned as part of the messages by clp-ffi-js, cannot be accurately represented in the formatted messages displayed in the log-viewer.

Potential solutions

  1. Override the serialization / deserialization behaviours for big numbers:
    const myNum = 1234567890123456789n
    
    BigInt.prototype.toJSON = function () {
        return JSON.rawJSON(this.toString());
    };
    
    const jsonStr = JSON.stringify(myNum)
    
    console.log(jsonStr)
    
    console.log(JSON.parse(jsonStr)) // loss of precision: 1234567890123456800
    const isNumberRepresentable = value => {
      const num = Number(value);
      return (
        false === isNaN(num) &&
        Number.isInteger(num) &&
        num <= Number.MAX_SAFE_INTEGER &&
        num >= Number.MIN_SAFE_INTEGER
      );
    };
    
    console.log(JSON.parse(jsonStr, (_, __, context) => {
        if (false === isNumberRepresentable(context.source)) {
            return BigInt(context.source)
        }
        return value;
    })) // 1234567890123456789n
    
  2. Instead of JSON, return the decoded structured log event in a byte-string format with another serialization method. e.g., MessagePack.

yscope-log-viewer version

#85

Environment

The sample code was run with Node.js 22 which uses the V8 engine which is also used by any Chromium-based browsers such as Google Chrome and Microsoft Edge.

Reproduction steps

  1. Run above sample code with Node.js 22 and observe the loss of precision in the console output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant