Execute webhooks directly rather than through D.NET
This commit is contained in:
		| @@ -10,7 +10,6 @@ | ||||
|     </ItemGroup> | ||||
|  | ||||
|     <ItemGroup> | ||||
|       <PackageReference Include="Discord.Net.Webhook" Version="2.1.1" /> | ||||
|       <PackageReference Include="Discord.Net.WebSocket" Version="2.1.1" /> | ||||
|       <PackageReference Include="Humanizer.Core" Version="2.6.2" /> | ||||
|       <PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="2.2.0" /> | ||||
|   | ||||
| @@ -4,7 +4,6 @@ using System.Linq; | ||||
| using System.Net.Http; | ||||
| using System.Threading.Tasks; | ||||
| using Discord; | ||||
| using Discord.Webhook; | ||||
| using Discord.WebSocket; | ||||
|  | ||||
| using Serilog; | ||||
|   | ||||
| @@ -3,11 +3,14 @@ using System.Net.Http; | ||||
| using System.Text.RegularExpressions; | ||||
| using System.Threading.Tasks; | ||||
| using App.Metrics; | ||||
| using App.Metrics.Logging; | ||||
|  | ||||
| using Discord; | ||||
| using Discord.Net; | ||||
| using Discord.Webhook; | ||||
| using Microsoft.Extensions.Caching.Memory; | ||||
|  | ||||
| using Humanizer; | ||||
|  | ||||
| using Newtonsoft.Json; | ||||
| using Newtonsoft.Json.Linq; | ||||
|  | ||||
| using Serilog; | ||||
|  | ||||
| namespace PluralKit.Bot | ||||
| @@ -15,14 +18,12 @@ namespace PluralKit.Bot | ||||
|     public class WebhookExecutorService: IDisposable | ||||
|     { | ||||
|         private WebhookCacheService _webhookCache; | ||||
|         private IMemoryCache _cache; | ||||
|         private ILogger _logger; | ||||
|         private IMetrics _metrics; | ||||
|         private HttpClient _client; | ||||
|  | ||||
|         public WebhookExecutorService(IMemoryCache cache, IMetrics metrics, WebhookCacheService webhookCache, ILogger logger) | ||||
|         public WebhookExecutorService(IMetrics metrics, WebhookCacheService webhookCache, ILogger logger) | ||||
|         { | ||||
|             _cache = cache; | ||||
|             _metrics = metrics; | ||||
|             _webhookCache = webhookCache; | ||||
|             _logger = logger.ForContext<WebhookExecutorService>(); | ||||
| @@ -48,72 +49,44 @@ namespace PluralKit.Bot | ||||
|         private async Task<ulong> ExecuteWebhookInner(IWebhook webhook, string name, string avatarUrl, string content, | ||||
|             IAttachment attachment, bool hasRetried = false) | ||||
|         { | ||||
|             var client = await GetClientFor(webhook); | ||||
|  | ||||
|             try | ||||
|             var mfd = new MultipartFormDataContent(); | ||||
|             mfd.Add(new StringContent(content.Truncate(2000)), "content"); | ||||
|             mfd.Add(new StringContent(FixClyde(name).Truncate(80)), "username"); | ||||
|             if (avatarUrl != null) mfd.Add(new StringContent(avatarUrl), "avatar_url"); | ||||
|  | ||||
|             if (attachment != null) | ||||
|             { | ||||
|                 // If we have an attachment, use the special SendFileAsync method | ||||
|                 if (attachment != null) | ||||
|                     using (var attachmentStream = await _client.GetStreamAsync(attachment.Url)) | ||||
|                     using (_metrics.Measure.Timer.Time(BotMetrics.WebhookResponseTime)) | ||||
|                         return await client.SendFileAsync(attachmentStream, attachment.Filename, content, | ||||
|                             username: FixClyde(name), | ||||
|                             avatarUrl: avatarUrl); | ||||
|  | ||||
|                 // Otherwise, send normally | ||||
|                 return await client.SendMessageAsync(content, username: FixClyde(name), avatarUrl: avatarUrl); | ||||
|                 var attachmentResponse = await _client.GetAsync(attachment.Url); | ||||
|                 var attachmentStream = await attachmentResponse.Content.ReadAsStreamAsync(); | ||||
|                 mfd.Add(new StreamContent(attachmentStream), "file", attachment.Filename); | ||||
|             } | ||||
|             catch (HttpException e) | ||||
|  | ||||
|             HttpResponseMessage response; | ||||
|             using (_metrics.Measure.Timer.Time(BotMetrics.WebhookResponseTime)) | ||||
|                 response = await _client.PostAsync($"{DiscordConfig.APIUrl}webhooks/{webhook.Id}/{webhook.Token}?wait=true", mfd); | ||||
|  | ||||
|             // TODO: are there cases where an error won't also return a parseable JSON object? | ||||
|             var responseJson = JsonConvert.DeserializeObject<JObject>(await response.Content.ReadAsStringAsync()); | ||||
|             if (responseJson.ContainsKey("code")) | ||||
|             { | ||||
|                 // If we hit an error, just retry (if we haven't already) | ||||
|                 if (e.DiscordCode == 10015 && !hasRetried) // Error 10015 = "Unknown Webhook" | ||||
|                 if (responseJson["code"].Value<int>() == 10015 && !hasRetried) | ||||
|                 { | ||||
|                     _logger.Warning(e, "Error invoking webhook {Webhook} in channel {Channel}", webhook.Id, webhook.ChannelId); | ||||
|                     // Error 10015 = "Unknown Webhook" - this likely means the webhook was deleted | ||||
|                     // but is still in our cache. Invalidate, refresh, try again | ||||
|                     _logger.Warning("Error invoking webhook {Webhook} in channel {Channel}", webhook.Id, webhook.ChannelId); | ||||
|                     return await ExecuteWebhookInner(await _webhookCache.InvalidateAndRefreshWebhook(webhook), name, avatarUrl, content, attachment, hasRetried: true); | ||||
|                 } | ||||
|  | ||||
|                 throw; | ||||
|                  | ||||
|                 // TODO: look into what this actually throws, and if this is the correct handling | ||||
|                 response.EnsureSuccessStatusCode(); | ||||
|             } | ||||
|         } | ||||
|          | ||||
|         private async Task<DiscordWebhookClient> GetClientFor(IWebhook webhook) | ||||
|         { | ||||
|             _logger.Verbose("Looking for client for webhook {Webhook} in cache", webhook.Id); | ||||
|             return await _cache.GetOrCreateAsync($"_webhook_client_{webhook.Id}", | ||||
|                 (entry) => MakeCachedClientFor(entry, webhook)); | ||||
|         } | ||||
|  | ||||
|         private Task<DiscordWebhookClient> MakeCachedClientFor(ICacheEntry entry, IWebhook webhook) { | ||||
|             _logger.Information("Client for {Webhook} not found in cache, creating", webhook.Id); | ||||
|              | ||||
|             // Define expiration for the client cache | ||||
|             // 10 minutes *without a query* and it gets yoten | ||||
|             entry.SlidingExpiration = TimeSpan.FromMinutes(10); | ||||
|  | ||||
|             // IMemoryCache won't automatically dispose of its values when the cache gets evicted | ||||
|             // so we register a hook to do so here. | ||||
|             entry.RegisterPostEvictionCallback((key, value, reason, state) => (value as IDisposable)?.Dispose()); | ||||
|              | ||||
|             // DiscordWebhookClient has a sync network call in its constructor (!!!) | ||||
|             // and we want to punt that onto a task queue, so we do that. | ||||
|             return Task.Run(async () => | ||||
|             { | ||||
|                 try | ||||
|                 { | ||||
|                     return new DiscordWebhookClient(webhook); | ||||
|                 } | ||||
|                 catch (InvalidOperationException) | ||||
|                 { | ||||
|                     // TODO: does this leak stuff inside the DiscordWebhookClient created above? | ||||
|                      | ||||
|                     // Webhook itself was found in cache, but has been deleted on the channel | ||||
|                     // We request a new webhook instead | ||||
|                     return new DiscordWebhookClient(await _webhookCache.InvalidateAndRefreshWebhook(webhook)); | ||||
|                 } | ||||
|  | ||||
|             }); | ||||
|             // At this point we're sure we have a 2xx status code, so just assume success | ||||
|             // TODO: can we do this without a round-trip to a string? | ||||
|             return responseJson["id"].Value<ulong>(); | ||||
|         } | ||||
|          | ||||
|  | ||||
|         private string FixClyde(string name) | ||||
|         { | ||||
|             // Check if the name contains "Clyde" - if not, do nothing | ||||
|   | ||||
		Reference in New Issue
	
	Block a user