Merge pull request #66 from Mimickal/mutex-fix

Author: Mimickal

.gitignore

@@ -3,4 +3,5 @@ todo.txt
 *.log
 *.swp
 *.sqlite3
-dev-config.json
+*-config.json
+invite

docs/hosting.md

@@ -8,6 +8,11 @@ like https://github.com/nvm-sh/nvm.
 This guide assumes you're hosting on a Linux distro with `systemd`. The bot will
 work on other platforms, but you're on your own figuring that out.
 
+## Bot account setup
+
+Your bot account needs the privileged "Server Members Intent" enabled.
+It also needs [these permissions](../README.md#permissions).
+
 ## Running as a user (in dev-mode)
 
 Quick and easy. Also (mostly) platform-independent!

package-lock.json

@@ -1,11 +1,12 @@
 {
   "name": "reactionrolebot",
-  "version": "2.0.2",
+  "version": "2.0.3",
   "description": "I'm a basic, no BS Discord bot that can assign and unassign roles using message reactions. I am completely free and open source, and always will be.",
   "main": "src/main.js",
   "dependencies": {
+    "async-mutex": "^0.3.2",
     "discord-command-registry": "^1.2.2",
-    "discord.js": "^13.3.1",
+    "discord.js": "^13.7.0",
     "knex": "^0.20.13",
     "lodash": "^4.17.20",
     "minimist": "^1.2.6",

package.json

@@ -12,6 +12,7 @@ const Perms = require('discord.js').Permissions.FLAGS;
 const commands = require('./commands');
 const config = require('./config');
 const database = require('./database');
+const UserMutex = require('./mutex');
 const logger = require('./logger');
 const {
 	detail,
@@ -20,6 +21,22 @@ const {
 	unindent,
 } = require('./util');
 
+/**
+ * Allows us to "lock" a user to prevent multiple events from trying to update
+ * their roles at the same time.
+ *
+ * Any event that modifies a user should acquire a lock on that user first.
+ * The corresponding `GUILD_MEMBER_UPDATE` event will release the lock. If that
+ * event doesn't fire for some reason, `UserMutex` has a fallback timer to
+ * release the lock anyway.
+ *
+ * Discord API request promises resolve when Discord *acknowledges* them, **not**
+ * when it *applies* them. Because of this, Discord.js does not update its
+ * internal user role cache until it receives the corresponding
+ * `GUILD_MEMBER_UPDATE` event.
+ */
+const USER_MUTEX = new UserMutex();
+
 const REQUIRED_PERMISSIONS = Object.freeze({
 	[Perms.ADD_REACTIONS]:        'Add Reactions',
 	[Perms.MANAGE_MESSAGES]:      'Manage Messages',
@@ -83,6 +100,17 @@ async function onGuildLeave(guild) {
 	}
 }
 
+/**
+ * Event handler for when a guild member is updated.
+ * Releases the mutex lock on the user, since this event is fired once a user's
+ * roles are fully updated.
+ *
+ * See docs for: {@link USER_MUTEX}
+ */
+async function onGuildMemberUpdate(old_member, new_member) {
+	USER_MUTEX.unlock(new_member);
+}
+
 /**
  * Event handler for receiving some kind of interaction.
  * Logs the interaction and passes it on to the command handler.
@@ -155,12 +183,14 @@ async function onReactionAdd(reaction, react_user) {
 	const member = await reaction.message.guild.members.fetch(react_user.id);
 
 	// Remove mutually exclusive roles from user
+	// FIXME this database call should optionally take an array
 	const mutex_roles = lodash.flatMap(
 		await Promise.all(role_ids.map(role_id => database.getMutexRoles({
 			guild_id: reaction.message.guild.id,
 			role_id: role_id,
 		})))
 	);
+	await USER_MUTEX.lock(member); // See USER_MUTEX comment
 	try {
 		await member.roles.remove(mutex_roles, 'Role bot removal (mutex)');
 		await member.roles.add(role_ids, 'Role bot assignment');
@@ -224,8 +254,16 @@ async function onReactionRemove(reaction, react_user) {
 		return reaction.message.react(emoji);
 	}
 
+	await USER_MUTEX.lock(react_user); // see USER_MUTEX comment
 	try {
 		const member = await reaction.message.guild.members.fetch(react_user.id);
+
+		// onGuildMemberUpdate won't fire if we don't actually change roles
+		if (!role_ids.some(role_id => member.roles.cache.has(role_id))) {
+			USER_MUTEX.unlock(react_user);
+			return;
+		}
+
 		await member.roles.remove(role_ids, 'Role bot removal');
 		logger.info(`Removed Roles ${stringify(role_ids)} from ${stringify(react_user)}`);
 	} catch (err) {
@@ -303,6 +341,7 @@ async function precache(client) {
 module.exports = {
 	onGuildJoin,
 	onGuildLeave,
+	onGuildMemberUpdate,
 	onInteraction,
 	onMessageBulkDelete,
 	onMessageDelete,

src/events.js

@@ -20,11 +20,13 @@ const client = new Discord.Client({
 	intents: [
 		Discord.Intents.FLAGS.GUILDS,
 		Discord.Intents.FLAGS.GUILD_EMOJIS_AND_STICKERS,
+		Discord.Intents.FLAGS.GUILD_MEMBERS,
 		Discord.Intents.FLAGS.GUILD_MESSAGES,
 		Discord.Intents.FLAGS.GUILD_MESSAGE_REACTIONS,
 	],
 	partials: [
 		// https://discordjs.guide/popular-topics/reactions.html#listening-for-reactions-on-old-messages
+		Discord.Constants.PartialTypes.GUILD_MEMBER,
 		Discord.Constants.PartialTypes.MESSAGE,
 		Discord.Constants.PartialTypes.CHANNEL,
 		Discord.Constants.PartialTypes.REACTION,
@@ -42,6 +44,7 @@ const Events = Discord.Constants.Events;
 client.on(Events.CLIENT_READY, events.onReady);
 client.on(Events.GUILD_CREATE, events.onGuildJoin);
 client.on(Events.GUILD_DELETE, events.onGuildLeave);
+client.on(Events.GUILD_MEMBER_UPDATE, events.onGuildMemberUpdate);
 client.on(Events.INTERACTION_CREATE, events.onInteraction);
 client.on(Events.MESSAGE_BULK_DELETE, events.onMessageBulkDelete);
 client.on(Events.MESSAGE_DELETE, events.onMessageDelete);