search

ExperiBass / esiJS

A simple Node module for EVE Onlines' ESI.
https://github.com/ExperiBass/esiJS
GNU General Public License v3.0
21 stars 10 forks source link

[BUG] Caching causes issues with some requests #28

Open Tmktahu opened 5 months ago

Tmktahu commented 5 months ago

Some ESI requests have you pass data via a payload instead of like a URL parameter. From what I can tell, the caching system at the moment is based on a fullURL that is assembled here:

https://github.com/ExperiBass/esiJS/blob/890faf6adfdf456a5200f1ceac1bcc1df76e7e20/src/util/util.js#L158-L178

Since that method doesn't account for payload parameters, it can cause the caching to return results from previous queries even though the parameters have changed.

The most blatant case of this that I have encountered thus far is with the universe.bulk.idsToNames endpoint as it takes an array of IDs as a payload. Making a second request before the cache times out causes it to return the previous results... which is incorrect.

Side note, my use case for this library is personal and I have explicit control over being respectful of ESI. So for my uses, I don't need a cache. As such I'm going to fork this library and modify it as needed for my own shenanigans. So this bug isn't a blocker for me personally, but I wanted to log it anyway.

ExperiBass commented 5 months ago

hmm yeah, i should prolly do a refactor of this lib and drop the cache in the process, no need for this to care about caching outside maybe forwarding the cache time? p sure thats in the headers tho lol

Tmktahu commented 4 months ago

Yea I wouldn't really worry about caching.

If anything (and you may have this already, dunno), you could have some error-handling logic in there. Since ESI functions off an error limit instead of a rate limit, having some emergency handlers in there that prevents you from making calls past the error limit, or perhaps even warnings when approaching the error limit, would be really nice. It would relieve some of the potentially-angering-the-CCP-gods stress lol.

ExperiBass commented 4 months ago

hmm, is exposing the headers not enough? i could add a .warning object to the request, preventing requests until the timeframe resets may be a bit janky with how the project is structured, but is also possible.