abersheeran / asgi-ratelimit

A ASGI Middleware to rate limit
Apache License 2.0
298 stars 13 forks source link

Library should handle unexpected exception related to backend #74

Open Bharat23 opened 6 months ago

Bharat23 commented 6 months ago

Description

When using Redis as a backend, the code doesn't handle any exceptions caused by the backend, i.e., Network/Connectivity related, which results in a 5xx internal server error.

How to reproduce

  1. Set up Redis backend
  2. start the service, and then
  3. bring down the Redis.
  4. Call the endpoint which matches any rule and the endpoint will return 5xx.

Expected Behavior:

On backend exceptions, the endpoint should not fail and just let it do the standard processing and disregard rate-limiting.

Traceback (most recent call last):
  File "../lib/python3.9/site-packages/redis/asyncio/connection.py", line 577, in connect
    await self.retry.call_with_retry(
  File "../lib/python3.9/site-packages/redis/asyncio/retry.py", line 59, in call_with_retry
    return await do()
  File "../lib/python3.9/site-packages/redis/asyncio/connection.py", line 922, in _connect
    reader, writer = await asyncio.open_connection(
  File "/opt/homebrew/Cellar/python@3.9/3.9.15/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/streams.py", line 52, in open_connection
    transport, _ = await loop.create_connection(
  File "uvloop/loop.pyx", line 2039, in create_connection
  File "uvloop/loop.pyx", line 2016, in uvloop.loop.Loop.create_connection
ConnectionRefusedError: [Errno 61] Connection refused

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "../lib/python3.9/site-packages/ratelimit/core.py", line 95, in __call__
    retry_after = await self.backend.retry_after(path, user, rule)
  File "../lib/python3.9/site-packages/ratelimit/backends/redis.py", line 46, in retry_after
    block_time = await self.is_blocking(user)
  File "../lib/python3.9/site-packages/ratelimit/backends/redis.py", line 42, in is_blocking
    a = await self._redis.ttl(f"blocking:{user}")
  File "../python3.9/site-packages/redis/asyncio/client.py", line 513, in execute_command
    conn = self.connection or await pool.get_connection(command_name, **options)
  File "../lib/python3.9/site-packages/redis/asyncio/connection.py", line 1375, in get_connection
    await connection.connect()
  File "../lib/python3.9/site-packages/redis/asyncio/connection.py", line 585, in connect
    raise ConnectionError(self._error_message(e))
redis.exceptions.ConnectionError: Error 61 connecting to 127.0.0.1:6379. 61.

Python version: 3.9 Library version: 0.10.0

Bharat23 commented 6 months ago

I am happy to create a PR to gracefully handle this.

abersheeran commented 6 months ago

Welcome PR.

Returning 503 directly when unable to connect to redis would be a good approach.

Bharat23 commented 5 months ago

Welcome PR.

Returning 503 directly when unable to connect to redis would be a good approach.

I was thinking "disable rate limiting if backend is down". The fact that redis is down may not mean that the underlying service is down. I think service should still respond back if it can, only the ratelimiting should be disabled. Thoughts?

abersheeran commented 5 months ago

This can lead to problems such as API theft.

Bharat23 commented 4 months ago

PR for the requested enhancements https://github.com/abersheeran/asgi-ratelimit/pull/75