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.
This commit is contained in:
sam 2024-10-12 23:47:18 +02:00
parent f524afb05b
commit d221441c10
2 changed files with 62 additions and 10 deletions

View file

@ -1,15 +1,11 @@
using System.Drawing; using System.Drawing;
using Catalogger.Backend.Cache.InMemoryCache; using Catalogger.Backend.Cache.InMemoryCache;
using Humanizer; using Humanizer;
using OneOf;
using Remora.Discord.API.Abstractions.Gateway.Events; using Remora.Discord.API.Abstractions.Gateway.Events;
using Remora.Discord.API.Abstractions.Objects; 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.Contexts;
using Remora.Discord.Commands.Extensions; using Remora.Discord.Commands.Extensions;
using Remora.Discord.Commands.Services; using Remora.Discord.Commands.Services;
using Remora.Discord.Extensions.Embeds;
using Remora.Rest.Core; using Remora.Rest.Core;
using Remora.Results; using Remora.Results;
@ -165,5 +161,17 @@ public static class DiscordExtensions
: $"*(unknown user {actionData.ModeratorId}) <@{actionData.ModeratorId}>*"; : $"*(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); public class DiscordRestException(string message) : Exception(message);
} }

View file

@ -1,4 +1,5 @@
using System.Collections.Concurrent; using System.Collections.Concurrent;
using System.Diagnostics.CodeAnalysis;
using Catalogger.Backend.Cache; using Catalogger.Backend.Cache;
using Catalogger.Backend.Cache.InMemoryCache; using Catalogger.Backend.Cache.InMemoryCache;
using Catalogger.Backend.Extensions; using Catalogger.Backend.Extensions;
@ -11,6 +12,11 @@ using Guild = Catalogger.Backend.Database.Models.Guild;
namespace Catalogger.Backend.Services; namespace Catalogger.Backend.Services;
[SuppressMessage(
"ReSharper",
"InconsistentlySynchronizedField",
Justification = "ILogger doesn't need to be synchronized"
)]
public class WebhookExecutorService( public class WebhookExecutorService(
Config config, Config config,
ILogger logger, ILogger logger,
@ -76,6 +82,14 @@ public class WebhookExecutorService(
.Select<FileData, OneOf.OneOf<FileData, IPartialAttachment>>(f => f) .Select<FileData, OneOf.OneOf<FileData, IPartialAttachment>>(f => f)
.ToList(); .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( _logger.Debug(
"Sending {EmbedCount} embeds/{FileCount} files to channel {ChannelId}", "Sending {EmbedCount} embeds/{FileCount} files to channel {ChannelId}",
embeds.Count, embeds.Count,
@ -108,9 +122,7 @@ public class WebhookExecutorService(
_timers[channelId] = new Timer( _timers[channelId] = new Timer(
_ => _ =>
{ {
_logger.Debug("Sending 5 queued embeds"); var __ = SendLogAsync(channelId, TakeFromQueue(channelId), []);
var __ = SendLogAsync(channelId, TakeFromQueue(channelId).ToList(), []);
if (!queue.IsEmpty) if (!queue.IsEmpty)
{ {
if (_timers.TryGetValue(channelId, out var timer)) if (_timers.TryGetValue(channelId, out var timer))
@ -124,8 +136,11 @@ public class WebhookExecutorService(
); );
} }
private const int MaxContentLength = 6000;
/// <summary> /// <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. /// Note that this locks the queue to prevent duplicate embeds from being sent.
/// </summary> /// </summary>
private List<IEmbed> TakeFromQueue(ulong channelId) private List<IEmbed> TakeFromQueue(ulong channelId)
@ -134,14 +149,43 @@ public class WebhookExecutorService(
var channelLock = _locks.GetOrAdd(channelId, channelId); var channelLock = _locks.GetOrAdd(channelId, channelId);
lock (channelLock) lock (channelLock)
{ {
var totalContentLength = 0;
var embeds = new List<IEmbed>(); 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; 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); 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; return embeds;
} }
} }