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

added optional limit, rate and remaining headers when using throttle #1425

Closed
wants to merge 2 commits into from
Closed

added optional limit, rate and remaining headers when using throttle #1425

wants to merge 2 commits into from

Conversation

eps1lon
Copy link

@eps1lon eps1lon commented Aug 1, 2017

As request in #1344 this exposes headers that specify how the api is throttled. This will not happen unless explicitly set in setHeaders.

Added headers are:

  • X-RateLimit-Remaining for remaining tokens
  • X-RateLimit-Limit for burst
  • X-RateLimit-Rate for the refresh reate

I did some quick research and i only found that Twitter uses x-rate-limit-* instead of camelCase.

Most changes were made in the test since i had to setup a new server for that test. As far es I can tell there is no way to change plugins options after they were added.

Copy link
Member

@retrohacker retrohacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we add the assert statement, this looks good to me! 👍 great work on this @eps1lon !

var tooManyRequests = !bucket.consume(1);

// set throttle headers after consume which changes the remaining tokens
if (options.setHeaders) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the top of this function, we should assert options.setHeaders is an optional boolean.

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

Successfully merging this pull request may close these issues.

None yet

2 participants