-
Notifications
You must be signed in to change notification settings - Fork 97
Description
Describe the bug
In case of ConnectionError from Redis (and acutally any error raised from storage access), traceback is swallowed along the way, so that I had to guess what was the reason of the problem (in my case redis.ConnectionError
was raised, I'm still not 100% sure why).
Produced traceback looks like this:
File "/src/.venv/lib/python3.12/site-packages/uvicorn/protocols/http/h11_impl.py", line 396, in run_asgi
result = await app( # type: ignore[func-returns-value]
File "/src/.venv/lib/python3.12/site-packages/uvicorn/middleware/proxy_headers.py", line 70, in __call__
return await self.app(scope, receive, send)
File "/src/.venv/lib/python3.12/site-packages/fastapi/applications.py", line 1054, in __call__
await super().__call__(scope, receive, send)
File "/src/.venv/lib/python3.12/site-packages/starlette/applications.py", line 123, in __call__
await self.middleware_stack(scope, receive, send)
File "/src/.venv/lib/python3.12/site-packages/starlette/middleware/errors.py", line 186, in __call__
raise exc
File "/src/.venv/lib/python3.12/site-packages/starlette/middleware/errors.py", line 164, in __call__
await self.app(scope, receive, _send)
File "/src/.venv/lib/python3.12/site-packages/starlette/middleware/base.py", line 189, in __call__
with collapse_excgroups():
File "/usr/local/lib/python3.12/contextlib.py", line 158, in __exit__
self.gen.throw(value)
File "/src/.venv/lib/python3.12/site-packages/starlette/_utils.py", line 93, in collapse_excgroups
raise exc
File "/src/.venv/lib/python3.12/site-packages/starlette/middleware/base.py", line 191, in __call__
response = await self.dispatch_func(request, call_next)
File "/src/.venv/lib/python3.12/site-packages/slowapi/middleware.py", line 130, in dispatch
error_response, should_inject_headers = sync_check_limits(
File "/src/.venv/lib/python3.12/site-packages/slowapi/middleware.py", line 77, in sync_check_limits
return exception_handler(request, exc), _bool # type: ignore
File "/src/.venv/lib/python3.12/site-packages/slowapi/extension.py", line 81, in _rate_limit_exceeded_handler
{"error": f"Rate limit exceeded: {exc.detail}"}, status_code=429
AttributeError: 'ConnectionError' object has no attribute 'detail'
To Reproduce
import pytest
import redis
from fastapi import FastAPI, testclient, APIRouter
from slowapi import Limiter, _rate_limit_exceeded_handler
from slowapi.util import get_remote_address
from slowapi.errors import RateLimitExceeded
from slowapi.middleware import SlowAPIMiddleware
router = APIRouter()
@router.get(
"/healthcheck",
)
def health_check():
"""App health check"""
return {"status": "healthy"}
@pytest.fixture(scope="session")
def app() -> FastAPI:
app = FastAPI()
limiter = Limiter(
key_func=get_remote_address,
default_limits=["100/minute"],
storage_uri="redis://default:password@localhost:6379/0"
)
app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
app.state.limiter = limiter
app.add_middleware(SlowAPIMiddleware)
app.include_router(router)
return app
@pytest.fixture(scope="session")
def test_client(
app: FastAPI,
) -> testclient.TestClient:
return testclient.TestClient(app)
def test_rate_limitter_on_redis_error(
app: FastAPI,
test_client: testclient.TestClient,
):
# given
def redis_side_effect(*args, **kwargs):
raise redis.ConnectionError()
app.state.limiter._limiter.hit = redis_side_effect
# when
response = test_client.get(app.url_path_for("health_check"))
# then
assert response.status_code == 200
assert response.json() == {"status": "healthy"}
Expected behavior
Error should be raised with correct traceback
Your app (please complete the following information):
- fastapi
- "0.110.3"
- slowapi version "0.1.9"
Additional context
- the original error was not included in traceback because it is returned from this function:
Line 53 in a72bcc6
return exception_handler, False, e - there is no exception_handler registered for ConnectionError (or existing one is a async one), so default is used:
Line 75 in a72bcc6
exception_handler = _rate_limit_exceeded_handler - but the default handler is the one for handling SPECIFIC errors from rate limiting, it is not prepared to handle all exceptions.
I think it would be good to separate the logic for handling rate limit errors from other errors.
Workarounds
In my case it was good enough to add in_memory_fallback_enabled=True,
. Slowapi is smart enough to reconnect when storage is up. But this approach has an unpleasant side effect that real problems are hidden.
Alternatively one could register event handler to do custom work, but I not tested this path.