Rate limit does not apply after a period of inactivity

[sarfata]

The rate limiter will not limit after a period of inactivity.

Expected behavior

If there is no activity on the rate limiter for a while, it should be filled to maximum capacity but not allow more than capacity tokens to be consumed at once.

Example:

• Rate limiter set to 1 call per second (Ratelimiter(1, 1, 1))
• No calls for 30 seconds
• Rate limiter should still only allow one call per second.

Actual behavior

• Rate limiter set to 1 call per second (Ratelimiter(1, 1, 1))
• No calls for 30 seconds
• Rate limiter will allow 31 calls before applying rate limit again

Comment

Because we increment self.next to be self.next = self.next + self.tick, the rate limiter will be less than now() when all the capacity is not used. Rate limiting only applies again after self.next catches up with now().

Steps to reproduce the behavior

I wrote a simple test to reproduce this. This could use some refactoring to use a mocked timesource for the tests but here it goes for now.

#[cfg(test)]
mod tests {
    use std::{thread::sleep, time::Duration};

    use crate::Ratelimiter;

    #[test]
    fn should_not_burst_beyond_capacity_gh() {
        let rl = Ratelimiter::new(1, 1, 1);

        // Sleep 10s
        sleep(Duration::from_secs(10));

        let mut tokens = 0;
        while rl.try_wait().is_ok() {
            tokens += 1;
        }

        assert_eq!(tokens, 1);
    }
}

Fails with:

thread 'tests::should_not_burst_beyond_capacity_gh' panicked at 'assertion failed: `(left == right)`
  left: `11`,
 right: `1`', ratelimit/src/lib.rs:386:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[brayniac]

Thanks for the detailed issue. Will take a look at the logic tomorrow.

[brayniac]

This was fixed by #37