From cf7007f2691080029995332418c0f764f06cc88e Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Thu, 26 Oct 2023 20:14:27 +0500 Subject: [PATCH] Handle guild data load errors better (#172) Previously, any errors in guild data load will cause the bot to be unusable in that guild. It didn't help that the end users had no information that something was wrong! Now, any errors will be logged better (with the full path to the file that couldn't be loaded), and the users will receive a message saying that functionality is degraded The old way to save objects was to serialize them directly into streams opened by `File#Create`. This can cause problems if the serialization isn't completed, because `File#Create` overwrites the file with an empty one on the spot. Now, objects are first deserialized into a temporary file, then the original is replaced by the temporary, then the temporary is deleted. Errors during guild data load would sometimes cause the bot to replace the corrupted file with a default one whenever a save is triggered. Now, guilds with load errors won't have their data saved to aid in debugging --------- Signed-off-by: Octol1ttle Signed-off-by: mctaylors --- locale/Messages.resx | 6 ++ locale/Messages.ru.resx | 6 ++ locale/Messages.tt-ru.resx | 6 ++ src/Data/GuildData.cs | 5 +- src/Messages.Designer.cs | 18 +++++- src/Responders/GuildLoadedResponder.cs | 53 ++++++++++++++--- src/Responders/GuildUnloadedResponder.cs | 2 +- src/Services/GuildDataService.cs | 74 ++++++++++++++++++------ 8 files changed, 142 insertions(+), 28 deletions(-) diff --git a/locale/Messages.resx b/locale/Messages.resx index 5e24811..a9367f7 100644 --- a/locale/Messages.resx +++ b/locale/Messages.resx @@ -570,6 +570,12 @@ Cleared {0} messages from {1} + + An error occurred during guild data load. + + + This will lead to unexpected behavior. Data will no longer be saved + An error occurred during command execution, try again later. diff --git a/locale/Messages.ru.resx b/locale/Messages.ru.resx index 1cc1f9e..d0cbd79 100644 --- a/locale/Messages.ru.resx +++ b/locale/Messages.ru.resx @@ -570,6 +570,12 @@ Очищено {0} сообщений от {1} + + Произошла ошибка при загрузке данных сервера. + + + Это может привести к неожиданному поведению. Данные больше не будут сохраняться. + Произошла ошибка при выполнении команды, повтори попытку позже. diff --git a/locale/Messages.tt-ru.resx b/locale/Messages.tt-ru.resx index 48ad8e7..3bed232 100644 --- a/locale/Messages.tt-ru.resx +++ b/locale/Messages.tt-ru.resx @@ -570,6 +570,12 @@ вырезано {0} забавных сообщений от {1} + + произошёл тотальный разнос в гилддате. + + + возможно всё съедет с крыши, но знай, что я больше ничё не сохраню. + произошёл тотальный разнос в команде, удачи. diff --git a/src/Data/GuildData.cs b/src/Data/GuildData.cs index a675037..5a903d6 100644 --- a/src/Data/GuildData.cs +++ b/src/Data/GuildData.cs @@ -17,10 +17,12 @@ public sealed class GuildData public readonly JsonNode Settings; public readonly string SettingsPath; + public readonly bool DataLoadFailed; + public GuildData( JsonNode settings, string settingsPath, Dictionary scheduledEvents, string scheduledEventsPath, - Dictionary memberData, string memberDataPath) + Dictionary memberData, string memberDataPath, bool dataLoadFailed) { Settings = settings; SettingsPath = settingsPath; @@ -28,6 +30,7 @@ public sealed class GuildData ScheduledEventsPath = scheduledEventsPath; MemberData = memberData; MemberDataPath = memberDataPath; + DataLoadFailed = dataLoadFailed; } public MemberData GetOrCreateMemberData(Snowflake memberId) diff --git a/src/Messages.Designer.cs b/src/Messages.Designer.cs index 5f38061..e2184d7 100644 --- a/src/Messages.Designer.cs +++ b/src/Messages.Designer.cs @@ -997,6 +997,22 @@ namespace Octobot { } } + internal static string DataLoadFailedTitle + { + get + { + return ResourceManager.GetString("DataLoadFailedTitle", resourceCulture); + } + } + + internal static string DataLoadFailedDescription + { + get + { + return ResourceManager.GetString("DataLoadFailedDescription", resourceCulture); + } + } + internal static string CommandExecutionFailed { get @@ -1004,7 +1020,7 @@ namespace Octobot { return ResourceManager.GetString("CommandExecutionFailed", resourceCulture); } } - + internal static string ContactDevelopers { get diff --git a/src/Responders/GuildLoadedResponder.cs b/src/Responders/GuildLoadedResponder.cs index 78dcc43..3e08060 100644 --- a/src/Responders/GuildLoadedResponder.cs +++ b/src/Responders/GuildLoadedResponder.cs @@ -8,6 +8,7 @@ using Remora.Discord.API.Abstractions.Rest; using Remora.Discord.API.Gateway.Events; using Remora.Discord.Extensions.Embeds; using Remora.Discord.Gateway.Responders; +using Remora.Rest.Core; using Remora.Results; namespace Octobot.Responders; @@ -42,7 +43,6 @@ public class GuildLoadedResponder : IResponder } var guild = gatewayEvent.Guild.AsT0; - _logger.LogInformation("Joined guild {ID} (\"{Name}\")", guild.ID, guild.Name); var data = await _guildData.GetData(guild.ID, ct); var cfg = data.Settings; @@ -51,6 +51,32 @@ public class GuildLoadedResponder : IResponder data.GetOrCreateMemberData(member.User.Value.ID); } + var botResult = await _userApi.GetCurrentUserAsync(ct); + if (!botResult.IsDefined(out var bot)) + { + return Result.FromError(botResult); + } + + if (data.DataLoadFailed) + { + var errorEmbed = new EmbedBuilder() + .WithSmallTitle(Messages.DataLoadFailedTitle, bot) + .WithDescription(Messages.DataLoadFailedDescription) + .WithFooter(Messages.ContactDevelopers) + .WithColour(ColorsList.Red) + .Build(); + + if (!errorEmbed.IsDefined(out var errorBuilt)) + { + return Result.FromError(errorEmbed); + } + + return (Result)await _channelApi.CreateMessageAsync( + GetEmergencyFeedbackChannel(guild, data), embeds: new[] { errorBuilt }, ct: ct); + } + + _logger.LogInformation("Loaded guild {ID} (\"{Name}\")", guild.ID, guild.Name); + if (!GuildSettings.ReceiveStartupMessages.Get(cfg)) { return Result.FromSuccess(); @@ -61,12 +87,6 @@ public class GuildLoadedResponder : IResponder return Result.FromSuccess(); } - var botResult = await _userApi.GetCurrentUserAsync(ct); - if (!botResult.IsDefined(out var bot)) - { - return Result.FromError(botResult); - } - Messages.Culture = GuildSettings.Language.Get(cfg); var i = Random.Shared.Next(1, 4); @@ -84,4 +104,23 @@ public class GuildLoadedResponder : IResponder return (Result)await _channelApi.CreateMessageAsync( GuildSettings.PrivateFeedbackChannel.Get(cfg), embeds: new[] { built }, ct: ct); } + + private static Snowflake GetEmergencyFeedbackChannel(IGuildCreate.IAvailableGuild guild, GuildData data) + { + var privateFeedback = GuildSettings.PrivateFeedbackChannel.Get(data.Settings); + if (!privateFeedback.Empty()) + { + return privateFeedback; + } + + var publicFeedback = GuildSettings.PublicFeedbackChannel.Get(data.Settings); + if (!publicFeedback.Empty()) + { + return publicFeedback; + } + + return guild.SystemChannelID.AsOptional().IsDefined(out var systemChannel) + ? systemChannel + : guild.Channels[0].ID; + } } diff --git a/src/Responders/GuildUnloadedResponder.cs b/src/Responders/GuildUnloadedResponder.cs index 0cffe25..47bde75 100644 --- a/src/Responders/GuildUnloadedResponder.cs +++ b/src/Responders/GuildUnloadedResponder.cs @@ -30,7 +30,7 @@ public class GuildUnloadedResponder : IResponder var isDataRemoved = _guildData.UnloadGuildData(guildId); if (isDataRemoved) { - _logger.LogInformation("Left guild {GuildId}", guildId); + _logger.LogInformation("Unloaded guild {GuildId}", guildId); } return Task.FromResult(Result.FromSuccess()); diff --git a/src/Services/GuildDataService.cs b/src/Services/GuildDataService.cs index 73d4a25..78203b5 100644 --- a/src/Services/GuildDataService.cs +++ b/src/Services/GuildDataService.cs @@ -43,25 +43,31 @@ public sealed class GuildDataService : IHostedService { var tasks = new List(); var datas = _datas.Values.ToArray(); - foreach (var data in datas) + foreach (var data in datas.Where(data => !data.DataLoadFailed)) { - await using var settingsStream = File.Create(data.SettingsPath); - tasks.Add(JsonSerializer.SerializeAsync(settingsStream, data.Settings, cancellationToken: ct)); - - await using var eventsStream = File.Create(data.ScheduledEventsPath); - tasks.Add(JsonSerializer.SerializeAsync(eventsStream, data.ScheduledEvents, cancellationToken: ct)); + tasks.Add(SerializeObjectSafelyAsync(data.Settings, data.SettingsPath, ct)); + tasks.Add(SerializeObjectSafelyAsync(data.ScheduledEvents, data.ScheduledEventsPath, ct)); var memberDatas = data.MemberData.Values.ToArray(); - foreach (var memberData in memberDatas) - { - await using var memberDataStream = File.Create($"{data.MemberDataPath}/{memberData.Id}.json"); - tasks.Add(JsonSerializer.SerializeAsync(memberDataStream, memberData, cancellationToken: ct)); - } + tasks.AddRange(memberDatas.Select(memberData => + SerializeObjectSafelyAsync(memberData, $"{data.MemberDataPath}/{memberData.Id}.json", ct))); } await Task.WhenAll(tasks); } + private static async Task SerializeObjectSafelyAsync(T obj, string path, CancellationToken ct) + { + var tempFilePath = path + ".tmp"; + await using (var tempFileStream = File.Create(tempFilePath)) + { + await JsonSerializer.SerializeAsync(tempFileStream, obj, cancellationToken: ct); + } + + File.Copy(tempFilePath, path, true); + File.Delete(tempFilePath); + } + public async Task GetData(Snowflake guildId, CancellationToken ct = default) { return _datas.TryGetValue(guildId, out var data) ? data : await InitializeData(guildId, ct); @@ -88,20 +94,50 @@ public sealed class GuildDataService : IHostedService await File.WriteAllTextAsync(scheduledEventsPath, "{}", ct); } + var dataLoadFailed = false; + await using var settingsStream = File.OpenRead(settingsPath); - var jsonSettings - = JsonNode.Parse(settingsStream); + JsonNode? jsonSettings = null; + try + { + jsonSettings = JsonNode.Parse(settingsStream); + } + catch (Exception e) + { + _logger.LogError(e, "Guild settings load failed: {Path}", settingsPath); + dataLoadFailed = true; + } await using var eventsStream = File.OpenRead(scheduledEventsPath); - var events - = await JsonSerializer.DeserializeAsync>( + Dictionary? events = null; + try + { + events = await JsonSerializer.DeserializeAsync>( eventsStream, cancellationToken: ct); + } + catch (Exception e) + { + _logger.LogError(e, "Guild scheduled events load failed: {Path}", scheduledEventsPath); + dataLoadFailed = true; + } var memberData = new Dictionary(); foreach (var dataFileInfo in Directory.CreateDirectory(memberDataPath).GetFiles()) { await using var dataStream = dataFileInfo.OpenRead(); - var data = await JsonSerializer.DeserializeAsync(dataStream, cancellationToken: ct); + MemberData? data; + try + { + data = await JsonSerializer.DeserializeAsync(dataStream, cancellationToken: ct); + } + catch (Exception e) + { + _logger.LogError(e, "Member data load failed: {MemberDataPath}/{FileName}", memberDataPath, + dataFileInfo.Name); + dataLoadFailed = true; + continue; + } + if (data is null) { continue; @@ -113,7 +149,8 @@ public sealed class GuildDataService : IHostedService var finalData = new GuildData( jsonSettings ?? new JsonObject(), settingsPath, events ?? new Dictionary(), scheduledEventsPath, - memberData, memberDataPath); + memberData, memberDataPath, + dataLoadFailed); _datas.TryAdd(guildId, finalData); @@ -129,7 +166,8 @@ public sealed class GuildDataService : IHostedService Directory.CreateDirectory($"{newPath}/.."); Directory.Move(oldPath, newPath); - _logger.LogInformation("Moved guild data to separate folder: \"{OldPath}\" -> \"{NewPath}\"", oldPath, newPath); + _logger.LogInformation("Moved guild data to separate folder: \"{OldPath}\" -> \"{NewPath}\"", oldPath, + newPath); } }