jaredwray / flat-cache

A stupidly simple key/value storage using files to persist the data
MIT License
165 stars 30 forks source link

Keyv: `Invalid String Length` #96

Closed TobiTenno closed 6 months ago

TobiTenno commented 6 months ago

Related, somewhat, to #94...

just chiming in as to the original discussion on keyv, that update has caused for me, as the version of json-buffer introduced causes

RangeError: Invalid string length
     at stringify (node_modules/json-buffer/index.js:32:35)
    at Object.stringify [as serialize] (node_modules/json-buffer/index.js:32:37)
    at node_modules/keyv/src/index.js:202:22

I'm testing on node lts/hydrogen (v18.19.1), npm v10.2.4. This is specifically happening on >= 3.1.1.

I'm not able to reproduce the same issue when pinned to 3.0.4.

jaredwray commented 6 months ago

@TobiTenno can you send a use case that is getting this error?

TobiTenno commented 6 months ago

@jaredwray for sure. it's a large install, but on this project, i had to pin to 3.0.4: https://github.com/WFCD/warframe-status/pull/1371

not specifically that PR, but that PR is where i pinned it because i'd had to regen package-lock and such when doing this fix, which pulled in the newer updates after 3.0.4, which then caused issues. specifically if you install and run the tests, you should see it readily after either unpinning and clearing the package-lock/node_modules, or just forcing an upgrade to latest/^3/^4.

TobiTenno commented 6 months ago

my assumption is because i have large json objects going in, but none of them should be circular, as they're plain json

jaredwray commented 6 months ago

do you have an example of the large json objects so I can test on this?

TobiTenno commented 6 months ago

the error arises from my tests, but the error doesn't specify which of my caches causes it.

the populate script here is the largest in my dataset.

TobiTenno commented 6 months ago

i can create a gist to recreate if that's preferable

jaredwray commented 6 months ago

that would help a ton. Thanks

TobiTenno commented 6 months ago

not a gist, but i did a whole repo so it'd have the dependencies already.

it doesn't fail on first write, but it does on second write

the kind of deadly aspect of this is that during most CI if you only write once, it wouldn't do anything out of the ordinary, and when it'd get to production, it would fail.

jaredwray commented 6 months ago

one quick work around is to just set keyv to nothing cache.keyv = undefined

jaredwray commented 6 months ago

I will look for a long term solution this weekend.

TobiTenno commented 6 months ago

sounds good. I pushed an update that should help demonstrate even better, it actually happens after the save "succeeds". (it includes ora, a spinner)

TobiTenno commented 6 months ago

i added cache.keyv = undefined; when i initially set up the cache, but the error still pops up. (see latest).

jaredwray commented 6 months ago

Thanks so much for the use case and it looks like the file is not caching correctly with Keyv. I have removed Keyv right now from the main path to get past this and published a v4.0.1 until I can get this fixed.

jaredwray commented 6 months ago

@TobiTenno can you do me a big favor and keep this cache-test repo live so I can come back to it and test it?

TobiTenno commented 6 months ago

@TobiTenno can you do me a big favor and keep this cache-test repo live so I can come back to it and test it?

happy to!

TobiTenno commented 6 months ago

it's also much faster on successive writes this way :+1:

jaredwray commented 6 months ago

it's also much faster on successive writes this way 👍

We will be moving to a more modern architecture for v5 and will add back in Keyv as the in memory caching at that point with async / await.