http-cache-semantics icon indicating copy to clipboard operation
http-cache-semantics copied to clipboard

timeToLive() returns floats

Open avivimgn opened this issue 4 years ago • 3 comments

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.

avivimgn avatar Aug 12 '21 13:08 avivimgn

Does this break anything other than expectations?

Milliseconds can be fractional too.

kornelski avatar Aug 12 '21 17:08 kornelski

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 avatar Sep 02 '21 10:09 avivimgn

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

avivimgn avatar Sep 02 '21 10:09 avivimgn