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

Library should handle unexpected exception related to backend #74

Open
Bharat23 opened this issue Apr 5, 2024 · 5 comments
Open

Library should handle unexpected exception related to backend #74

Bharat23 opened this issue Apr 5, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@Bharat23
Copy link

Bharat23 commented Apr 5, 2024

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/[email protected]/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
Copy link
Author

Bharat23 commented Apr 5, 2024

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

@Bharat23 Bharat23 changed the title Library should handle unexpcted exception related to backend Library should handle unexpected exception related to backend Apr 5, 2024
@abersheeran
Copy link
Owner

Welcome PR.

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

@abersheeran abersheeran added the enhancement New feature or request label Apr 6, 2024
@Bharat23
Copy link
Author

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
Copy link
Owner

This can lead to problems such as API theft.

@Bharat23
Copy link
Author

Bharat23 commented Jun 6, 2024

PR for the requested enhancements
#75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants