Hit incorrectly returns True when cost higher than limit

[aarmoa]

Hello limits team. I think there is a problem when the hit method is called passing a cost higher than the limit amount configured in the limit. I tested it with the following code:

from limits import storage
from limits import strategies
from limits import parse


# my_storage = storage.RedisStorage("redis://localhost:6379/1")
my_storage = storage.MemoryStorage()
window = strategies.MovingWindowRateLimiter(my_storage)

ten_per_minute = parse("10/minute")

window.clear(ten_per_minute, "test_namespace", "foo")

assert True == window.hit(ten_per_minute, "test_namespace", "foo", cost=5)
assert False == window.hit(ten_per_minute, "test_namespace", "foo", cost=100)

In the script the limit is created as 10 hits per minute. I first consume 5 with the first hit call and it returns True (as expected). Then I try to consume 100, that should not be permitted, and the hit returns True again.

The error that occurs is:

Traceback (most recent call last):
  File "/Users/aarmoa/Library/Application Support/JetBrains/PyCharm2022.3/scratches/test_limits.py", line 15, in <module>
    assert False == window.hit(ten_per_minute, "test_namespace", "foo", cost=100)
AssertionError

I have only reproduced the issue with MovingWindowRateLimiter, but think it might be happening for all storages (I reproduced the error both with MemoryStorage and with RedisStorage.

[alisaifee]

Thanks for reporting this @aarmoa. It is definitely a bug in the implementation for moving window (across all storage types). I'll add some test cases for this scenario and try to address it by EOD.

[alisaifee]

This is resolved in master and will be available in the next release.

[alisaifee]

@aarmoa the fix is available in 3.1.6. Please close the issue once you've had a chance to verify.

[alisaifee]

Closing due to inactivity.