Improve error handling and reporting after command rewrite
This commit is contained in:
		@@ -85,7 +85,8 @@ namespace PluralKit.Bot
 | 
				
			|||||||
            .AddSingleton<IDiscordClient, DiscordShardedClient>(_ => new DiscordShardedClient(new DiscordSocketConfig
 | 
					            .AddSingleton<IDiscordClient, DiscordShardedClient>(_ => new DiscordShardedClient(new DiscordSocketConfig
 | 
				
			||||||
            {
 | 
					            {
 | 
				
			||||||
                MessageCacheSize = 5,
 | 
					                MessageCacheSize = 5,
 | 
				
			||||||
                ExclusiveBulkDelete = true
 | 
					                ExclusiveBulkDelete = true,
 | 
				
			||||||
 | 
					                DefaultRetryMode = RetryMode.AlwaysRetry
 | 
				
			||||||
            }))
 | 
					            }))
 | 
				
			||||||
            .AddSingleton<Bot>()
 | 
					            .AddSingleton<Bot>()
 | 
				
			||||||
            .AddTransient<CommandTree>()
 | 
					            .AddTransient<CommandTree>()
 | 
				
			||||||
@@ -296,9 +297,10 @@ namespace PluralKit.Bot
 | 
				
			|||||||
            logger.Error(e, "Exception in bot event handler");
 | 
					            logger.Error(e, "Exception in bot event handler");
 | 
				
			||||||
            
 | 
					            
 | 
				
			||||||
            var evt = new SentryEvent(e);
 | 
					            var evt = new SentryEvent(e);
 | 
				
			||||||
            SentrySdk.CaptureEvent(evt, scope);
 | 
					 | 
				
			||||||
            
 | 
					            
 | 
				
			||||||
            Console.Error.WriteLine(e);
 | 
					            // Don't blow out our Sentry budget on sporadic not-our-problem erorrs
 | 
				
			||||||
 | 
					            if (e.IsOurProblem())
 | 
				
			||||||
 | 
					                SentrySdk.CaptureEvent(evt, scope);
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
    
 | 
					    
 | 
				
			||||||
@@ -359,7 +361,17 @@ namespace PluralKit.Bot
 | 
				
			|||||||
                        "select systems.* from systems, accounts where accounts.uid = @Id and systems.id = accounts.system",
 | 
					                        "select systems.* from systems, accounts where accounts.uid = @Id and systems.id = accounts.system",
 | 
				
			||||||
                        new {Id = msg.Author.Id});
 | 
					                        new {Id = msg.Author.Id});
 | 
				
			||||||
 | 
					
 | 
				
			||||||
                await _tree.ExecuteCommand(new Context(_services, msg, argPos, system));
 | 
					                try
 | 
				
			||||||
 | 
					                {
 | 
				
			||||||
 | 
					                    await _tree.ExecuteCommand(new Context(_services, msg, argPos, system));
 | 
				
			||||||
 | 
					                }
 | 
				
			||||||
 | 
					                catch (Exception e)
 | 
				
			||||||
 | 
					                {
 | 
				
			||||||
 | 
					                    await HandleCommandError(msg, e);
 | 
				
			||||||
 | 
					                    // HandleCommandError only *reports* the error, we gotta pass it through to the parent
 | 
				
			||||||
 | 
					                    // error handler by rethrowing:
 | 
				
			||||||
 | 
					                    throw;
 | 
				
			||||||
 | 
					                }
 | 
				
			||||||
            }
 | 
					            }
 | 
				
			||||||
            else
 | 
					            else
 | 
				
			||||||
            {
 | 
					            {
 | 
				
			||||||
@@ -375,6 +387,22 @@ namespace PluralKit.Bot
 | 
				
			|||||||
            }
 | 
					            }
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        private async Task HandleCommandError(SocketUserMessage msg, Exception exception)
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					            // This function *specifically* handles reporting a command execution error to the user.
 | 
				
			||||||
 | 
					            // We'll fetch the event ID and send a user-facing error message.
 | 
				
			||||||
 | 
					            // ONLY IF this error's actually our problem. As for what defines an error as "our problem",
 | 
				
			||||||
 | 
					            // check the extension method :)
 | 
				
			||||||
 | 
					            if (exception.IsOurProblem())
 | 
				
			||||||
 | 
					            {
 | 
				
			||||||
 | 
					                var eid = _services.GetService<EventIdProvider>().EventId;
 | 
				
			||||||
 | 
					                await msg.Channel.SendMessageAsync(
 | 
				
			||||||
 | 
					                    $"{Emojis.Error} Internal error occurred. Please join the support server (https://discord.gg/PczBt78), and send the developer this ID: `{eid}`");
 | 
				
			||||||
 | 
					            }
 | 
				
			||||||
 | 
					            
 | 
				
			||||||
 | 
					            // If not, don't care. lol.
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        private void RegisterMessageMetrics(SocketMessage msg)
 | 
					        private void RegisterMessageMetrics(SocketMessage msg)
 | 
				
			||||||
        {
 | 
					        {
 | 
				
			||||||
            _metrics.Measure.Meter.Mark(BotMetrics.MessagesReceived);
 | 
					            _metrics.Measure.Meter.Mark(BotMetrics.MessagesReceived);
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -86,6 +86,11 @@ namespace PluralKit.Bot.CommandSystem
 | 
				
			|||||||
            {
 | 
					            {
 | 
				
			||||||
                await Reply($"{Emojis.Error} {e.Message}");
 | 
					                await Reply($"{Emojis.Error} {e.Message}");
 | 
				
			||||||
            }
 | 
					            }
 | 
				
			||||||
 | 
					            catch (TimeoutException e)
 | 
				
			||||||
 | 
					            {
 | 
				
			||||||
 | 
					                // Got a complaint the old error was a bit too patronizing. Hopefully this is better?
 | 
				
			||||||
 | 
					                await Reply($"{Emojis.Error} Operation timed out, sorry. Try again, perhaps?");
 | 
				
			||||||
 | 
					            }
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        public async Task<IUser> MatchUser()
 | 
					        public async Task<IUser> MatchUser()
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -2,9 +2,11 @@ using System;
 | 
				
			|||||||
using System.Globalization;
 | 
					using System.Globalization;
 | 
				
			||||||
using System.Linq;
 | 
					using System.Linq;
 | 
				
			||||||
using System.Net.Http;
 | 
					using System.Net.Http;
 | 
				
			||||||
 | 
					using System.Net.Sockets;
 | 
				
			||||||
using System.Text.RegularExpressions;
 | 
					using System.Text.RegularExpressions;
 | 
				
			||||||
using System.Threading.Tasks;
 | 
					using System.Threading.Tasks;
 | 
				
			||||||
using Discord;
 | 
					using Discord;
 | 
				
			||||||
 | 
					using Discord.Net;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
using PluralKit.Core;
 | 
					using PluralKit.Core;
 | 
				
			||||||
using Image = SixLabors.ImageSharp.Image;
 | 
					using Image = SixLabors.ImageSharp.Image;
 | 
				
			||||||
@@ -117,6 +119,24 @@ namespace PluralKit.Bot
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
        public static async Task<bool> HasPermission(this IChannel channel, ChannelPermission permission) =>
 | 
					        public static async Task<bool> HasPermission(this IChannel channel, ChannelPermission permission) =>
 | 
				
			||||||
            (await PermissionsIn(channel)).Has(permission);
 | 
					            (await PermissionsIn(channel)).Has(permission);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        public static bool IsOurProblem(this Exception e)
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					            // This function filters out sporadic errors out of our control from being reported to Sentry
 | 
				
			||||||
 | 
					            // otherwise we'd blow out our error reporting budget as soon as Discord takes a dump, or something.
 | 
				
			||||||
 | 
					            
 | 
				
			||||||
 | 
					            // Discord server errors are *not our problem*
 | 
				
			||||||
 | 
					            if (e is HttpException he && ((int) he.HttpCode) >= 500) return false;
 | 
				
			||||||
 | 
					            
 | 
				
			||||||
 | 
					            // Socket errors are *not our problem*
 | 
				
			||||||
 | 
					            if (e is SocketException) return false;
 | 
				
			||||||
 | 
					            
 | 
				
			||||||
 | 
					            // Tasks being cancelled for whatver reason are, you guessed it, also not our problem.
 | 
				
			||||||
 | 
					            if (e is TaskCanceledException) return false;
 | 
				
			||||||
 | 
					            
 | 
				
			||||||
 | 
					            // This may expanded at some point.
 | 
				
			||||||
 | 
					            return true;
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    /// <summary>
 | 
					    /// <summary>
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user