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

timeToLive() returns floats #35

Closed
avivimgn opened this issue Aug 12, 2021 · 3 comments
Closed

timeToLive() returns floats #35

avivimgn opened this issue Aug 12, 2021 · 3 comments

Comments

@avivimgn
Copy link

avivimgn commented Aug 12, 2021

Not always of course, but in some cases timeToLive() returns floats.
Which makes sense since in maxAge() there's a division by 1000 and then later a multiplication by 1000 done by timeToLive(), yet the code ignores computers' struggle with float arithmetic.
Since the doc mentions timeToLive() returns time in milliseconds, one would expect it to return an integer.

Math.round() in the correct spot or some other solution would be appreciated, let me know what you think.

@kornelski
Copy link
Owner

Does this break anything other than expectations?

Milliseconds can be fractional too.

@avivimgn
Copy link
Author

avivimgn commented Sep 2, 2021

It causes a problem in https://github.com/lukechilds/cacheable-request. Since this is the root cause of the problem, I opened the issue here.
Although milliseconds can have fractions, the fractions in this case are not caused by the division between the numbers, but by float arithmetic problems, it seems to me.

That's the error I'm getting in cacheable-request which is coming from Keyv I think:
Error [CacheError]: ERR value is not an integer or out of range

code snippet in cacheable-request:

let ttl = opts.strictTtl ? response.cachePolicy.timeToLive() : undefined;
if (opts.maxTtl) {
	ttl = ttl ? Math.min(ttl, opts.maxTtl) : opts.maxTtl;
}

await this.cache.set(key, value, ttl);

The program gets to this.cache.set() providing ttl = response.cachePolicy.timeToLive() = 1256996.9999999998 for example, and once set() is executed, the error above is thrown.

In order to trigger this behavior I'm setting private,max-age=4 on cache-control header in the request, max-age 4 can be changed to any other number that triggers this behavior.

@avivimgn
Copy link
Author

avivimgn commented Sep 2, 2021

If you think this behavior is correct, it's fine, I'll just open an issue in cacheable-request. let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants