Improve application command logging

ApplicationCommands/BirthdayModule.cs

@@ -85,7 +85,7 @@ public class BirthdayModule : BotModuleBase {
     public async Task CmdGetBday([Summary(description: "Optional: The user's birthday to look up.")] SocketGuildUser? user = null) {
         var self = user is null;
         if (self) user = (SocketGuildUser)Context.User;
-        var targetdata = await GuildUserConfiguration.LoadAsync(Context.Guild.Id, user!.Id).ConfigureAwait(false);
+        var targetdata = await user!.GetConfigAsync().ConfigureAwait(false);
 
         if (!targetdata.IsKnown) {
             if (self) await RespondAsync(":x: You do not have your birthday registered.", ephemeral: true).ConfigureAwait(false);
@@ -93,7 +93,7 @@ public class BirthdayModule : BotModuleBase {
             return;
         }
 
-        await RespondAsync($"{Common.FormatName(user, false)}: `{FormatDate(targetdata.BirthMonth, targetdata.BirthDay)}`" +
+        await RespondAsync($"{Common.FormatName(user!, false)}: `{FormatDate(targetdata.BirthMonth, targetdata.BirthDay)}`" +
             (targetdata.TimeZone == null ? "" : $" - {targetdata.TimeZone}")).ConfigureAwait(false);
     }
 
@@ -103,7 +103,7 @@ public class BirthdayModule : BotModuleBase {
     [SlashCommand("show-nearest", HelpCmdNearest)]
     public async Task CmdShowNearest() {
         if (!await HasMemberCacheAsync(Context.Guild).ConfigureAwait(false)) {
-            await RespondAsync(MemberCacheEmptyError, ephemeral: true);
+            await RespondAsync(MemberCacheEmptyError, ephemeral: true).ConfigureAwait(false);
             return;
         }
 
@@ -121,7 +121,7 @@ public class BirthdayModule : BotModuleBase {
                 await RespondAsync(msg).ConfigureAwait(false);
                 hasOutputOneLine = true;
             } else {
-                await Context.Channel.SendMessageAsync(msg).ConfigureAwait(false);
+                await ReplyAsync(msg).ConfigureAwait(false);
             }
         }
 
@@ -178,7 +178,7 @@ public class BirthdayModule : BotModuleBase {
     [SlashCommand("export", HelpPfxModOnly + HelpCmdExport)]
     public async Task CmdExport([Summary(description: "Specify whether to export the list in CSV format.")] bool asCsv = false) {
         if (!await HasMemberCacheAsync(Context.Guild)) {
-            await RespondAsync(MemberCacheEmptyError).ConfigureAwait(false);
+            await RespondAsync(MemberCacheEmptyError, ephemeral: true).ConfigureAwait(false);
             return;
         }
 

ApplicationCommands/BotModuleBase.cs

@@ -1,5 +1,4 @@
-using BirthdayBot.Data;
+using Discord.Interactions;
-using Discord.Interactions;
 using NodaTime;
 using System.Collections.ObjectModel;
 using System.Diagnostics.CodeAnalysis;

ApplicationCommands/ConfigModule.cs

@@ -102,21 +102,20 @@ public class ConfigModule : BotModuleBase {
     [Group("block", HelpCmdBlocking)]
     public class SubCmdsConfigBlocking : BotModuleBase {
         [SlashCommand("add-block", HelpPfxModOnly + "Add a user to the block list.")]
-        public Task CmdAddBlock([Summary(description: "The user to block.")] SocketGuildUser user) => DoBlocklist(user.Id, true);
+        public Task CmdAddBlock([Summary(description: "The user to block.")] SocketGuildUser user) => UpdateBlockAsync(user, true);
 
         [SlashCommand("remove-block", HelpPfxModOnly + "Remove a user from the block list.")]
-        public Task CmdDelBlock([Summary(description: "The user to unblock.")] SocketGuildUser user) => DoBlocklist(user.Id, false);
+        public Task CmdDelBlock([Summary(description: "The user to unblock.")] SocketGuildUser user) => UpdateBlockAsync(user, false);
 
-        private async Task DoBlocklist(ulong userId, bool setting) {
+        private async Task UpdateBlockAsync(SocketGuildUser user, bool setting) {
             var gconf = await Context.Guild.GetConfigAsync().ConfigureAwait(false);
-            bool already = setting == await gconf.IsUserBlockedAsync(userId).ConfigureAwait(false);
+            bool already = setting == await gconf.IsUserBlockedAsync(user.Id).ConfigureAwait(false);
             if (already) {
-                // TODO bug: this may sometimes be misleading when in moderated mode
+                await RespondAsync($":white_check_mark: User is already {(setting ? "" : "not ")}blocked.").ConfigureAwait(false);
-                await ReplyAsync($":white_check_mark: User is already {(setting ? "" : "not ")}blocked.").ConfigureAwait(false);
             } else {
-                if (setting) await gconf.BlockUserAsync(userId).ConfigureAwait(false);
+                if (setting) await gconf.BlockUserAsync(user.Id).ConfigureAwait(false);
-                else await gconf.UnblockUserAsync(userId).ConfigureAwait(false);
+                else await gconf.UnblockUserAsync(user.Id).ConfigureAwait(false);
-                await ReplyAsync($":white_check_mark: User has been {(setting ? "" : "un")}blocked.");
+                await RespondAsync($":white_check_mark: {Common.FormatName(user, false)} has been {(setting ? "" : "un")}blocked.");
             }
         }
 
@@ -208,7 +207,7 @@ public class ConfigModule : BotModuleBase {
             if (conf.AnnounceMessages.Item2 != null) {
                 em = em.AddField("Multi", prepareAnnouncePreview(conf.AnnounceMessages.Item2));
             }
-            await channel.SendMessageAsync(embed: em.Build()).ConfigureAwait(false);
+            await ReplyAsync(embed: em.Build()).ConfigureAwait(false);
         }
     }
 

ApplicationCommands/HelpModule.cs

@@ -21,13 +21,13 @@ public class HelpModule : BotModuleBase {
         $"` ⤷check` - {ConfigModule.HelpCmdCheck}\n" +
         $"` ⤷announce` - {ConfigModule.HelpCmdAnnounce}\n" +
         $"`  ⤷` See also: `/config announce help`.\n" +
-        $"` ⤷blocking` - {ConfigModule.HelpCmdBlocking}\n" +
+        $"` ⤷block` - {ConfigModule.HelpCmdBlocking}\n" +
         $"`  ⤷add-block`, `⤷remove-block`, `⤷set-moderated`\n" +
         $"` ⤷role` - {ConfigModule.HelpCmdRole}\n" +
         $"`  ⤷set-birthday-role`, `⤷set-moderator-role`\n" +
         $"`/override` - {BirthdayOverrideModule.HelpCmdOverride}\n" +
         $"` ⤷set-birthday`, `⤷set-timezone`, `⤷remove`\n" +
-        "**Caution:** Skipping optional parameters may __remove__ their configuration.";
+        "**Caution:** Skipping optional parameters __removes__ their configuration.";
 
     [SlashCommand("help", "Show an overview of available commands.")]
     public async Task CmdHelp() {

BirthdayBot.csproj

@@ -22,7 +22,7 @@
 
   <ItemGroup>
     <PackageReference Include="CommandLineParser" Version="2.8.0" />
-    <PackageReference Include="Discord.Net" Version="3.3.2" />
+    <PackageReference Include="Discord.Net" Version="3.4.1" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0" />
     <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
     <PackageReference Include="NodaTime" Version="3.0.9" />

ShardInstance.cs

@@ -1,4 +1,5 @@
-using BirthdayBot.BackgroundServices;
+using BirthdayBot.ApplicationCommands;
+using BirthdayBot.BackgroundServices;
 using BirthdayBot.Data;
 using Discord.Interactions;
 using Discord.Net;
@@ -11,7 +12,7 @@ namespace BirthdayBot;
 /// <summary>
 /// Single shard instance for Birthday Bot. This shard independently handles all input and output to Discord.
 /// </summary>
-public class ShardInstance : IDisposable {
+public sealed class ShardInstance : IDisposable {
     private readonly ShardManager _manager;
     private readonly ShardBackgroundWorker _background;
     private readonly Dictionary<string, CommandHandler> _textDispatch;
@@ -51,6 +52,7 @@ public class ShardInstance : IDisposable {
 
         // Background task constructor begins background processing immediately.
         _background = new ShardBackgroundWorker(this);
+        Log(nameof(ShardInstance), "Instance created.");
     }
 
     /// <summary>
@@ -66,19 +68,11 @@ public class ShardInstance : IDisposable {
     /// Does all necessary steps to stop this shard, including canceling background tasks and disconnecting.
     /// </summary>
     public void Dispose() {
-        // TODO are these necessary?
-        _interactionService.SlashCommandExecuted -= InteractionService_SlashCommandExecuted;
-        DiscordClient.InteractionCreated -= DiscordClient_InteractionCreated;
-        DiscordClient.Log -= Client_Log;
-        DiscordClient.Ready -= Client_Ready;
-        DiscordClient.MessageReceived -= Client_MessageReceived;
-
-        _interactionService.Dispose();
         _background.Dispose();
         DiscordClient.LogoutAsync().Wait(5000);
-        DiscordClient.StopAsync().Wait(5000);
         DiscordClient.Dispose();
-        Log(nameof(ShardInstance), "Shard instance disposed.");
+        _interactionService.Dispose();
+        Log(nameof(ShardInstance), "Instance disposed.");
     }
 
     public void Log(string source, string message) => Program.Log($"Shard {ShardId:00}] [{source}", message);
@@ -114,16 +108,20 @@ public class ShardInstance : IDisposable {
     /// Additionally, sets the shard's status to display the help command.
     /// </summary>
     private async Task Client_Ready() {
-        await DiscordClient.SetGameAsync(CommandPrefix + "help");
+        // TODO get rid of this eventually? or change it to something fun...
+        await DiscordClient.SetGameAsync("/help");
 
 #if !DEBUG
         // Update slash/interaction commands
-        if (ShardId == 0) await _interactionService.RegisterCommandsGloballyAsync(true).ConfigureAwait(false);
+        if (ShardId == 0) {
+            await _interactionService.RegisterCommandsGloballyAsync(true).ConfigureAwait(false);
+            Log(nameof(ShardInstance), "Updated global command registration.");
+        }
 #else
         // Debug: Register our commands locally instead, in each guild we're in
         foreach (var g in DiscordClient.Guilds) {
             await _interactionService.RegisterCommandsToGuildAsync(g.Id, true).ConfigureAwait(false);
-            // TODO log?
+            Log(nameof(ShardInstance), $"Updated DEBUG command registration in guild {g.Id}.");
         }
 #endif
     }
@@ -168,48 +166,59 @@ public class ShardInstance : IDisposable {
         }
     }
 
-    private Task InteractionService_SlashCommandExecuted(SlashCommandInfo arg1, IInteractionContext arg2, IResult arg3) {
+    // Slash command preparation and invocation
-        // TODO extract command and subcommands to log here
-        Log("Interaction", $"/{arg1.Name} executed by {arg2.Guild.Name}!{arg2.User}.");
-        if (!arg3.IsSuccess) {
-            Log("Interaction", Enum.GetName(typeof(InteractionCommandError), arg3.Error) + ": " + arg3.ErrorReason);
-        }
-
-        // TODO finish this up
-        return Task.CompletedTask;
-    }
-    
     private async Task DiscordClient_InteractionCreated(SocketInteraction arg) {
-        // TODO verify this whole thing - it's a hastily done mash-up of example code and my old code
         var context = new SocketInteractionContext(DiscordClient, arg);
 
-        // Specific reply for DM messages
+        // Blocklist/moderated check
-        if (context.Channel is not SocketGuildChannel) {
+        // TODO convert to precondition
-            Log("Interaction", $"DM interaction. User ID {context.User.Id}, {context.User}");
+        var gconf = await GuildConfiguration.LoadAsync(context.Guild.Id, false);
-            await arg.RespondAsync("DMs are not supported by this bot.").ConfigureAwait(false);
+        if (context.Channel is SocketGuildChannel) { // Check only if in a guild context
+            if (!gconf!.IsBotModerator((SocketGuildUser)arg.User)) { // Moderators exempted from this check
+                if (await gconf.IsUserBlockedAsync(arg.User.Id)) {
+                    Log("Interaction", $"Interaction blocked per guild policy for {context.Guild}!{context.User}");
+                    await arg.RespondAsync(BotModuleBase.AccessDeniedError, ephemeral: true).ConfigureAwait(false);
                     return;
                 }
-
-        // Blocklist/moderated check
-        var gconf = await GuildConfiguration.LoadAsync(context.Guild.Id, false);
-        if (!gconf!.IsBotModerator((SocketGuildUser)arg.User)) // Except if moderator
-        {
-            if (await gconf.IsUserBlockedAsync(arg.User.Id)) {
-                Log("Interaction", $"Blocking interaction per guild policy. User ID {context.User.Id}, {context.User}");
-                await arg.RespondAsync(ApplicationCommands.BotModuleBase.AccessDeniedError, ephemeral: true).ConfigureAwait(false);
-                return;
             }
         }
 
         try {
-            await _interactionService.ExecuteCommandAsync(context, _services);
+            await _interactionService.ExecuteCommandAsync(context, _services).ConfigureAwait(false);
-        } catch (Exception ex) {
+        } catch (Exception e) {
-            Console.WriteLine(ex);
+            Log(nameof(DiscordClient_InteractionCreated), $"Unhandled exception. {e}");
-
+            // TODO when implementing proper error logging, see here
-            // If a Slash Command execution fails it is most likely that the original interaction acknowledgement will persist. It is a good idea to delete the original
-            // response, or at least let the user know that something went wrong during the command execution.
             if (arg.Type == InteractionType.ApplicationCommand)
-                await arg.GetOriginalResponseAsync().ContinueWith(async (msg) => await msg.Result.DeleteAsync());
+                if (arg.HasResponded) await arg.ModifyOriginalResponseAsync(prop => prop.Content = ":warning: An unknown error occured.");
         }
     }
+
+    // Slash command logging and failed execution handling
+    private async Task InteractionService_SlashCommandExecuted(SlashCommandInfo info, IInteractionContext context, IResult result) {
+        string sender;
+        if (context.Guild != null) {
+            sender = $"{context.Guild}!{context.User}";
+        } else {
+            sender = $"{context.User} in non-guild context";
+        }
+        var logresult = $"{(result.IsSuccess ? "Success" : "Fail")}: `/{info}` by {sender}.";
+
+        if (result.Error != null) {
+            // Additional log information with error detail
+            logresult += Enum.GetName(typeof(InteractionCommandError), result.Error) + ": " + result.ErrorReason;
+
+            // Specific responses to errors, if necessary
+            if (result.Error == InteractionCommandError.UnmetPrecondition && result.ErrorReason == RequireBotModeratorAttribute.FailMsg) {
+                await context.Interaction.RespondAsync(RequireBotModeratorAttribute.Reply, ephemeral: true).ConfigureAwait(false);
+            } else {
+                // Generic error response
+                var ia = context.Interaction;
+                if (ia.HasResponded) await ia.ModifyOriginalResponseAsync(p => p.Content = InternalError).ConfigureAwait(false);
+                else await ia.RespondAsync(InternalError).ConfigureAwait(false);
+                // TODO when implementing proper error logging, see here
+            }
+        }
+
+        Log("Command", logresult);
+    }
 }