Merge pull request #44 from NoiTheCat/fixes/userdl-hang

Fix hanging, unnecessary disconnections when AutoUserDownload is hanging

BackgroundServices/AutoUserDownload.cs

@@ -8,17 +8,27 @@ namespace BirthdayBot.BackgroundServices;
 class AutoUserDownload : BackgroundService {
     public AutoUserDownload(ShardInstance instance) : base(instance) { }
 
+    private static readonly HashSet<ulong> _failedDownloads = new();
+    private static readonly TimeSpan _singleDlTimeout = ShardManager.DeadShardThreshold / 3;
+
     public override async Task OnTick(int tickCount, CancellationToken token) {
         // Take action if a guild's cache is incomplete...
-        var incompleteCaches = ShardInstance.DiscordClient.Guilds.Where(g => !g.HasAllMembers).Select(g => g.Id).ToHashSet();
+        var incompleteCaches = ShardInstance.DiscordClient.Guilds
+            .Where(g => !g.HasAllMembers)
+            .Select(g => g.Id)
+            .ToHashSet();
         // ...and if the guild contains any user data
         IEnumerable<ulong> mustFetch;
         try {
             await DbConcurrentOperationsLock.WaitAsync(token);
             using var db = new BotDatabaseContext();
-            mustFetch = db.UserEntries.AsNoTracking()
+            lock (_failedDownloads)
-                .Where(e => incompleteCaches.Contains(e.GuildId)).Select(e => e.GuildId).Distinct()
+                mustFetch = db.UserEntries.AsNoTracking()
-                .ToList();
+                    .Where(e => incompleteCaches.Contains(e.GuildId))
+                    .Select(e => e.GuildId)
+                    .Distinct()
+                    .Where(e => !_failedDownloads.Contains(e))
+                    .ToList();
         } finally {
             try {
                 DbConcurrentOperationsLock.Release();
@@ -26,17 +36,31 @@ class AutoUserDownload : BackgroundService {
         }
         
         var processed = 0;
+        var processStartTime = DateTimeOffset.UtcNow;
         foreach (var item in mustFetch) {
-            // May cause a disconnect in certain situations. Cancel all further attempts until the next pass if it happens.
+            // May cause a disconnect in certain situations. Make no further attempts until the next pass if it happens.
             if (ShardInstance.DiscordClient.ConnectionState != ConnectionState.Connected) break;
 
-            var guild = ShardInstance.DiscordClient.GetGuild((ulong)item);
+            var guild = ShardInstance.DiscordClient.GetGuild(item);
             if (guild == null) continue; // A guild disappeared...?
-            await guild.DownloadUsersAsync().ConfigureAwait(false); // We're already on a seperate thread, no need to use Task.Run
+
-            await Task.Delay(200, CancellationToken.None).ConfigureAwait(false); // Must delay, or else it seems to hang...
+            await Task.Delay(200, CancellationToken.None); // Delay a bit (reduces the possibility of hanging, somehow).
             processed++;
+            var dl = guild.DownloadUsersAsync();
+            dl.Wait((int)_singleDlTimeout.TotalMilliseconds / 2, token);
+            if (dl.IsFaulted) {
+                Log("Exception thrown by download task: " + dl.Exception);
+                break;
+            } else if (!dl.IsCompletedSuccessfully) {
+                Log($"Task for guild {guild.Id} is unresponsive. Skipping guild. Members: {guild.MemberCount}. Name: {guild.Name}.");
+                lock (_failedDownloads) _failedDownloads.Add(guild.Id);
+                continue;
+            }
+
+            // Prevent unnecessary disconnections by ShardManager if we're taking too long
+            if (DateTimeOffset.UtcNow - processStartTime > _singleDlTimeout) break;
         }
 
-        if (processed > 25) Log($"Explicit user list request processed for {processed} guild(s).");
+        if (processed > 10) Log($"Member list downloads handled for {processed} guilds.");
     }
 }

BirthdayBot.csproj

@@ -5,7 +5,7 @@
     <TargetFramework>net6.0</TargetFramework>
 	  <ImplicitUsings>enable</ImplicitUsings>
 	  <Nullable>enable</Nullable>
-    <Version>3.4.7</Version>
+    <Version>3.4.8</Version>
     <Authors>NoiTheCat</Authors>
   </PropertyGroup>
 
@@ -22,7 +22,7 @@
 
   <ItemGroup>
     <PackageReference Include="CommandLineParser" Version="2.9.1" />
-    <PackageReference Include="Discord.Net" Version="3.8.1" />
+    <PackageReference Include="Discord.Net" Version="3.9.0" />
     <PackageReference Include="EFCore.NamingConventions" Version="7.0.0" />
     <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="7.0.0">
       <PrivateAssets>all</PrivateAssets>

ShardManager.cs

@@ -25,7 +25,7 @@ class ShardManager : IDisposable {
     /// Amount of time without a completed background service run before a shard instance
     /// is considered "dead" and tasked to be removed.
     /// </summary>
-    private static readonly TimeSpan DeadShardThreshold = new(0, 20, 0);
+    public static readonly TimeSpan DeadShardThreshold = new(0, 20, 0);
 
     /// <summary>
     /// A dictionary with shard IDs as its keys and shard instances as its values.