Repository: python-discord/bot
Issue: Infraction DMs fail for all bans/kicks due to order change

In a recent PR, we merged a commit that changed the order in which our members are notified of their infraction and we actually the infraction: Instead of sending the DM first and applying the infraction after, we now apply the infraction first and try to DM the member after.

However, in the case of ban/kick infractions, this means that we try to send a DM to a member that has just been kicked off the server. That will obviously not work, as the bot can only send DMs to users it shares a server with.

This is the commit that changed the order: 32a5bf97e31addb32d17bd0479bc4cf2f4dd9eb7
It was merged in PR #864

The reason this change was made was to suppress DMs when we actually fail to apply the infraction on the Discord side. However, we only know if that's going to fail once we've made the request to Discord and we need to send the DM (in the case of kick/ban infractions) before we do that.

I think that we should just change the DM order back to what it was before.

Changed files:
bot/cogs/moderation/scheduler.py

diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index f0a3ad1b..b03d8953 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -106,6 +106,27 @@ class InfractionScheduler(Scheduler):
         log_content = None
         failed = False
 
+        # DM the user about the infraction if it's not a shadow/hidden infraction.
+        # This needs to happen before we apply the infraction, as the bot cannot
+        # send DMs to user that it doesn't share a guild with. If we were to
+        # apply kick/ban infractions first, this would mean that we'd make it
+        # impossible for us to deliver a DM. See python-discord/bot#982.
+        if not infraction["hidden"]:
+            dm_result = f"{constants.Emojis.failmail} "
+            dm_log_text = "\\nDM: **Failed**"
+
+            # Sometimes user is a discord.Object; make it a proper user.
+            try:
+                if not isinstance(user, (discord.Member, discord.User)):
+                    user = await self.bot.fetch_user(user.id)
+            except discord.HTTPException as e:
+                log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})")
+            else:
+                # Accordingly display whether the user was successfully notified via DM.
+                if await utils.notify_infraction(user, infr_type, expiry, reason, icon):
+                    dm_result = ":incoming_envelope: "
+                    dm_log_text = "\\nDM: Sent"
+
         if infraction["actor"] == self.bot.user.id:
             log.trace(
                 f"Infraction #{id_} actor is bot; including the reason in the confirmation message."
@@ -150,27 +171,7 @@ class InfractionScheduler(Scheduler):
                     log.exception(log_msg)
                 failed = True
 
-        # DM the user about the infraction if it's not a shadow/hidden infraction.
-        # Don't send DM when applying failed.
-        if not infraction["hidden"] and not failed:
-            dm_result = f"{constants.Emojis.failmail} "
-            dm_log_text = "\\nDM: **Failed**"
-
-            # Sometimes user is a discord.Object; make it a proper user.
-            try:
-                if not isinstance(user, (discord.Member, discord.User)):
-                    user = await self.bot.fetch_user(user.id)
-            except discord.HTTPException as e:
-                log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})")
-            else:
-                # Accordingly display whether the user was successfully notified via DM.
-                if await utils.notify_infraction(user, infr_type, expiry, reason, icon):
-                    dm_result = ":incoming_envelope: "
-                    dm_log_text = "\\nDM: Sent"
-
         if failed:
-            dm_log_text = "\\nDM: **Canceled**"
-            dm_result = f"{constants.Emojis.failmail} "
             log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.")
             try:
                 await self.bot.api_client.delete(f"bot/infractions/{id_}")