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

Return an error page for the blocked users. More relevant http status code. #22

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Spunkie
Copy link
Contributor

@Spunkie Spunkie commented Feb 23, 2017

Commit MSG:
Return an error page for the blocked users. Changed the returned status code from 429 to 403. This is to better reflect the functionality of a blocklist rather than rate limiting which 429 implies.

You can find a preview of the error page here: http://codepen.io/Spunkie/pen/wJvpKJ

Based on personal experience I thought it would be good to include an email or some way of contacting you to discuss/dispute a block. For now I just went with a generic email of [email protected]. I'm not sure if you even have email setup on that domain or would you prefer to link to the github issue tracker instead?

@eduardoboucas
Copy link
Member

Thanks for doing this. I like the idea of having a helpful help message, but wouldn't it make more sense for it to use the same styling as the site? We could even load the same style sheet, meaning that would avoid putting styles in the template.

@Spunkie
Copy link
Contributor Author

Spunkie commented Feb 24, 2017

It certainly would be nice, although I had a few reasons I set it up this way.

  1. Convention, in my experience if an API serves stylized responses at all it'll usually be very minimalistic. Often serving just a couple of inline styles for emphasis.
  2. In the case of this specific blacklist response where the server responds to a single request with a single response that could summed up as "fuck off". I wonder if it makes sense to turn this 1 request into 1 request + a bunch of requests for assets. If assets are being served by a CDN this may not be a real concern though.
  3. The most important reason is simply that I don't know what would be best/proper way to implement that. I'm unfamiliar with jekyll or static site generators in general, in fact this is the first nodejs project I've messed with other than stuff like npm and gulp.

…us code from 429 to 403. This is to better reflect the functionality of a blocklist rather than rate limiting which 429 implies. Rebase to fix git author.

Signed-off-by: Spunkie <[email protected]>
@Spunkie Spunkie force-pushed the dev-better-blocklist-error branch from bf9c9d0 to e3fd6c6 Compare February 24, 2017 01:47
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.

2 participants