8000 TokenBucketLimiter throws on interval below 1 second · Issue #48871 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

TokenBucketLimiter throws on interval below 1 second #48871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
geek-merlin opened this issue Jan 4, 2023 · 8 comments
Closed

TokenBucketLimiter throws on interval below 1 second #48871

geek-merlin opened this issue Jan 4, 2023 · 8 comments
Labels
Bug RateLimiter Stalled Status: Needs Review

Comments

@geek-merlin
Copy link
geek-merlin commented Jan 4, 2023 •
edited
Loading

Symfony version(s) affected

6.2.2

Description

I need limiter sometimes for very fine grained time intervals. But below 1 second it throws.
(Fixed WindowLimiter is better here but #47676)

How to reproduce

    $this->limiter = (new RateLimiterFactory([
        'id' => 'dontcare',
        'policy' => 'token_bucket',
        'limit' => 999,
        'rate' => ['interval' => '200 milliseconds', 'amount' => 1],
      ], new InMemoryStorage()))->create();
    $this->limiter->reserve()->wait();

gives

PHP Fatal error:  Uncaught DivisionByZeroError: Division by zero in /home/merlin/Code-Incubator/bbnd/untrack-email-analyzer/vendor/symfony/rate-limiter/Policy/Rate.php:97
Stack trace:
#0 /home/merlin/Code-Incubator/bbnd/untrack-email-analyzer/vendor/symfony/rate-limiter/Policy/TokenBucket.php(71): Symfony\Component\RateLimiter\Policy\Rate->calculateNewTokensDuringInterval()
#1 /home/merlin/Code-Incubator/bbnd/untrack-email-analyzer/vendor/symfony/rate-limiter/Policy/TokenBucketLimiter.php(68): Symfony\Component\RateLimiter\Policy\TokenBucket->getAvailableTokens()
#2 /home/merlin/Code-Incubator/bbnd/untrack-email-analyzer/src/Utility/ThrottlingGuzzleClientDecorator.php(35): Symfony\Component\RateLimiter\Policy\TokenBucketLimiter->reserve()

Possible Solution

No response

Additional Context

No response
@geek-merlin geek-merlin added the Bug label Jan 4, 2023
@MatTheCat
Copy link
Contributor
MatTheCat commented Jan 5, 2023 •
edited
Loading

This has been discussed on #42194 and particularly #42194 (comment)

TL;DR you shouldn’t use the Symony’s rate limiter if you need sub-second intervals, but you can still submit a PR to implement it (unless @wouterj changed his mind of course!).

@geek-merlin
Copy link
Author

Thanks for clarifying! So maybe a docs issue currently.

@xabbuh
Copy link
Member
xabbuh commented Jan 6, 2023

Maybe we could improve DX by validating the interval in the constructor of the Rate class to throw early when the given interval is less than one second.

@wouterj
Copy link
Member
wouterj commented Jan 6, 2023

The referenced PR is closed as completed, as there was a PR to fix it: #43677

So if something is wrong still, it would be good if someone can debug it. Are you up for that, @geek-merlin?

@xabbuh
Copy link
Member
xabbuh commented Jan 6, 2023

@wouterj Rate::calculateNewTokensDuringInterval() uses TimeUtil::dateIntervalToSeconds() to compute the number of cycles.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Friendly ping? Should this still be open? I will close if I don't hear anything.

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug RateLimiter Stalled Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@wouterj @OskarStark @MatTheCat @xabbuh @carsonbot @geek-merlin
0