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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rate limiting is not updated until after the handler returns #2295

Closed
dgw opened this issue May 30, 2022 · 0 comments · Fixed by #2297
Closed

Rate limiting is not updated until after the handler returns #2295

dgw opened this issue May 30, 2022 · 0 comments · Fixed by #2297
Assignees
Labels
Bug Things to squish; generally used for issues
Milestone

Comments

@dgw
Copy link
Member

dgw commented May 30, 2022

Description

A long-running plugin callable with a rate limit can be called again in the same rate-limit context while it is still running. Metrics are not updated until after a Rule's handler completes, because the current metrics also need its exit code, but rate limiting is checked before dispatching the callable.

Reproduction steps

  1. Build a long-running plugin callable ("dummy" way: time.sleep(10))
  2. Give the callable a command, let's say .ratefail
  3. Apply @plugin.rate(60) or whatever test value you like
  4. Try to call .ratefail twice from a non-admin user (admins bypass rate limits)
  5. Actual result: .ratefail will run twice

Expected behavior

.ratefail cannot be called twice because it is rate limited.

Environment

  • Sopel .version: 7, 8-dev, probably earlier
  • Sopel installed via: any method
  • Python version: any supported
  • Operating system: any supported

Notes

We discussed this on IRC today, and @Exirel already has a plan of attack. Therefore I've assigned the issue to him—but since we already thought there was an open issue about this when there actually wasn't, I thought it was a good idea to make one.

@dgw dgw added the Bug Things to squish; generally used for issues label May 30, 2022
@dgw dgw added this to the 8.0.0 milestone May 30, 2022
@dgw dgw closed this as completed in #2297 Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants