upstash / ratelimit-js

Rate limiting library for serverless runtimes
https://ratelimit-with-vercel-kv.vercel.app
MIT License
1.64k stars 32 forks source link

Unable to check remaining uses without triggering an increment #17

Closed jamesward1 closed 8 months ago

jamesward1 commented 1 year ago

For longer term rate limits, it's useful to show the end user a count of how many uses they have left within a rate limit window. Currently, there doesn't appear to be any way to access the key consistently without also triggering an increment on the number of uses. The key appears to be appended with a variable number value that gets in the way of doing a standard redis GET query.

Is there some way to consistently do a GET call on the key without implementing some further custom logic? If not, this would be nice to have.

chronark commented 1 year ago

That variable number is actually just the timestamp of the current window For example in fixedWindow you could simply recreate the key like this:

const bucket = Math.floor(Date.now() / windowDuration);
const key = [identifier, bucket].join(":");

const counter = await redis.get(key)

windowDuration is the number of milliseconds in your chosen window

Sounds useful, I'll add it to my backlog

AtotheY commented 1 year ago

+1 on this request - having a limiter.checkLimit(key); would be really helpful - otherwise the only way for users to know how many requests they have left is to actually use a credit

AtotheY commented 1 year ago

Also wondering if there is a timeline for this - I know you mentioned it was backlog so just curious if it's coming anytime soon 👀 👍

chronark commented 1 year ago

Damn, I didn't see your responses, sorry about that @AtotheY @jamesward1 I'm working on some improvements and will ship something for this as well: tomorrow or next week.

Than-DE commented 1 year ago

Hi @chronark just wondering if this was still in the backlog. I am trying to look into how the increment events are being counted on redis but I can't seem to figure out how I would implement such a checkLimit() feature. 🙏

chronark commented 1 year ago

we have recently added ratelimit analytics, does that solve your problem?

Than-DE commented 1 year ago

Ah not really, unfortunately @chronark. What I'd need is a way to check for a given identifier what is the remaining limit for the current window (fixed in my case). The same way remaining is returned by the .limit() method.

This is useful so that the user can see their remaining limit without having to actually make another request. I am using this rate limiting for an expensive OpenAI API call and I'm only allowing X requests per day.

The node-rate-limiter has a getRemainingTokens() method that does exactly what I need, but since I am using a serverless function, that package is not an option 😢

chronark commented 1 year ago

I see the tricky thing is that it's different for each algorithm, which one are you using?

Than-DE commented 1 year ago

I am using the fixed window. I assume it is difficult because neither the identifier string entry nor the event hash in redis actually tells you how many tokens were consumed at what time?

chronark commented 1 year ago

ah ok that one is fairly trivial

const intervalDuration = 1000 // 1s - change this to your setting
const prefix = "@upstash/ratelimit"
const key = [prefix, identifier, Math.floor(Date.now() / intervalDuration)].join(":")

const remaining = await redis.get<number>(key)
Than-DE commented 1 year ago

Thanks, that is good to hear. I did try this but have a mismatch between what .limit() returns and what this calculation returns. Do you think it has something to do with the fact that I am testing this in the middle of an ongoing window and by tomorrow (my window is 1 day = 1000 * 60 * 60 * 24) these two values will sync?

Edit: Actually - I think this calculation does not return the remaining but the already consumed tokens, meaning the value of this should be subtracted from the maximum tokens specified in the limiter configuration. Am I understanding this correctly?

chronark commented 1 year ago

Ah sorry, yes you are correct. It returns the used amount in the current window

Than-DE commented 1 year ago

Very cool, I think this would be nice to be documented. Shall I make a PR?

chronark commented 1 year ago

that'd be great, thanks

Than-DE commented 1 year ago

Revisiting this because I'm not 100% sure if this remaining token calculation for the fixed window really works. I checked the .limit() method and as far as I understand, it increments the redis key first before checking whether to set success to true: https://github.com/upstash/ratelimit/blob/c509628a6c1490121d810822ce7c880ddb02c44c/packages/sdk/src/single.ts#L158-L160

This means that when I call .limit() and check the returned remaining number it can also return a negative remaining value. For some reason, this doesn't happen every time, though.

CleanShot 2023-06-19 at 19 39 55

chronark commented 1 year ago

yeah that looks like an oversight it can return negative, but the check if it may pass also accounts for that

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 8 months ago

This issue was closed because it has been stalled for 30 days with no activity.