Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

ipfs-http-client & go-ipfs - BigInt issue with stats.bw() #3782

Closed
jurelik opened this issue Jul 31, 2021 · 3 comments · Fixed by #3798
Closed

ipfs-http-client & go-ipfs - BigInt issue with stats.bw() #3782

jurelik opened this issue Jul 31, 2021 · 3 comments · Fixed by #3798
Labels
need/triage Needs initial labeling and prioritization

Comments

@jurelik
Copy link

jurelik commented Jul 31, 2021

  • Version:
    ipfs-http-client: 51.0.1
    go-ipfs: 0.9.1

  • Platform:
    Darwin jure.local 18.7.0 Darwin Kernel Version 18.7.0: Tue Jun 22 19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64 x86_64

  • Subsystem:
    stats.bw

Severity:

Medium - A non-essential functionality does not work, performance issues, etc.

Description:

I suspect this issue is related to #3726 and it's subsequent fix in #3735. I'm getting the same error as the user in the OP, namely:

The number xxxx.yyyy cannot be converted to a BigInt because it is not an integer

I have fixed the issue by changing the code in ipfs-http-client/src/stats/bw.js by following the lead in #3735 like so:

  async function * bw (options = {}) {
    const res = await api.post('stats/bw', {
      timeout: options.timeout,
      signal: options.signal,
      searchParams: toUrlSearchParams(options),
      headers: options.headers,
      transform: (stats) => ({
        totalIn: BigInt(Math.round(stats.TotalIn)),
        totalOut: BigInt(Math.roundstats.TotalOut)),
        rateIn: BigInt(Math.round(stats.RateIn)),
        rateOut: BigInt(Math.round(stats.RateOut))
      })
    })

    yield * res.ndjson()
  }

This works fine as a quick fix but I'm not sure if:

  • Math.Round is the way you want to approach solving this
  • that's an issue that should be solved in go-ipfs or ipfs-http-client.

If it's something you would like to have fixed in ipfs-http-client I'd be happy to submit a PR, but I can't help you with the golang side of things. Any advice would be much appreciated!

Steps to reproduce the error:

Set up ipfs-http-client to work with the go-ipfs npm package and run:

for await (const stats of ipfs.stats.bw({ poll: true, interval: '1s' })) {
  console.log(stats.rateIn);
}
@jurelik jurelik added the need/triage Needs initial labeling and prioritization label Jul 31, 2021
@welcome
Copy link

welcome bot commented Jul 31, 2021

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

  • "Priority" labels will show how urgent this is for the team.
  • "Status" labels will show if this is ready to be worked on, blocked, or in progress.
  • "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

@achingbrain
Copy link
Member

These values and others are returned by the go-IPFS HTTP API as int64, float64, etc as JSON. From what I can tell the JSON spec doesn't define a max size for numbers so in theory it's valid but in practice JSON.parse will return regular numbers so the value is limited to 53 bits because lol Javascript "numbers".

Using Math.round isn't terrible in this case (though it should be unnecessary for totalIn and totalOut as they should only ever be integers) as JSON.parse will have already truncated large values, but it does mean in js-land you're only ever going to get accurate numbers if you're using an in-process js-IPFS node.

Really we need the go-IPFS HTTP API to return these (and all *64 numbers) as strings instead, then we can preserve the full value as BigInts. I think rateIn/rateOut could just be regular floats, I'm not sure we care that much about 64 bit float precision for input/output bandwidth.

I've opened ipfs/kubo#8319 to discuss this.

@jurelik
Copy link
Author

jurelik commented Aug 2, 2021

Thank you for the fast response Alex. Going the string route sounds like a good idea. I was wondering if BigInt is necessary for rateIn/rateOut myself - floats would be more than enough for my use case but there might be people out there who need the precision. I'm sure you'll figure out what's best.

Looking forward to an update to go-ipfs!

achingbrain added a commit that referenced this issue Aug 9, 2021
The return type for `rateIn`/`rateOut` should be a float and not an
integer.

go-IPFS returns a `float64` which isn't representible in js, but we
are not interested in that level of accuracy for this value so just
return it as a regular `float`.

Fixes #3782
achingbrain added a commit that referenced this issue Aug 10, 2021
The return type for `rateIn`/`rateOut` should be a number and not an integer.

go-IPFS returns a `float64` which isn't representable in js, but we are not interested in that level of accuracy for this value so just return it as a regular `number` which can be a `float`.

Fixes #3782

BREAKING CHANGE: rateIn/rateOut are returned as numbers
SgtPooki referenced this issue in ipfs/js-kubo-rpc-client Aug 18, 2022
The return type for `rateIn`/`rateOut` should be a number and not an integer.

go-IPFS returns a `float64` which isn't representable in js, but we are not interested in that level of accuracy for this value so just return it as a regular `number` which can be a `float`.

Fixes #3782

BREAKING CHANGE: rateIn/rateOut are returned as numbers
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants