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

Send log on first rate-limited request #16

Open
413n opened this issue Apr 23, 2024 · 8 comments
Open

Send log on first rate-limited request #16

413n opened this issue Apr 23, 2024 · 8 comments

Comments

@413n
Copy link
Contributor

413n commented Apr 23, 2024

I wanted to send an information to my logger service when someone would get rate-limited (I'm using the memory store), but if I send the log in the onLimit function, I would get flooded with a lot of logs (and this make sense because the onLimit function is called on each request).
Would there be an option to have a function that is called only on the first request where the user (with the same fingerprint) is being rate limited?
If there is no option at the moment, I think this would be a feature request

Thanks for the amazing library btw!

@413n 413n changed the title Send warning to logger Send log on first rate-limited request Apr 23, 2024
@OrJDev
Copy link
Owner

OrJDev commented Apr 23, 2024

I wanted to send an information to my logger service when someone would get rate-limited (I'm using the memory store), but if I send the log in the onLimit function, I would get flooded with a lot of logs (and this make sense because the onLimit function is called on each request).

Would there be an option to have a function that is called only on the first request where the user (with the same fingerprint) is being rate limited?

If there is no option at the moment, I think this would be a feature request

Thanks for the amazing library btw!

We would have to create some logic that determines that this is the first time we limit this user, that being said we need to store it somewhere, when using memorystore its easy to do but redis / upstash it equals more storage so that's going to cost more.

@413n
Copy link
Contributor Author

413n commented Apr 23, 2024

Would this be a good thing to implement maybe even only for memory store or you don't like the idea to have different APIs?
Else I would need to store that somewhere else but I liked the idea to keep that logic inside the MemoryStore

@OrJDev
Copy link
Owner

OrJDev commented Apr 23, 2024

Would this be a good thing to implement maybe even only for memory store or you don't like the idea to have different APIs?

Else I would need to store that somewhere else but I liked the idea to keep that logic inside the MemoryStore

I could probably add isFirstLimit property to the hitinfo provided by memorystore. As each provider provides different hitinfo anyways.

@413n
Copy link
Contributor Author

413n commented Apr 23, 2024

That would be awesome!

@413n
Copy link
Contributor Author

413n commented Apr 24, 2024

If you are thinking to add it, do you have an estimation of when it could be added?
I could also try to help with a PR if you don't have much time

@OrJDev
Copy link
Owner

OrJDev commented Apr 24, 2024

If you are thinking to add it, do you have an estimation of when it could be added?

I could also try to help with a PR if you don't have much time

Yep sorry about that, its not that its hard to implement it's literally a few lines however i work full time i cant lose any work day based on the current status, therefore i don't really get the chance to open the computer considering that i go gym and see my girlfriend once i finished work.

If you make a pr, i will make sure to review and merge it asap

@413n
Copy link
Contributor Author

413n commented Apr 30, 2024

I was looking to implement it, but I'm slightly confused about where you would like to have it. You would add a new property in the MemoryStore and then pass it to the onLimit through the hitInfo variable in the @core package?
Looking at the code the hitInfo returned by isBlocked is always a number, right? or maybe I missed something?

@OrJDev
Copy link
Owner

OrJDev commented May 24, 2024

I was looking to implement it, but I'm slightly confused about where you would like to have it. You would add a new property in the MemoryStore and then pass it to the onLimit through the hitInfo variable in the @core package?

Looking at the code the hitInfo returned by isBlocked is always a number, right? or maybe I missed something?

HitInfo can be anything not just a number, anyways hitinfo shouldn't be a factor here it should indeed be a new property within the memorystore

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