laurentS / slowapi

A rate limiter for Starlette and FastAPI
https://pypi.org/project/slowapi/
MIT License
1.18k stars 74 forks source link

Real traceback is lost in case of any internal error (AttributeError: 'XXX' object has no attribute 'detail') #213

Open UncleGoogle opened 4 weeks ago

UncleGoogle commented 4 weeks ago

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):

Additional context

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.