From d221441c105203d575d8619d536419d5889284fb Mon Sep 17 00:00:00 2001
From: sam <sam@starshines.gay>
Date: Sat, 12 Oct 2024 23:47:18 +0200
Subject: [PATCH] feat: tweak embed dequeueing logic

We no longer blindly dequeue 5 embeds, we check their length too.
The webhook executor will now send up to 10 embeds OR
embeds totaling less than 6000 characters, whichever is less.
Embeds longer than 6000 characters are discarded to prevent errors.
We also check for an empty request body in SendLogAsync and bail to prevent 400s.
---
 .../Extensions/DiscordExtensions.cs           | 16 ++++--
 .../Services/WebhookExecutorService.cs        | 56 +++++++++++++++++--
 2 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/Catalogger.Backend/Extensions/DiscordExtensions.cs b/Catalogger.Backend/Extensions/DiscordExtensions.cs
index 9487a88..b7be6f6 100644
--- a/Catalogger.Backend/Extensions/DiscordExtensions.cs
+++ b/Catalogger.Backend/Extensions/DiscordExtensions.cs
@@ -1,15 +1,11 @@
 using System.Drawing;
 using Catalogger.Backend.Cache.InMemoryCache;
 using Humanizer;
-using OneOf;
 using Remora.Discord.API.Abstractions.Gateway.Events;
 using Remora.Discord.API.Abstractions.Objects;
-using Remora.Discord.API.Abstractions.Rest;
-using Remora.Discord.API.Objects;
 using Remora.Discord.Commands.Contexts;
 using Remora.Discord.Commands.Extensions;
 using Remora.Discord.Commands.Services;
-using Remora.Discord.Extensions.Embeds;
 using Remora.Rest.Core;
 using Remora.Results;
 
@@ -165,5 +161,17 @@ public static class DiscordExtensions
             : $"*(unknown user {actionData.ModeratorId}) <@{actionData.ModeratorId}>*";
     }
 
+    public static int TextLength(this IEmbed embed)
+    {
+        var length = OptionalStringLength(embed.Description) + OptionalStringLength(embed.Title);
+        var fieldLength = (embed.Fields.OrDefault() ?? [])
+            .Select(f => f.Name.Length + f.Value.Length)
+            .Sum();
+
+        return length + fieldLength;
+    }
+
+    private static int OptionalStringLength(Optional<string> s) => s.OrDefault()?.Length ?? 0;
+
     public class DiscordRestException(string message) : Exception(message);
 }
diff --git a/Catalogger.Backend/Services/WebhookExecutorService.cs b/Catalogger.Backend/Services/WebhookExecutorService.cs
index 20d48a6..764c0bf 100644
--- a/Catalogger.Backend/Services/WebhookExecutorService.cs
+++ b/Catalogger.Backend/Services/WebhookExecutorService.cs
@@ -1,4 +1,5 @@
 using System.Collections.Concurrent;
+using System.Diagnostics.CodeAnalysis;
 using Catalogger.Backend.Cache;
 using Catalogger.Backend.Cache.InMemoryCache;
 using Catalogger.Backend.Extensions;
@@ -11,6 +12,11 @@ using Guild = Catalogger.Backend.Database.Models.Guild;
 
 namespace Catalogger.Backend.Services;
 
+[SuppressMessage(
+    "ReSharper",
+    "InconsistentlySynchronizedField",
+    Justification = "ILogger doesn't need to be synchronized"
+)]
 public class WebhookExecutorService(
     Config config,
     ILogger logger,
@@ -76,6 +82,14 @@ public class WebhookExecutorService(
             .Select<FileData, OneOf.OneOf<FileData, IPartialAttachment>>(f => f)
             .ToList();
 
+        if (embeds.Count == 0 && attachments.Count == 0)
+        {
+            _logger.Error(
+                "SendLogAsync was called with zero embeds and zero attachments, bailing to prevent a bad request error"
+            );
+            return;
+        }
+
         _logger.Debug(
             "Sending {EmbedCount} embeds/{FileCount} files to channel {ChannelId}",
             embeds.Count,
@@ -108,9 +122,7 @@ public class WebhookExecutorService(
         _timers[channelId] = new Timer(
             _ =>
             {
-                _logger.Debug("Sending 5 queued embeds");
-
-                var __ = SendLogAsync(channelId, TakeFromQueue(channelId).ToList(), []);
+                var __ = SendLogAsync(channelId, TakeFromQueue(channelId), []);
                 if (!queue.IsEmpty)
                 {
                     if (_timers.TryGetValue(channelId, out var timer))
@@ -124,8 +136,11 @@ public class WebhookExecutorService(
         );
     }
 
+    private const int MaxContentLength = 6000;
+
     /// <summary>
-    /// Takes 5 embeds from the queue for the given channel.
+    /// Takes as many embeds as possible from the queue for the given channel.
+    /// Up to ten embeds are returned, or less if their combined length is longer than 6000 characters.
     /// Note that this locks the queue to prevent duplicate embeds from being sent.
     /// </summary>
     private List<IEmbed> TakeFromQueue(ulong channelId)
@@ -134,14 +149,43 @@ public class WebhookExecutorService(
         var channelLock = _locks.GetOrAdd(channelId, channelId);
         lock (channelLock)
         {
+            var totalContentLength = 0;
             var embeds = new List<IEmbed>();
-            for (var i = 0; i < 5; i++)
+            while (embeds.Count < 10 && totalContentLength < MaxContentLength)
             {
-                if (!queue.TryDequeue(out var embed))
+                if (!queue.TryPeek(out var embed))
                     break;
+
+                var length = embed.TextLength();
+                if (length > MaxContentLength)
+                {
+                    _logger.Warning(
+                        "Queued embed for {ChannelId} exceeds maximum length, discarding it",
+                        channelId
+                    );
+                    queue.TryDequeue(out _);
+                    break;
+                }
+
+                if (totalContentLength + length > MaxContentLength)
+                    break;
+
+                totalContentLength += length;
+
+                queue.TryDequeue(out _);
                 embeds.Add(embed);
             }
 
+            if (embeds.Count == 0)
+                return embeds;
+
+            _logger.Debug(
+                "Took {EmbedCount} embeds from queue for {ChannelId}, total length is {TotalLength}",
+                embeds.Count,
+                channelId,
+                totalContentLength
+            );
+
             return embeds;
         }
     }