search

heroku / identity

[DEPRECATED] Login and OAuth management service for Heroku
https://id.heroku.com/
MIT License
246 stars 20 forks source link

Handle 429 errors from API #154

Closed dmcinnes closed 9 years ago

dmcinnes commented 9 years ago

Currently when a user hitting identity is rate limited in API, we throw a 500 (and spam rollbar). This fixes it to render the 503 error page, "API Unavailable", instead of "Internal Server Error", and no longer log them in Rollbar.

Ready for review @heroku/api

brandur commented 9 years ago

Hah, see also #32, we is something that we should really get fixed the right way. +1 to get these out of Rollbar for now though.

mikehale commented 9 years ago

I guess the right way would be to just proxy the 429 to the client?

brandur commented 9 years ago

I guess the right way would be to just proxy the 429 to the client?

Yeah, exactly. Either bubble these back up to the user and tell them that they're rate limit, or possibly even better, devise a system that puts internal components into different buckets. I tried to do the latter as part of heroku/rfcs#2, but unfortunately never got around to getting the implementation together.

dmcinnes commented 9 years ago

+1 for bubbling the error up to the user. I could add another error view like this one: https://github.com/heroku/identity/blob/master/views/errors/503.slim Something along the lines of You are currently making too many requests to Heroku. Please wait a while and try to log in again. Thoughts?

brandur commented 9 years ago

@dmcinnes If you're up to making the change, then major +1! Thanks.

dmcinnes commented 9 years ago

Added the 429 template/handler code and tests. Also, it appears the 503 errors were responding with a 500, this should fix that.

brandur commented 9 years ago

@dmcinnes AMAZING! Ship it.

Also, it appears the 503 errors were responding with a 500, this should fix that.

Hah, somebody tried to put in a fix for this awhile back and it never seemed to have worked quite right. Thanks for taking a look!