Skip to content
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 our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Limiter Middleware behaves wrongly after latest changes #1567

Closed
DomiM94 opened this issue Oct 7, 2021 · 3 comments 路 Fixed by #1568
Closed

馃悰 Limiter Middleware behaves wrongly after latest changes #1567

DomiM94 opened this issue Oct 7, 2021 · 3 comments 路 Fixed by #1568

Comments

@DomiM94
Copy link

DomiM94 commented Oct 7, 2021

Fiber version
v 2.20+ (Limiter Middleware Commit #1542 )

Issue description
Since the new attributes for skip options were implemented the limiter does not work as expected anymore.
When I don't set any of the two new attributes the request will be forwarded to the upstream server even in case if this request will hit the limit. Therefore I end up with executing a successful request and returning a 429 (since after this request's execution the limiter notices that the limit was hit/overdone).

The solution would be
(1):
Don't forward the request when both of the attributes are set to false.

(2):
Don't return 429 on requests which were in fact executed successfully on the upstream (this affects the lastest request which is executed before the limit was hit (or overdone with the new implementation)). This will hide the actual statuscode.

@welcome
Copy link

welcome bot commented Oct 7, 2021

Thanks for opening your first issue here! 馃帀 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

@aliereno could you check this

@aliereno
Copy link
Contributor

aliereno commented Oct 7, 2021

Hi, I tried to solve it with this pr #1568.
This solution
1- doesn't forward the request before the limit reached calculations.
2- doesn't returns 429 on successful requests and doesn't overwrite the actual statuscode.

But there is a problem that I can't solve. I need some help.
When remaining number is 1 and next request should be skipped, this code will return 429 because it can't see the actual status code before checking limit reached.

@ReneWerner87 ReneWerner87 linked a pull request Oct 11, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants