-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
feat(locks): member locks #1172
base: main
Are you sure you want to change the base?
Conversation
@JPBM135 is attempting to deploy a commit to the discordjs Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks decent, but a couple points to consider:
- Can we move the locking into
createCase
? That's really where it matters, and might cut down on code duplication. - I'd really like to see locking handled with a
try...finally
block to ensure the lock is properly disposed once the resource is no longer needed. Unfortunately JS doesn't have a good way to do this, so I'd recommend either just always using atry...finally
block or passing a callback to the locking function with the code that requires the lock; callback is probably the cleanest and most explicit about the lock boundaries.
1- The thing about moving into 2- I couldn't find a good way to do this either, but I will deff give another look into it |
Found a solution for this, implemented the member lock in the command handler, only using extends when needed |
Neat that's pretty clean. |
9d82acd
to
cab5360
Compare
Why?
Duplicated actions have been a rare but occurrent problem, this PR uses Redis to create a one-time member lock to prevent more than one mod to action a member at the same time
Fixes #1149
Const
MEMBER_LOCK_EXPIRE_SECONDS
using 20 seconds is the value I found to be between the 15 seconds action timer and a too-big valueTODO:
try...finally
style