Properly clean up after proxy operations

PluralKit.Bot/Services/ProxyService.cs

@@ -28,7 +28,7 @@ namespace PluralKit.Bot
         public string ProxyName => Member.Name + (System.Tag != null ? " " + System.Tag : "");
     }
 
-    class ProxyService {
+    class ProxyService: IDisposable {
         private IDiscordClient _client;
         private DbConnectionFactory _conn;
         private LogChannelService _logChannel;
@@ -38,6 +38,8 @@ namespace PluralKit.Bot
         private IMetrics _metrics;
         private ILogger _logger;
 
+        private HttpClient _httpClient;
+        
         public ProxyService(IDiscordClient client, WebhookCacheService webhookCache, DbConnectionFactory conn, LogChannelService logChannel, MessageStore messageStorage, EmbedService embeds, IMetrics metrics, ILogger logger)
         {
             _client = client;
@@ -48,6 +50,8 @@ namespace PluralKit.Bot
             _embeds = embeds;
             _metrics = metrics;
             _logger = logger.ForContext<ProxyService>();
+            
+            _httpClient = new HttpClient();
         }
 
         private ProxyMatch GetProxyTagMatch(string message, IEnumerable<ProxyDatabaseResult> potentials)
@@ -157,19 +161,23 @@ namespace PluralKit.Bot
             }
             catch (InvalidOperationException)
             {
+                // TODO: does this leak internal stuff in the (now-invalid) client?
+                
                 // webhook was deleted or invalid
                 webhook = await _webhookCache.InvalidateAndRefreshWebhook(webhook);
                 client = new DiscordWebhookClient(webhook);
             }
 
+            // TODO: clean this entire block up
+            using (client)
+            {
                 ulong messageId;
 
                 try
                 {
                     if (attachment != null)
                     {
-                    using (var http = new HttpClient())
+                        using (var stream = await _httpClient.GetStreamAsync(attachment.Url))
-                    using (var stream = await http.GetStreamAsync(attachment.Url))
                         {
                             messageId = await client.SendFileAsync(stream, filename: attachment.Filename, text: text,
                                 username: username, avatarUrl: avatarUrl);
@@ -180,14 +188,16 @@ namespace PluralKit.Bot
                         messageId = await client.SendMessageAsync(text, username: username, avatarUrl: avatarUrl);
                     }
 
-                _logger.Information("Invoked webhook {Webhook} in channel {Channel}", webhook.Id, webhook.ChannelId);
+                    _logger.Information("Invoked webhook {Webhook} in channel {Channel}", webhook.Id,
+                        webhook.ChannelId);
 
                     // Log it in the metrics
                     _metrics.Measure.Meter.Mark(BotMetrics.MessagesProxied, "success");
                 }
                 catch (HttpException e)
                 {
-                _logger.Warning(e, "Error invoking webhook {Webhook} in channel {Channel}", webhook.Id, webhook.ChannelId);
+                    _logger.Warning(e, "Error invoking webhook {Webhook} in channel {Channel}", webhook.Id,
+                        webhook.ChannelId);
 
                     // Log failure in metrics and rethrow (we still need to cancel everything else)
                     _metrics.Measure.Meter.Mark(BotMetrics.MessagesProxied, "failure");
@@ -198,6 +208,7 @@ namespace PluralKit.Bot
                 // doesn't work if there's no permission to)
                 return messageId;
             }
+        }
 
         public Task HandleReactionAddedAsync(Cacheable<IUserMessage, ulong> message, ISocketMessageChannel channel, SocketReaction reaction)
         {
@@ -269,5 +280,10 @@ namespace PluralKit.Bot
             // since Discord blocks webhooks containing the word "Clyde"... for some reason. /shrug
             return name.Substring(0, match.Index + 1) + '\u200A' + name.Substring(match.Index + 1);
         }
+
+        public void Dispose()
+        {
+            _httpClient.Dispose();
+        }
     }
 }

PluralKit.Bot/Services/WebhookCacheService.cs

@@ -3,6 +3,7 @@ using System.Collections.Concurrent;
 using System.Linq;
 using System.Threading.Tasks;
 using Discord;
+using Serilog;
 
 namespace PluralKit.Bot
 {
@@ -13,9 +14,12 @@ namespace PluralKit.Bot
         private IDiscordClient _client;
         private ConcurrentDictionary<ulong, Lazy<Task<IWebhook>>> _webhooks;
 
-        public WebhookCacheService(IDiscordClient client)
+        private ILogger _logger;
+
+        public WebhookCacheService(IDiscordClient client, ILogger logger)
         {
-            this._client = client;
+            _client = client;
+            _logger = logger.ForContext<WebhookCacheService>();
             _webhooks = new ConcurrentDictionary<ulong, Lazy<Task<IWebhook>>>();
         }
 
@@ -43,16 +47,27 @@ namespace PluralKit.Bot
 
         public async Task<IWebhook> InvalidateAndRefreshWebhook(IWebhook webhook)
         {
-            _webhooks.TryRemove(webhook.Channel.Id, out _);
+            _logger.Information("Refreshing webhook for channel {Channel}", webhook.ChannelId);
-            return await GetWebhook(webhook.Channel.Id);
+            
+            _webhooks.TryRemove(webhook.ChannelId, out _);
+            return await GetWebhook(webhook.ChannelId);
         }
 
         private async Task<IWebhook> GetOrCreateWebhook(ITextChannel channel) =>
             await FindExistingWebhook(channel) ?? await DoCreateWebhook(channel);
 
-        private async Task<IWebhook> FindExistingWebhook(ITextChannel channel) => (await channel.GetWebhooksAsync()).FirstOrDefault(IsWebhookMine);
+        private async Task<IWebhook> FindExistingWebhook(ITextChannel channel)
+        {
+            _logger.Debug("Finding webhook for channel {Channel}", channel.Id);
+            return (await channel.GetWebhooksAsync()).FirstOrDefault(IsWebhookMine);
+        }
+
+        private async Task<IWebhook> DoCreateWebhook(ITextChannel channel)
+        {
+            _logger.Information("Creating new webhook for channel {Channel}", channel.Id);
+            return await channel.CreateWebhookAsync(WebhookName);
+        }
 
-        private async Task<IWebhook> DoCreateWebhook(ITextChannel channel) => await channel.CreateWebhookAsync(WebhookName);
         private bool IsWebhookMine(IWebhook arg) => arg.Creator.Id == _client.CurrentUser.Id && arg.Name == WebhookName;
 
         public int CacheSize => _webhooks.Count;