Issue with implementing limits in fastapi
anandtripathi5 opened this issue · 5 comments
I'm implementing limits in fastapi to have more control over the ratelimiting logic that cannot be done by other libraries. I have the below implementation
limiter = FixedWindowElasticExpiryRateLimiter(storage_from_string("async+redis://localhost:6379"))
def app_rate_limit(func):
@wraps(func)
async def wrapper(*args, **kwargs):
is_allowed = await limiter.hit(parse('1/second'), "random-key")
if not is_allowed:
raise HTTPException(status_code=429, detail="Too many requests")
return await func(request, *args, db=db, **kwargs)
return wrapper
And in the fastapi route I'm attaching the above decorator
@router.post(...)
@app_rate_limit
def route(...)
pass
its giving the below error
File "/Users/anandtripathi/.virtualenvs/verification-backend-CPjzdNsq-py3.8/lib/python3.8/site-packages/limits/aio/strategies.py", line 196, in hit
amount = await self.storage.incr(
ReferenceError: weakly-referenced object no longer exists
I checked inside the code of limits it seems to be coming from storage initialization but not sure what I'm doing wrong
Aah its its seems from inside the code the storage is self.storage: Storage = weakref.proxy(storage)
weakref proxy. So it will not work with below code
limiter = FixedWindowElasticExpiryRateLimiter(storage_from_string("async+redis://localhost:6379"))
But instead I have to put the storage in a variable so that the object will not be collected by garbage collector once its out of scope. So I have to change it to below code
redis = storage_from_string("async+redis://localhost:6379")
limiter = FixedWindowElasticExpiryRateLimiter(redis)
Can we include this somewhere in the docs?
Yeap you got it! Looks like that code has been mostly the same for about 10 years (ref and I can't actually reason why the weak reference is even there. It might be simpler to just change the code to use a strong reference.
Totally makes sense
@anandtripathi5 thanks again for raising this. I've removed the weak reference in 3.4.0 so if you upgrade you should be able to do without the workaround.
Resolved in 3.4.0