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

Proper rate limiting headers #139

Closed
iCrawl opened this issue Jun 6, 2018 · 3 comments
Closed

Proper rate limiting headers #139

iCrawl opened this issue Jun 6, 2018 · 3 comments

Comments

@iCrawl
Copy link

iCrawl commented Jun 6, 2018

I was told to x-post this on here since it's more of an issue/feature with/for the oxy middleware.
traefik/traefik#3440

Do you want to request a feature or report a bug?

Feature

What did you expect to see?

The way the current rate limits are implemented are a bit of lackluster. I hope this can be improved upon.
Currently when you define your rate limits as suggested in the .yml or .toml file, you can specify a duration, an average and a burst. Additionally to an expression it should trigger on.

Now iIf you hit one of those rate limits in the loadbalancer it will return a string which says the following:

max rate reached: retry-in 30s

func (m *MaxRateError) Error() string {
return fmt.Sprintf("max rate reached: retry-in %v", m.delay)
}

While this may be sort of okay, the headers that get set are:

X-Retry-In: 30s

func (e *RateErrHandler) ServeHTTP(w http.ResponseWriter, req *http.Request, err error) {
if rerr, ok := err.(*MaxRateError); ok {
w.Header().Set("X-Retry-In", rerr.delay.String())
w.WriteHeader(429)
w.Write([]byte(err.Error()))
return
}
utils.DefaultHandler.ServeHTTP(w, req, err)
}

For users that want to properly build around those rate limits and throttle depending on how close you are to hitting them, it would be kind of nice to have a way of always sending rate limit headers?

Maybe something along the lines of on every request:

X-Ratelimit-Limit: 50
X-Ratelimit-Remaining: 20 // or 0 if you hit the ratelimit
X-Ratelimit-Reset: Unix Timestamps of reset

This way the users that interface with any service could properly integrate their application into those ratelimits.

Additionally, it would be nice to have a configurable response. Currently as mentioned above it returns a string, which can certainly throw off any implementation on a JSON based REST API since the response body would not match the expected JSON response.

@herver
Copy link
Contributor

herver commented Jun 19, 2018

Hi there !

With a little bit of investigation, it appears that RFC6585 mentions a Retry-After header and is expressed in seconds, so I think we should use this header.

@iCrawl
Copy link
Author

iCrawl commented Jun 19, 2018

Sure!

Thanks for looking it up. I think its not completely necessary to abolish it, don't get me wrong.
A Retry-After header when you have exceeded the rate limits (expressed in seconds like you mentioned) is fine.

But it would be nice to have some sort of customizability to specify the other stuff that I mentioned. Mainly expressing the rate limits limit and remaining. Reset can be helpful when it comes to easily have a "back-off" to make sure to not make more requests.

@emilevauge
Copy link
Contributor

Closed by #141

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

3 participants