Fix roles not updating

Accidentally rewrote a handful of things while I was at it. The changes are staying.

After a long search, I found that this issue only shows itself if the bot is in several servers, encountering a valid configuration while it has yet to look at more servers (that is, a situation my debug bot is not in). The loop had been quitting halfway through for reasons seemingly beyond any logic or comprehension.

I already had a hunch that the query I wrote earlier was needlessly complex, as one might expect considering it was written during a state of delirium. It was easy to simplify it. I hoped that would be it, but no change occurred. On the contrary, some checks I slipped in there in the meantime were *passing*.

Further and further investigation finally revealed that the exception handler for background services doesn't actually do what it's expected to do. It's been silently dropping the exceptions that EF had been throwing this whole time, telling me quite plainly what the problem was and why it was quitting at a seemingly random spot.

The fix: Turns out the query evaluates during the foreach loop, which conflicts with wanting to do additional queries within it. Throwing a ToList() somewhere in it seems to have solved it.

These last few hours were quite the adventure...
My head hurts. Now I'm off to take a look at the exception handler.
This commit is contained in:
Noi 2022-03-21 23:47:35 -07:00
parent 2a1478fb0b
commit 021def4f88

View file

@ -16,43 +16,39 @@ class BirthdayRoleUpdate : BackgroundService {
/// </summary> /// </summary>
public override async Task OnTick(int tickCount, CancellationToken token) { public override async Task OnTick(int tickCount, CancellationToken token) {
// For database efficiency, fetch all database information at once before proceeding // For database efficiency, fetch all database information at once before proceeding
// and combine it into the guild IDs that will be processed
using var db = new BotDatabaseContext(); using var db = new BotDatabaseContext();
var shardGuilds = ShardInstance.DiscordClient.Guilds.Select(g => (long)g.Id).ToHashSet(); var shardGuilds = ShardInstance.DiscordClient.Guilds.Select(g => (long)g.Id).ToHashSet();
var settings = db.GuildConfigurations.Where(s => shardGuilds.Contains(s.GuildId)); var presentGuildSettings = db.GuildConfigurations.Where(s => shardGuilds.Contains(s.GuildId));
var guildChecks = shardGuilds.Join(settings, o => o, i => i.GuildId, (id, conf) => new { Key = (ulong)id, Value = conf }); var guildChecks = presentGuildSettings.ToList().Select(s => new Tuple<ulong, GuildConfig>((ulong)s.GuildId, s));
var exceptions = new List<Exception>(); var exceptions = new List<Exception>();
foreach (var pair in guildChecks) { foreach (var (guildId, settings) in guildChecks) {
var guild = ShardInstance.DiscordClient.GetGuild(pair.Key); var guild = ShardInstance.DiscordClient.GetGuild(guildId);
if (guild == null) continue; // A guild disappeared...? if (guild == null) continue; // A guild disappeared...?
var guildConf = pair.Value;
// Check task cancellation here. Processing during a single guild is never interrupted. // Check task cancellation here. Processing during a single guild is never interrupted.
if (token.IsCancellationRequested) throw new TaskCanceledException(); if (token.IsCancellationRequested) throw new TaskCanceledException();
if (ShardInstance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) { if (ShardInstance.DiscordClient.ConnectionState != ConnectionState.Connected) {
Log("Client is not connected. Stopping early."); Log("Client is not connected. Stopping early.");
return; return;
} }
try { try {
// Verify that role settings and permissions are usable // Verify that role settings and permissions are usable
SocketRole? role = guild.GetRole((ulong)(guildConf.RoleId ?? 0)); SocketRole? role = guild.GetRole((ulong)(settings.RoleId ?? 0));
if (role == null || !guild.CurrentUser.GuildPermissions.ManageRoles || role.Position >= guild.CurrentUser.Hierarchy) return; if (role == null || !guild.CurrentUser.GuildPermissions.ManageRoles || role.Position >= guild.CurrentUser.Hierarchy) return;
// Load up user configs and begin processing birthdays // Load up user configs and begin processing birthdays
await db.Entry(guildConf).Collection(t => t.UserEntries).LoadAsync(CancellationToken.None); await db.Entry(settings).Collection(t => t.UserEntries).LoadAsync(CancellationToken.None);
var birthdays = GetGuildCurrentBirthdays(guildConf.UserEntries, guildConf.TimeZone); var birthdays = GetGuildCurrentBirthdays(settings.UserEntries, settings.TimeZone);
// Note: Don't quit here if zero people are having birthdays. Roles may still need to be removed by BirthdayApply.
// Update roles as appropriate // Add or remove roles as appropriate
var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays); var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays);
// Birthday announcement // Process birthday announcement
var channel = guild.GetTextChannel((ulong)(guildConf.ChannelAnnounceId ?? 0));
if (announcementList.Any()) { if (announcementList.Any()) {
await AnnounceBirthdaysAsync(guildConf, channel, announcementList); await AnnounceBirthdaysAsync(settings, guild, announcementList);
} }
} catch (Exception ex) { } catch (Exception ex) {
// Catch all exceptions per-guild but continue processing, throw at end. // Catch all exceptions per-guild but continue processing, throw at end.
@ -66,9 +62,7 @@ class BirthdayRoleUpdate : BackgroundService {
/// Gets all known users from the given guild and returns a list including only those who are /// Gets all known users from the given guild and returns a list including only those who are
/// currently experiencing a birthday in the respective time zone. /// currently experiencing a birthday in the respective time zone.
/// </summary> /// </summary>
#pragma warning disable 618
[Obsolete(Database.ObsoleteReason)] [Obsolete(Database.ObsoleteReason)]
#pragma warning restore 618
public static HashSet<ulong> GetGuildCurrentBirthdays(IEnumerable<GuildUserConfiguration> guildUsers, string? defaultTzStr) { public static HashSet<ulong> GetGuildCurrentBirthdays(IEnumerable<GuildUserConfiguration> guildUsers, string? defaultTzStr) {
var tzdb = DateTimeZoneProviders.Tzdb; var tzdb = DateTimeZoneProviders.Tzdb;
DateTimeZone defaultTz = (defaultTzStr != null ? DateTimeZoneProviders.Tzdb.GetZoneOrNull(defaultTzStr) : null) DateTimeZone defaultTz = (defaultTzStr != null ? DateTimeZoneProviders.Tzdb.GetZoneOrNull(defaultTzStr) : null)
@ -94,33 +88,27 @@ class BirthdayRoleUpdate : BackgroundService {
} }
return birthdayUsers; return birthdayUsers;
} }
/// <summary> /// <summary>
/// Gets all known users from the given guild and returns a list including only those who are /// Gets all known users from the given guild and returns a list including only those who are
/// currently experiencing a birthday in the respective time zone. /// currently experiencing a birthday in the respective time zone.
/// </summary> /// </summary>
public static HashSet<ulong> GetGuildCurrentBirthdays(IEnumerable<UserEntry> guildUsers, string? defaultTzStr) { public static HashSet<ulong> GetGuildCurrentBirthdays(IEnumerable<UserEntry> guildUsers, string? ServerDefaultTzId) {
var tzdb = DateTimeZoneProviders.Tzdb;
DateTimeZone defaultTz = (defaultTzStr != null ? DateTimeZoneProviders.Tzdb.GetZoneOrNull(defaultTzStr) : null) ?? tzdb.GetZoneOrNull("UTC")!;
var birthdayUsers = new HashSet<ulong>(); var birthdayUsers = new HashSet<ulong>();
foreach (var item in guildUsers) {
// Determine final time zone to use for calculation
DateTimeZone tz = (item.TimeZone != null ? tzdb.GetZoneOrNull(item.TimeZone) : null) ?? defaultTz;
var targetMonth = item.BirthMonth; foreach (var record in guildUsers) {
var targetDay = item.BirthDay; // Determine final time zone to use for calculation
DateTimeZone tz = DateTimeZoneProviders.Tzdb
.GetZoneOrNull(record.TimeZone ?? ServerDefaultTzId ?? "UTC")!;
var checkNow = SystemClock.Instance.GetCurrentInstant().InZone(tz); var checkNow = SystemClock.Instance.GetCurrentInstant().InZone(tz);
// Special case: If birthday is February 29 and it's not a leap year, recognize it on March 1st // Special case: If user's birthday is 29-Feb and it's currently not a leap year, check against 1-Mar
if (targetMonth == 2 && targetDay == 29 && !DateTime.IsLeapYear(checkNow.Year)) { if (!DateTime.IsLeapYear(checkNow.Year) && record.BirthMonth == 2 && record.BirthDay == 29) {
targetMonth = 3; if (checkNow.Month == 3 && checkNow.Day == 1) birthdayUsers.Add((ulong)record.UserId);
targetDay = 1; } else if (record.BirthMonth == checkNow.Month && record.BirthDay== checkNow.Day) {
} birthdayUsers.Add((ulong)record.UserId);
if (targetMonth == checkNow.Month && targetDay == checkNow.Day) {
birthdayUsers.Add((ulong)item.UserId);
} }
} }
return birthdayUsers; return birthdayUsers;
} }
@ -130,30 +118,33 @@ class BirthdayRoleUpdate : BackgroundService {
/// <returns> /// <returns>
/// List of users who had the birthday role applied, used to announce. /// List of users who had the birthday role applied, used to announce.
/// </returns> /// </returns>
private static async Task<IEnumerable<SocketGuildUser>> UpdateGuildBirthdayRoles(SocketGuild g, SocketRole r, HashSet<ulong> names) { private async Task<IEnumerable<SocketGuildUser>> UpdateGuildBirthdayRoles(SocketGuild g, SocketRole r, HashSet<ulong> toApply) {
// Check members currently with the role. Figure out which users to remove it from. var removals = new List<SocketGuildUser>(); // TODO check if roles can be removed in-place instead of building a list first
var roleRemoves = new List<SocketGuildUser>(); var no_ops = new HashSet<ulong>();
var roleKeeps = new HashSet<ulong>(); var additions = new List<SocketGuildUser>();
foreach (var member in r.Members) {
if (!names.Contains(member.Id)) roleRemoves.Add(member); // Scan role for members no longer needing it
else roleKeeps.Add(member.Id); foreach (var user in r.Members) {
if (!toApply.Contains(user.Id)) removals.Add(user);
else no_ops.Add(user.Id);
}
foreach (var user in removals) {
// TODO this gets hit with rate limits sometimes. figure something out.
await user.RemoveRoleAsync(r);
} }
foreach (var user in roleRemoves) { foreach (var target in toApply) {
await user.RemoveRoleAsync(r).ConfigureAwait(false); if (no_ops.Contains(target)) continue;
}
// Apply role to members not already having it. Prepare announcement list. var user = g.GetUser(target);
var newBirthdays = new List<SocketGuildUser>(); if (user == null) {
foreach (var target in names) { Log($"Encountered null user while processing guild {g.Id}. User: {target}.");
var member = g.GetUser(target); continue;
if (member == null) continue; }
if (roleKeeps.Contains(member.Id)) continue; // already has role - do nothing await user.AddRoleAsync(r);
await member.AddRoleAsync(r).ConfigureAwait(false); additions.Add(user);
newBirthdays.Add(member);
} }
return additions;
return newBirthdays;
} }
public const string DefaultAnnounce = "Please wish a happy birthday to %n!"; public const string DefaultAnnounce = "Please wish a happy birthday to %n!";
@ -162,7 +153,8 @@ class BirthdayRoleUpdate : BackgroundService {
/// <summary> /// <summary>
/// Attempts to send an announcement message. /// Attempts to send an announcement message.
/// </summary> /// </summary>
private static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketTextChannel? c, IEnumerable<SocketGuildUser> names) { internal static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketGuild g, IEnumerable<SocketGuildUser> names) {
var c = g.GetTextChannel((ulong)(settings.ChannelAnnounceId ?? 0));
if (c == null) return; if (c == null) return;
if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return; if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return;