pgjones / quart-rate-limiter

Quart-Rate-Limiter is an extension for Quart to allow for rate limits to be defined and enforced on a per route basis.
MIT License
22 stars 6 forks source link

HTML body returned when rate limit is exceeded #8

Closed daniel-j-h closed 8 months ago

daniel-j-h commented 8 months ago

Hey folks I'm giving Quart and this rate limiter a try right now.

What caught me off-guard is that on a rate exceeded error the user gets to see a default HTML page. This is problematic e.g. when building a JSON API and all of a sudden HTML shows up. I understand that the 429 status code allows me to check if it's an actual JSON response; the fact that we get HTML by default just by using @rate_limit is confusing to me, nevertheless.

@app.post("/ping")
async def hello(..)
  ..
  return MyResponse(id=1)

It looks like it's also not getting validated in any way with quart-schema like in the following

@app.post("/ping")
@validate_request(MyRequest)
@validate_response(MyResponse)
async def hello(..) -> MyResponse:

Here the first request going through

HTTP/1.1 200
content-length: 11
content-type: application/json
date: Sun, 14 Jan 2024 12:36:23 GMT
ratelimit-limit: 1
ratelimit-remaining: 0
ratelimit-reset: 9
server: hypercorn-h11
vary: Origin

{
    "id": "1"
}

and now the second request getting rate limited and returning an html body

HTTP/1.1 429
content-length: 169
content-type: text/html; charset=utf-8
date: Sun, 14 Jan 2024 12:36:24 GMT
ratelimit-limit: 1
ratelimit-remaining: 0
ratelimit-reset: 9
retry-after: 9
retry-after: 9
server: hypercorn-h11
vary: Origin

<!doctype html>
<html lang=en>
<title>429 Too Many Requests</title>
<h1>Too Many Requests</h1>
<p>This user has exceeded an allotted request count. Try again later.</p>

I believe this response is ultimately coming from werkzeug (see the 429 exception description here).


What's a good way to build a JSON-only API with this rate limiter and have errors stay within the JSON protocol?

In addition I was wondering if this behavior is worth calling out in the readme since it's - at least to me - unexpected behavior.

Thank you!

pgjones commented 8 months ago

You can add an error handler if you want to change the error response,

from quart_rate_limiter import RateLimitExceeded

@app.errorhandler(RateLimitExceeded)
async def handle_rate_limit_exceeded_error(error):
    return {}, error.get_headers(), 429

This pattern is quite common for Flask and Quart, so I'm not sure it needs highlighting specifically. That said I wouldn't reject a PR.

pgjones commented 8 months ago

(Note I typo'd my initial response suggesting I would rather than wouldn't reject a PR)

daniel-j-h commented 8 months ago

Ow, makes sense! :bow: This makes me think: should we move this conversation to the quart-schema repo. since it's not rate-limiter related but it does side-step all schema response validation logic :thinking: