fix(connections): stop a transient request timeout from killing the connection loop

A per-request timeout during a connected window surfaces as an
OperationCanceledException even when the connection token is NOT cancelled.
GetRigPositionsAsync rethrew it unconditionally and RunAsync treats any OCE as
a shutdown signal, so a transient slow map endpoint permanently terminated the
per-server connection loop with no reconnect (bot connected, dropped ~20s later,
then went silent). Confirmed via a live repro trace.

Guard the timeout catches with `when (ct.IsCancellationRequested)` in
GetRigPositionsAsync, PollMarkersAsync and PollReachabilityAsync so a request
timeout degrades that one window instead of tearing down the connection, and add
the missing `.WaitAsync` timeout guard to GetClanInfoAsync (the one connect-time
probe that could block indefinitely on a dead socket).

Regression test: Connect_MonumentsTimeout_DoesNotKillLoop_AndStillReconnects.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: HandyS11

src/RustPlusBot.Features.Connections/Listening/RustPlusSocketSource.cs

@@ -380,7 +380,10 @@ public async Task<ClanProbeResult> GetClanInfoAsync(TimeSpan timeout, Cancellati
             {
                 // CONFIRMED (2.0.0-beta.4): GetClanInfoAsync returns Task<Response<ClanInfo>>, exposing
                 // IsSuccess, Data, and Error?.Code as the response accessors.
-                var response = await _rustPlus.GetClanInfoAsync(timeoutCts.Token).ConfigureAwait(false);
+                // .WaitAsync guards the timeout even if the beta call doesn't internally honor the token
+                // (matching the other probes): on a dead socket this must never block the connect path.
+                var response = await _rustPlus.GetClanInfoAsync(timeoutCts.Token)
+                    .WaitAsync(timeoutCts.Token).ConfigureAwait(false);
                 return ClanMapping.FromResponse(response.IsSuccess, response.Error?.Code, response.Data);
             }
             catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)

src/RustPlusBot.Features.Connections/Supervisor/ConnectionSupervisor.cs

@@ -703,14 +703,16 @@ await eventBus.PublishAsync(
                         .ConfigureAwait(false);
                 }
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (ct.IsCancellationRequested)
             {
                 return; // stopping
             }
-#pragma warning disable CA1031 // Broad catch: a failed poll is logged and skipped; the previous snapshot is retained.
+#pragma warning disable CA1031 // Broad catch: a failed poll (incl. a per-request timeout) is logged and skipped; the previous snapshot is retained.
             catch (Exception ex)
 #pragma warning restore CA1031
             {
+                // A per-request timeout (OperationCanceledException with ct NOT cancelled) must NOT end the
+                // poll loop — that would silently stop marker/rig/AFK detection for the rest of the connection.
                 LogMarkerPollFailed(logger, ex, key.Server);
             }
 
@@ -767,14 +769,16 @@ private async Task PollReachabilityAsync(
             {
                 current = await ReadAllReachabilityAsync(key, connection, ct).ConfigureAwait(false);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (ct.IsCancellationRequested)
             {
                 throw;
             }
-#pragma warning disable CA1031 // Broad catch: a failed reachability sweep is logged and retried next cycle.
+#pragma warning disable CA1031 // Broad catch: a failed reachability sweep (incl. a per-request timeout) is logged and retried next cycle.
             catch (Exception ex)
 #pragma warning restore CA1031
             {
+                // A per-request timeout (OperationCanceledException with ct NOT cancelled) must be retried next
+                // cycle, not rethrown — rethrowing would tear the sweep down for the rest of the connection.
                 LogReachabilityPollFailed(logger, ex, key.Server);
                 continue;
             }
@@ -942,14 +946,17 @@ private async Task<IReadOnlyList<RigPosition>> GetRigPositionsAsync(
 
             return rigs;
         }
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (ct.IsCancellationRequested)
         {
+            // Real shutdown/reconnect of THIS connection: propagate so the loop tears down.
             throw;
         }
-#pragma warning disable CA1031 // Broad catch: a monuments-fetch failure just disables rig detection this window.
+#pragma warning disable CA1031 // Broad catch: a monuments-fetch failure (incl. a per-request timeout) just disables rig detection this window.
         catch (Exception ex)
 #pragma warning restore CA1031
         {
+            // A per-request timeout surfaces as OperationCanceledException with ct NOT cancelled; it must
+            // degrade rig detection for this window, never terminate the connection loop.
             LogMonumentsFetchFailed(logger, ex, serverId); // rig detection degrades gracefully for this window
             return [];
         }

tests/RustPlusBot.Features.Connections.Tests/ConnectionSupervisorTests.cs

@@ -231,6 +231,33 @@ public async Task Heartbeat_Unreachable_ReconnectsAndRecovers()
         Assert.True(source.CreateCount >= 2);
     }
 
+    /// <summary>
+    /// A per-request timeout during connect-time setup (here the oil-rig monuments fetch) surfaces as an
+    /// <see cref="OperationCanceledException"/> even though the connection token is not cancelled. It must
+    /// degrade rig detection for this window, NOT terminate the whole connection loop — otherwise a
+    /// transient slow map endpoint permanently kills the connection with no reconnect (the real-world bug).
+    /// </summary>
+    [Fact]
+    public async Task Connect_MonumentsTimeout_DoesNotKillLoop_AndStillReconnects()
+    {
+        var source = new FakeRustSocketSource();
+        source.EnqueueConnect(SocketConnectOutcome.Connected);
+        source.TimeoutOnMonumentsOnce(); // connect-time rig fetch times out on the FIRST connection only
+        source.EnqueueHeartbeat(HeartbeatResult.Ok(2)); // first heartbeat -> Connected
+        source.EnqueueHeartbeat(HeartbeatResult.Unreachable); // next heartbeat -> drop
+        source.EnqueueConnect(SocketConnectOutcome.Connected); // reconnect
+        source.EnqueueHeartbeat(HeartbeatResult.Ok(4));
+        await using var h = CreateHarness(source);
+        var (serverId, _, _) = await SeedAsync(h.Provider);
+
+        await h.Supervisor.EnsureConnectionAsync(10UL, serverId);
+
+        var recovered = await WaitForStateAsync(
+            h.Provider, serverId, s => s.Status == ConnectionStatus.Connected && s.PlayerCount == 4);
+        Assert.NotNull(recovered);
+        Assert.True(source.CreateCount >= 2);
+    }
+
     /// <summary>
     /// WasConnected must be computed from in-process state (the DB status survives restarts and
     /// would claim Connected at boot): statuses before the first Connected carry false; the drop

tests/RustPlusBot.Features.Connections.Tests/Fakes/FakeRustSocketSource.cs

@@ -29,6 +29,7 @@ internal sealed class FakeRustSocketSource : IRustSocketSource
     private HeartbeatResult _lastHeartbeat = HeartbeatResult.Ok(0);
     private ClanProbeResult? _pendingClanProbe;
     private IReadOnlyList<MonumentSnapshot> _pendingMonuments = [];
+    private bool _pendingMonumentsTimeout;
 
     /// <summary>Number of times <see cref="Create"/> has been called. Safe to read from any thread.</summary>
     public int CreateCount => Volatile.Read(ref _createCount);
@@ -60,6 +61,11 @@ public IRustServerConnection Create(string ip, int port, ulong steamId, string p
         connection.MonumentsResult = _pendingMonuments;
         _pendingMonuments = [];
 
+        // Transfer any pre-staged monuments timeout so the connect-time rig fetch throws for the NEXT
+        // connection only (mimicking a real per-request timeout, which surfaces as OperationCanceledException).
+        connection.MonumentsTimeout = _pendingMonumentsTimeout;
+        _pendingMonumentsTimeout = false;
+
         // Transfer any pre-staged storage contents so they are in place before the prime loop starts.
         foreach (var (entityId, contents) in _pendingStorageContents)
         {
@@ -119,6 +125,14 @@ public void EnqueueMarkers(IReadOnlyList<MapMarkerSnapshot> markers) =>
     /// <param name="monuments">The monument list to return from <see cref="IRustServerConnection.GetMonumentsAsync"/>.</param>
     public void SetMonuments(IReadOnlyList<MonumentSnapshot> monuments) => _pendingMonuments = monuments;
 
+    /// <summary>
+    /// Makes the NEXT connection's connect-time <see cref="FakeConnection.GetMonumentsAsync"/> throw
+    /// <see cref="OperationCanceledException"/>, simulating a per-request timeout (the outer connection
+    /// token is NOT cancelled). Applies to the next connection only. Call before
+    /// <see cref="EnsureConnectionAsync"/>.
+    /// </summary>
+    public void TimeoutOnMonumentsOnce() => _pendingMonumentsTimeout = true;
+
     /// <summary>
     /// Pre-stages the probe result returned by <see cref="FakeConnection.GetClanInfoAsync"/> for the NEXT
     /// connection created by <see cref="Create"/>. Transferred to the new connection at creation time, before
@@ -244,6 +258,10 @@ internal sealed class FakeConnection(SocketConnectOutcome outcome, FakeRustSocke
         /// <summary>The monuments returned by <see cref="GetMonumentsAsync"/>. Defaults to empty.</summary>
         public IReadOnlyList<MonumentSnapshot> MonumentsResult { get; set; } = [];
 
+        /// <summary>When true, <see cref="GetMonumentsAsync"/> throws <see cref="OperationCanceledException"/>
+        /// (a per-request timeout) instead of returning <see cref="MonumentsResult"/>.</summary>
+        public bool MonumentsTimeout { get; set; }
+
         /// <summary>The bytes returned by <see cref="GetMapImageAsync"/>. Defaults to null.</summary>
         public byte[]? MapImageResult { get; set; }
 
@@ -393,7 +411,9 @@ public Task<IReadOnlyList<MapMarkerSnapshot>> GetMapMarkersAsync(TimeSpan timeou
 
         public Task<IReadOnlyList<MonumentSnapshot>> GetMonumentsAsync(TimeSpan timeout,
             CancellationToken cancellationToken = default) =>
-            Task.FromResult(MonumentsResult);
+            MonumentsTimeout
+                ? Task.FromException<IReadOnlyList<MonumentSnapshot>>(new OperationCanceledException())
+                : Task.FromResult(MonumentsResult);
 
         public Task<byte[]?> GetMapImageAsync(TimeSpan timeout, CancellationToken cancellationToken = default) =>
             Task.FromResult(MapImageResult);