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

Add rate limiting #9

Closed
ZacharyDavidSaunders opened this issue Apr 15, 2019 · 3 comments
Closed

Add rate limiting #9

ZacharyDavidSaunders opened this issue Apr 15, 2019 · 3 comments

Comments

@ZacharyDavidSaunders
Copy link
Owner

The API needs rate limiting (see ZacharyDavidSaunders/pseudoname#9).

https://www.npmjs.com/package/express-rate-limit may be a good option.

@ZacharyDavidSaunders
Copy link
Owner Author

The rate-limiting functionality has been added to the "rate-limiting" branch. I'm going to explore the possibility of adding tests but I don't want the tests to take 30 minutes to finish (the current cooldown period for each route). With that said however, there is value adding rate-limiting tests. 🤔

@ZacharyDavidSaunders
Copy link
Owner Author

I've looked into this a bit further. The challenge is that each route test and each middleware test counts towards the rate-limited number (permittedRequestsPerInterval in the code). I could make the tests talk to each other and keep a collective count of how many requests have been made for each route, but that still doesn't address the issue of waiting until the delay has finished. With all of this in mind, I don't think tests are worth the added complexity in this case. I'm going omit tests and merge this change into the "API Rebuild" branch shortly.

@ZacharyDavidSaunders
Copy link
Owner Author

Merged (#23) 😄

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

1 participant