NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens#53282

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 2 commits intosymfony:6.4fromwouterj:fix_rate_limit_bc_after_51676_pr

Dec 30, 2023

Merged

[RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens#53282

nicolas-grekas merged 2 commits intosymfony:6.4fromwouterj:fix_rate_limit_bc_after_51676_pr

Dec 30, 2023

Conversation

Copy link

Member

wouterj commentedDec 29, 2023

Q	A
Branch?	6.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	-
License	MIT

Replaces#52835 Original description:

Have got some BC after updating the project to Symfony 6.4 (after that PR#51676).
Sometimes I need to getRateLimit object without consuming just before consuming a try, in example:
$rateLimit =$limiter->consume(0);if ($rateLimit->getRemainingTokens() ===0) {thrownewSomeException($rateLimit->getRetryAfter());}...$limiter->consume(1)...
and found that in that case$rateLimit->getRetryAfter() always returnsnow.
So this PR fixes it.

Reintroduce peek consume test for sliding window policy

169e383

carsonbot added Status: Needs Review Bug RateLimiter labels

Dec 29, 2023

carsonbot added this to the6.4 milestone

Dec 29, 2023

OskarStark changed the title~~[RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens.~~[RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens

Dec 29, 2023

wouterj mentioned this pull request

Dec 29, 2023

[RateLimiter] FixRateLimit->getRetryAfter() return value when consuming0 or last tokens.#52835

Closed

[RateLimit] Allow to get RateLimit without consuming again

677b8b7

nicolas-grekas force-pushed thefix_rate_limit_bc_after_51676_pr branch from701dc3c to677b8b7Compare

December 30, 2023 09:55

Copy link

Member

nicolas-grekas commentedDec 30, 2023

Thank you@wouterj.

nicolas-grekas merged commit4e10a49 intosymfony:6.4

Dec 30, 2023

Copy link

Member

nicolas-grekas commentedDec 30, 2023

And thank you@ERuban

wouterj deleted the fix_rate_limit_bc_after_51676_pr branch

December 30, 2023 10:13

This was referencedDec 30, 2023

Release v6.4.2#53314

Merged

Release v7.0.2#53315

Merged

Copy link

Contributor

ERuban commentedJan 23, 2024

Huh, have found that this PR didn't fix some cases that was fixed by my ones.

if you will try to checkgetRetryAfter right after using the last token(s) - then it will always returnnow() but it should be a time when first attempt will be available
When there is no available tokens and 0 is consumed thengetRetryAfter will show the time when all tokens will be available...

/cc@wouterj