From 50a24f03a7f52ccc7921ee09351d164f24f048eb Mon Sep 17 00:00:00 2001 From: spiral Date: Fri, 14 Jan 2022 18:39:03 -0500 Subject: [PATCH] refactor: only pass shard ID to event handlers instead of full shard object --- Myriad/Cache/DiscordCacheExtensions.cs | 10 ++-- PluralKit.Bot/Bot.cs | 55 ++++++++++--------- .../CommandSystem/Context/Context.cs | 6 +- PluralKit.Bot/Handlers/IEventHandler.cs | 2 +- PluralKit.Bot/Handlers/InteractionCreated.cs | 2 +- PluralKit.Bot/Handlers/MessageCreated.cs | 20 ++++--- PluralKit.Bot/Handlers/MessageDeleted.cs | 4 +- PluralKit.Bot/Handlers/MessageEdited.cs | 4 +- PluralKit.Bot/Handlers/ReactionAdded.cs | 2 +- PluralKit.Bot/Proxy/ProxyService.cs | 11 ++-- PluralKit.Bot/Services/ShardInfoService.cs | 2 +- PluralKit.Bot/Utils/SentryUtils.cs | 22 ++++---- .../Utils/SerilogGatewayEnricherFactory.cs | 4 +- 13 files changed, 74 insertions(+), 70 deletions(-) diff --git a/Myriad/Cache/DiscordCacheExtensions.cs b/Myriad/Cache/DiscordCacheExtensions.cs index 762340de..7a47f9f0 100644 --- a/Myriad/Cache/DiscordCacheExtensions.cs +++ b/Myriad/Cache/DiscordCacheExtensions.cs @@ -53,15 +53,15 @@ public static class DiscordCacheExtensions return default; } - public static ValueTask TryUpdateSelfMember(this IDiscordCache cache, Shard shard, IGatewayEvent evt) + public static ValueTask TryUpdateSelfMember(this IDiscordCache cache, ulong userId, IGatewayEvent evt) { if (evt is GuildCreateEvent gc) - return cache.SaveSelfMember(gc.Id, gc.Members.FirstOrDefault(m => m.User.Id == shard.User?.Id)!); - if (evt is MessageCreateEvent mc && mc.Member != null && mc.Author.Id == shard.User?.Id) + return cache.SaveSelfMember(gc.Id, gc.Members.FirstOrDefault(m => m.User.Id == userId)!); + if (evt is MessageCreateEvent mc && mc.Member != null && mc.Author.Id == userId) return cache.SaveSelfMember(mc.GuildId!.Value, mc.Member); - if (evt is GuildMemberAddEvent gma && gma.User.Id == shard.User?.Id) + if (evt is GuildMemberAddEvent gma && gma.User.Id == userId) return cache.SaveSelfMember(gma.GuildId, gma); - if (evt is GuildMemberUpdateEvent gmu && gmu.User.Id == shard.User?.Id) + if (evt is GuildMemberUpdateEvent gmu && gmu.User.Id == userId) return cache.SaveSelfMember(gmu.GuildId, gmu); return default; diff --git a/PluralKit.Bot/Bot.cs b/PluralKit.Bot/Bot.cs index cd86a0e8..89b2abad 100644 --- a/PluralKit.Bot/Bot.cs +++ b/PluralKit.Bot/Bot.cs @@ -56,7 +56,7 @@ public class Bot public void Init() { - _cluster.EventReceived += OnEventReceived; + _cluster.EventReceived += (shard, evt) => OnEventReceived(shard.ShardId, evt); // Init the shard stuff _services.Resolve().Init(); @@ -72,40 +72,43 @@ public class Bot }, null, timeTillNextWholeMinute, TimeSpan.FromMinutes(1)); } - private async Task OnEventReceived(Shard shard, IGatewayEvent evt) + private async Task OnEventReceived(int shardId, IGatewayEvent evt) { - await _cache.TryUpdateSelfMember(shard, evt); + // we HandleGatewayEvent **before** getting the own user, because the own user is set in HandleGatewayEvent for ReadyEvent await _cache.HandleGatewayEvent(evt); + var userId = await _cache.GetOwnUser(); + await _cache.TryUpdateSelfMember(userId, evt); + // HandleEvent takes a type parameter, automatically inferred by the event type // It will then look up an IEventHandler in the DI container and call that object's handler method // For registering new ones, see Modules.cs if (evt is MessageCreateEvent mc) - await HandleEvent(shard, mc); + await HandleEvent(shardId, mc); if (evt is MessageUpdateEvent mu) - await HandleEvent(shard, mu); + await HandleEvent(shardId, mu); if (evt is MessageDeleteEvent md) - await HandleEvent(shard, md); + await HandleEvent(shardId, md); if (evt is MessageDeleteBulkEvent mdb) - await HandleEvent(shard, mdb); + await HandleEvent(shardId, mdb); if (evt is MessageReactionAddEvent mra) - await HandleEvent(shard, mra); + await HandleEvent(shardId, mra); if (evt is InteractionCreateEvent ic) - await HandleEvent(shard, ic); + await HandleEvent(shardId, ic); // Update shard status for shards immediately on connect if (evt is ReadyEvent re) - await HandleReady(shard, re); + await HandleReady(shardId, re); if (evt is ResumedEvent) - await HandleResumed(shard); + await HandleResumed(shardId); } - private Task HandleResumed(Shard shard) => UpdateBotStatus(shard); + private Task HandleResumed(int shardId) => UpdateBotStatus(shardId); - private Task HandleReady(Shard shard, ReadyEvent _) + private Task HandleReady(int shardId, ReadyEvent _) { _hasReceivedReady = true; - return UpdateBotStatus(shard); + return UpdateBotStatus(shardId); } public async Task Shutdown() @@ -128,7 +131,7 @@ public class Bot }))); } - private Task HandleEvent(Shard shard, T evt) where T : IGatewayEvent + private Task HandleEvent(int shardId, T evt) where T : IGatewayEvent { // We don't want to stall the event pipeline, so we'll "fork" inside here var _ = HandleEventInner(); @@ -157,13 +160,12 @@ public class Bot var queue = serviceScope.ResolveOptional>(); using var _ = LogContext.PushProperty("EventId", Guid.NewGuid()); - using var __ = LogContext.Push(await serviceScope.Resolve() - .GetEnricher(shard, evt)); + using var __ = LogContext.Push(await serviceScope.Resolve().GetEnricher(shardId, evt)); _logger.Verbose("Received gateway event: {@Event}", evt); // Also, find a Sentry enricher for the event type (if one is present), and ask it to put some event data in the Sentry scope var sentryEnricher = serviceScope.ResolveOptional>(); - sentryEnricher?.Enrich(serviceScope.Resolve(), shard, evt); + sentryEnricher?.Enrich(serviceScope.Resolve(), shardId, evt); using var timer = _metrics.Measure.Timer.Time(BotMetrics.EventsHandled, new MetricTags("event", typeof(T).Name.Replace("Event", ""))); @@ -172,26 +174,28 @@ public class Bot // the TryHandle call returns true if it's handled the event // Usually it won't, so just pass it on to the main handler if (queue == null || !await queue.TryHandle(evt)) - await handler.Handle(shard, evt); + await handler.Handle(shardId, evt); } catch (Exception exc) { - await HandleError(shard, handler, evt, serviceScope, exc); + await HandleError(handler, evt, serviceScope, exc); } } } - private async Task HandleError(Shard shard, IEventHandler handler, T evt, ILifetimeScope serviceScope, + private async Task HandleError(IEventHandler handler, T evt, ILifetimeScope serviceScope, Exception exc) where T : IGatewayEvent { _metrics.Measure.Meter.Mark(BotMetrics.BotErrors, exc.GetType().FullName); + var ourUserId = await _cache.GetOwnUser(); + // Make this beforehand so we can access the event ID for logging var sentryEvent = new SentryEvent(exc); // If the event is us responding to our own error messages, don't bother logging - if (evt is MessageCreateEvent mc && mc.Author.Id == shard.User?.Id) + if (evt is MessageCreateEvent mc && mc.Author.Id == ourUserId) return; var shouldReport = exc.IsOurProblem(); @@ -220,7 +224,6 @@ public class Bot return; // Once we've sent it to Sentry, report it to the user (if we have permission to) - var ourUserId = await _cache.GetOwnUser(); var reportChannel = handler.ErrorChannelFor(evt, ourUserId); if (reportChannel == null) return; @@ -243,7 +246,7 @@ public class Bot _logger.Debug("Submitted metrics to backend"); } - private async Task UpdateBotStatus(Shard specificShard = null) + private async Task UpdateBotStatus(int? specificShardId = null) { // If we're not on any shards, don't bother (this happens if the periodic timer fires before the first Ready) if (!_hasReceivedReady) return; @@ -266,8 +269,8 @@ public class Bot } }); - if (specificShard != null) - await UpdateStatus(specificShard); + if (specificShardId != null) + await UpdateStatus(_cluster.Shards[specificShardId.Value]); else // Run shard updates concurrently await Task.WhenAll(_cluster.Shards.Values.Select(UpdateStatus)); } diff --git a/PluralKit.Bot/CommandSystem/Context/Context.cs b/PluralKit.Bot/CommandSystem/Context/Context.cs index eca831a8..acfbe4e7 100644 --- a/PluralKit.Bot/CommandSystem/Context/Context.cs +++ b/PluralKit.Bot/CommandSystem/Context/Context.cs @@ -28,11 +28,11 @@ public class Context private Command? _currentCommand; - public Context(ILifetimeScope provider, Shard shard, Guild? guild, Channel channel, MessageCreateEvent message, int commandParseOffset, + public Context(ILifetimeScope provider, int shardId, Guild? guild, Channel channel, MessageCreateEvent message, int commandParseOffset, PKSystem senderSystem, SystemConfig config, MessageContext messageContext) { Message = (Message)message; - Shard = shard; + ShardId = shardId; Guild = guild; Channel = channel; System = senderSystem; @@ -58,7 +58,7 @@ public class Context public readonly Message Message; public readonly Guild Guild; - public readonly Shard Shard; + public readonly int ShardId; public readonly Cluster Cluster; public readonly MessageContext MessageContext; diff --git a/PluralKit.Bot/Handlers/IEventHandler.cs b/PluralKit.Bot/Handlers/IEventHandler.cs index 64cc3cc7..fd18849e 100644 --- a/PluralKit.Bot/Handlers/IEventHandler.cs +++ b/PluralKit.Bot/Handlers/IEventHandler.cs @@ -4,7 +4,7 @@ namespace PluralKit.Bot; public interface IEventHandler where T : IGatewayEvent { - Task Handle(Shard shard, T evt); + Task Handle(int shardId, T evt); ulong? ErrorChannelFor(T evt, ulong userId) => null; } \ No newline at end of file diff --git a/PluralKit.Bot/Handlers/InteractionCreated.cs b/PluralKit.Bot/Handlers/InteractionCreated.cs index 42eac9f2..25a9c0f8 100644 --- a/PluralKit.Bot/Handlers/InteractionCreated.cs +++ b/PluralKit.Bot/Handlers/InteractionCreated.cs @@ -16,7 +16,7 @@ public class InteractionCreated: IEventHandler _services = services; } - public async Task Handle(Shard shard, InteractionCreateEvent evt) + public async Task Handle(int shardId, InteractionCreateEvent evt) { if (evt.Type == Interaction.InteractionType.MessageComponent) { diff --git a/PluralKit.Bot/Handlers/MessageCreated.cs b/PluralKit.Bot/Handlers/MessageCreated.cs index 7a0bf0ff..437ac37b 100644 --- a/PluralKit.Bot/Handlers/MessageCreated.cs +++ b/PluralKit.Bot/Handlers/MessageCreated.cs @@ -63,9 +63,9 @@ public class MessageCreated: IEventHandler // We consider a message duplicate if it has the same ID as the previous message that hit the gateway _lastMessageCache.GetLastMessage(msg.ChannelId)?.Current.Id == msg.Id; - public async Task Handle(Shard shard, MessageCreateEvent evt) + public async Task Handle(int shardId, MessageCreateEvent evt) { - if (evt.Author.Id == shard.User?.Id) return; + if (evt.Author.Id == await _cache.GetOwnUser()) return; if (evt.Type != Message.MessageType.Default && evt.Type != Message.MessageType.Reply) return; if (IsDuplicateMessage(evt)) return; @@ -90,9 +90,9 @@ public class MessageCreated: IEventHandler if (evt.Author.Bot || evt.WebhookId != null || evt.Author.System == true) return; - if (await TryHandleCommand(shard, evt, guild, channel, ctx)) + if (await TryHandleCommand(shardId, evt, guild, channel, ctx)) return; - await TryHandleProxy(shard, evt, guild, channel, ctx); + await TryHandleProxy(evt, guild, channel, ctx); } private async ValueTask TryHandleLogClean(MessageCreateEvent evt, MessageContext ctx) @@ -105,14 +105,16 @@ public class MessageCreated: IEventHandler return true; } - private async ValueTask TryHandleCommand(Shard shard, MessageCreateEvent evt, Guild? guild, + private async ValueTask TryHandleCommand(int shardId, MessageCreateEvent evt, Guild? guild, Channel channel, MessageContext ctx) { var content = evt.Content; if (content == null) return false; + var ourUserId = await _cache.GetOwnUser(); + // Check for command prefix - if (!HasCommandPrefix(content, shard.User?.Id ?? default, out var cmdStart) || cmdStart == content.Length) + if (!HasCommandPrefix(content, ourUserId, out var cmdStart) || cmdStart == content.Length) return false; // Trim leading whitespace from command without actually modifying the string @@ -125,7 +127,7 @@ public class MessageCreated: IEventHandler { var system = ctx.SystemId != null ? await _repo.GetSystem(ctx.SystemId.Value) : null; var config = ctx.SystemId != null ? await _repo.GetSystemConfig(ctx.SystemId.Value) : null; - await _tree.ExecuteCommand(new Context(_services, shard, guild, channel, evt, cmdStart, system, config, ctx)); + await _tree.ExecuteCommand(new Context(_services, shardId, guild, channel, evt, cmdStart, system, config, ctx)); } catch (PKError) { @@ -156,14 +158,14 @@ public class MessageCreated: IEventHandler return false; } - private async ValueTask TryHandleProxy(Shard shard, MessageCreateEvent evt, Guild guild, Channel channel, + private async ValueTask TryHandleProxy(MessageCreateEvent evt, Guild guild, Channel channel, MessageContext ctx) { var botPermissions = await _cache.PermissionsIn(channel.Id); try { - return await _proxy.HandleIncomingMessage(shard, evt, ctx, guild, channel, ctx.AllowAutoproxy, + return await _proxy.HandleIncomingMessage(evt, ctx, guild, channel, ctx.AllowAutoproxy, botPermissions); } diff --git a/PluralKit.Bot/Handlers/MessageDeleted.cs b/PluralKit.Bot/Handlers/MessageDeleted.cs index 5e883a51..408c363d 100644 --- a/PluralKit.Bot/Handlers/MessageDeleted.cs +++ b/PluralKit.Bot/Handlers/MessageDeleted.cs @@ -24,7 +24,7 @@ public class MessageDeleted: IEventHandler, IEventHandler(); } - public Task Handle(Shard shard, MessageDeleteEvent evt) + public Task Handle(int shardId, MessageDeleteEvent evt) { // Delete deleted webhook messages from the data store // Most of the data in the given message is wrong/missing, so always delete just to be sure. @@ -43,7 +43,7 @@ public class MessageDeleted: IEventHandler, IEventHandler _logger = logger.ForContext(); } - public async Task Handle(Shard shard, MessageUpdateEvent evt) + public async Task Handle(int shardId, MessageUpdateEvent evt) { if (evt.Author.Value?.Id == await _cache.GetOwnUser()) return; @@ -69,7 +69,7 @@ public class MessageEdited: IEventHandler try { - await _proxy.HandleIncomingMessage(shard, equivalentEvt, ctx, allowAutoproxy: false, guild: guild, + await _proxy.HandleIncomingMessage(equivalentEvt, ctx, allowAutoproxy: false, guild: guild, channel: channel, botPermissions: botPermissions); } // Catch any failed proxy checks so they get ignored in the global error handler diff --git a/PluralKit.Bot/Handlers/ReactionAdded.cs b/PluralKit.Bot/Handlers/ReactionAdded.cs index b67ac832..1b8bd461 100644 --- a/PluralKit.Bot/Handlers/ReactionAdded.cs +++ b/PluralKit.Bot/Handlers/ReactionAdded.cs @@ -42,7 +42,7 @@ public class ReactionAdded: IEventHandler _logger = logger.ForContext(); } - public async Task Handle(Shard shard, MessageReactionAddEvent evt) + public async Task Handle(int shardId, MessageReactionAddEvent evt) { await TryHandleProxyMessageReactions(evt); } diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index 1a66bc5d..185af0c6 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -50,7 +50,7 @@ public class ProxyService _logger = logger.ForContext(); } - public async Task HandleIncomingMessage(Shard shard, MessageCreateEvent message, MessageContext ctx, + public async Task HandleIncomingMessage(MessageCreateEvent message, MessageContext ctx, Guild guild, Channel channel, bool allowAutoproxy, PermissionSet botPermissions) { if (!ShouldProxy(channel, message, ctx)) @@ -80,7 +80,7 @@ public class ProxyService var allowEmbeds = senderPermissions.HasFlag(PermissionSet.EmbedLinks); // Everything's in order, we can execute the proxy! - await ExecuteProxy(shard, message, ctx, match, allowEveryone, allowEmbeds); + await ExecuteProxy(message, ctx, match, allowEveryone, allowEmbeds); return true; } @@ -119,7 +119,7 @@ public class ProxyService return true; } - private async Task ExecuteProxy(Shard shard, Message trigger, MessageContext ctx, + private async Task ExecuteProxy(Message trigger, MessageContext ctx, ProxyMatch match, bool allowEveryone, bool allowEmbeds) { // Create reply embed @@ -160,7 +160,7 @@ public class ProxyService Embeds = embeds.ToArray(), AllowEveryone = allowEveryone }); - await HandleProxyExecutedActions(shard, ctx, trigger, proxyMessage, match); + await HandleProxyExecutedActions(ctx, trigger, proxyMessage, match); } private async Task<(string?, string?)> FetchReferencedMessageAuthorInfo(Message trigger, Message referenced) @@ -279,8 +279,7 @@ public class ProxyService private string FixSameNameInner(string name) => $"{name}\u17b5"; - private async Task HandleProxyExecutedActions(Shard shard, MessageContext ctx, - Message triggerMessage, Message proxyMessage, ProxyMatch match) + private async Task HandleProxyExecutedActions(MessageContext ctx, Message triggerMessage, Message proxyMessage, ProxyMatch match) { var sentMessage = new PKMessage { diff --git a/PluralKit.Bot/Services/ShardInfoService.cs b/PluralKit.Bot/Services/ShardInfoService.cs index 59f3babd..3f1ec7d1 100644 --- a/PluralKit.Bot/Services/ShardInfoService.cs +++ b/PluralKit.Bot/Services/ShardInfoService.cs @@ -138,7 +138,7 @@ public class ShardInfoService } } - public ShardInfo GetShardInfo(Shard shard) => _shardInfo[shard.ShardId]; + public ShardInfo GetShardInfo(int shardId) => _shardInfo[shardId]; public class ShardInfo { diff --git a/PluralKit.Bot/Utils/SentryUtils.cs b/PluralKit.Bot/Utils/SentryUtils.cs index 9f870abe..97471821 100644 --- a/PluralKit.Bot/Utils/SentryUtils.cs +++ b/PluralKit.Bot/Utils/SentryUtils.cs @@ -6,7 +6,7 @@ namespace PluralKit.Bot; public interface ISentryEnricher where T : IGatewayEvent { - void Enrich(Scope scope, Shard shard, T evt); + void Enrich(Scope scope, int shardId, T evt); } public class SentryEnricher: @@ -26,7 +26,7 @@ public class SentryEnricher: // TODO: should this class take the Scope by dependency injection instead? // Would allow us to create a centralized "chain of handlers" where this class could just be registered as an entry in - public void Enrich(Scope scope, Shard shard, MessageCreateEvent evt) + public void Enrich(Scope scope, int shardId, MessageCreateEvent evt) { scope.AddBreadcrumb(evt.Content, "event.message", data: new Dictionary @@ -36,7 +36,7 @@ public class SentryEnricher: {"guild", evt.GuildId.ToString()}, {"message", evt.Id.ToString()} }); - scope.SetTag("shard", shard.ShardId.ToString()); + scope.SetTag("shard", shardId.ToString()); // Also report information about the bot's permissions in the channel // We get a lot of permission errors so this'll be useful for determining problems @@ -46,7 +46,7 @@ public class SentryEnricher: // scope.AddBreadcrumb(perms.ToPermissionString(), "permissions"); } - public void Enrich(Scope scope, Shard shard, MessageDeleteBulkEvent evt) + public void Enrich(Scope scope, int shardId, MessageDeleteBulkEvent evt) { scope.AddBreadcrumb("", "event.messageDelete", data: new Dictionary @@ -55,10 +55,10 @@ public class SentryEnricher: {"guild", evt.GuildId.ToString()}, {"messages", string.Join(",", evt.Ids)} }); - scope.SetTag("shard", shard.ShardId.ToString()); + scope.SetTag("shard", shardId.ToString()); } - public void Enrich(Scope scope, Shard shard, MessageUpdateEvent evt) + public void Enrich(Scope scope, int shardId, MessageUpdateEvent evt) { scope.AddBreadcrumb(evt.Content.Value ?? "", "event.messageEdit", data: new Dictionary @@ -67,10 +67,10 @@ public class SentryEnricher: {"guild", evt.GuildId.Value.ToString()}, {"message", evt.Id.ToString()} }); - scope.SetTag("shard", shard.ShardId.ToString()); + scope.SetTag("shard", shardId.ToString()); } - public void Enrich(Scope scope, Shard shard, MessageDeleteEvent evt) + public void Enrich(Scope scope, int shardId, MessageDeleteEvent evt) { scope.AddBreadcrumb("", "event.messageDelete", data: new Dictionary @@ -79,10 +79,10 @@ public class SentryEnricher: {"guild", evt.GuildId.ToString()}, {"message", evt.Id.ToString()} }); - scope.SetTag("shard", shard.ShardId.ToString()); + scope.SetTag("shard", shardId.ToString()); } - public void Enrich(Scope scope, Shard shard, MessageReactionAddEvent evt) + public void Enrich(Scope scope, int shardId, MessageReactionAddEvent evt) { scope.AddBreadcrumb("", "event.reaction", data: new Dictionary @@ -93,6 +93,6 @@ public class SentryEnricher: {"message", evt.MessageId.ToString()}, {"reaction", evt.Emoji.Name} }); - scope.SetTag("shard", shard.ShardId.ToString()); + scope.SetTag("shard", shardId.ToString()); } } \ No newline at end of file diff --git a/PluralKit.Bot/Utils/SerilogGatewayEnricherFactory.cs b/PluralKit.Bot/Utils/SerilogGatewayEnricherFactory.cs index 942d0881..d7a34fd7 100644 --- a/PluralKit.Bot/Utils/SerilogGatewayEnricherFactory.cs +++ b/PluralKit.Bot/Utils/SerilogGatewayEnricherFactory.cs @@ -20,9 +20,9 @@ public class SerilogGatewayEnricherFactory _botConfig = botConfig; } - public async Task GetEnricher(Shard shard, IGatewayEvent evt) + public async Task GetEnricher(int shardId, IGatewayEvent evt) { - var props = new List { new("ShardId", new ScalarValue(shard.ShardId)) }; + var props = new List { new("ShardId", new ScalarValue(shardId)) }; if (_botConfig.Cluster != null) props.Add(new LogEventProperty("ClusterId", new ScalarValue(_botConfig.Cluster.NodeName)));