detect timout of critical tracer

Author: mgravell

src/StackExchange.Redis/Enums/CommandFlags.cs

@@ -109,5 +109,7 @@ public enum CommandFlags
         // 1024: Removed - was used for async timeout checks; never user-specified, so not visible on the public API
 
         // 2048: Use subscription connection type; never user-specified, so not visible on the public API
+
+        // 4096: Identifies handshake completion messages; never user-specified, so not visible on the public API
     }
 }

src/StackExchange.Redis/Message.cs

@@ -61,7 +61,8 @@ internal abstract partial class Message : ICompletable
 
         private const CommandFlags AskingFlag = (CommandFlags)32,
                                    ScriptUnavailableFlag = (CommandFlags)256,
-                                   DemandSubscriptionConnection = (CommandFlags)2048;
+                                   DemandSubscriptionConnection = (CommandFlags)2048,
+                                   HandshakeCompletionFlag = (CommandFlags)4096;
 
         private const CommandFlags MaskPrimaryServerPreference = CommandFlags.DemandMaster
                                                                | CommandFlags.DemandReplica
@@ -720,6 +721,8 @@ internal void SetWriteTime()
 
         public virtual string CommandString => Command.ToString();
 
+        public bool IsHandshakeCompletion => (Flags & HandshakeCompletionFlag) != 0;
+
         /// <summary>
         /// Sends this command to the subscription connection rather than the interactive.
         /// </summary>
@@ -742,6 +745,8 @@ internal void SetAsking(bool value)
             else Flags &= ~AskingFlag; // and the bits taketh away
         }
 
+        internal void SetHandshakeCompletion() => Flags |= HandshakeCompletionFlag;
+
         internal void SetNoRedirect() => Flags |= CommandFlags.NoRedirect;
 
         internal void SetPreferPrimary() =>

src/StackExchange.Redis/PhysicalBridge.cs

@@ -617,7 +617,6 @@ internal void OnHeartbeat(bool ifConnectedOnly)
                         // We need to time that out and cleanup the PhysicalConnection if needed, otherwise that reader and socket will remain open
                         // for the lifetime of the application due to being orphaned, yet still referenced by the active task doing the pipe read.
                     case (int)State.ConnectedEstablished:
-                        shouldResetConnectionRetryCount = true;
                         var tmp = physical;
                         if (tmp != null)
                         {

src/StackExchange.Redis/PhysicalConnection.cs

@@ -282,15 +282,15 @@ internal void SetProtocol(RedisProtocol value)
         }
 
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times", Justification = "Trust me yo")]
-        internal void Shutdown()
+        internal void Shutdown(ConnectionFailureType failureType = ConnectionFailureType.ConnectionDisposed)
         {
             var ioPipe = Interlocked.Exchange(ref _ioPipe, null); // compare to the critical read
             var socket = Interlocked.Exchange(ref _socket, null);
 
             if (ioPipe != null)
             {
                 Trace("Disconnecting...");
-                try { BridgeCouldBeNull?.OnDisconnected(ConnectionFailureType.ConnectionDisposed, this, out _, out _); } catch { }
+                try { BridgeCouldBeNull?.OnDisconnected(failureType, this, out _, out _); } catch { }
                 try { ioPipe.Input?.CancelPendingRead(); } catch { }
                 try { ioPipe.Input?.Complete(); } catch { }
                 try { ioPipe.Output?.CancelPendingFlush(); } catch { }
@@ -777,6 +777,12 @@ internal int OnBridgeHeartbeat()
                                 multiplexer.OnAsyncTimeout();
                                 result++;
                             }
+                            else if (msg.IsHandshakeCompletion)
+                            {
+                                // Critical handshake validation timed out; note that this doesn't have a result-box,
+                                // so doesn't get timed out via the above.
+                                Shutdown(ConnectionFailureType.UnableToConnect);
+                            }
                         }
                         else
                         {

src/StackExchange.Redis/ServerEndPoint.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Net;
 using System.Runtime.CompilerServices;
@@ -1068,8 +1069,10 @@ private async Task HandshakeAsync(PhysicalConnection connection, ILogger? log)
             }
 
             var tracer = GetTracerMessage(true);
+            tracer.SetHandshakeCompletion();
             tracer = LoggingMessage.Create(log, tracer);
             log?.LogInformationSendingCriticalTracer(new(this), tracer.CommandAndKey);
+            Debug.Assert(tracer.IsHandshakeCompletion, "Tracer message should identify as handshake completion");
             await WriteDirectOrQueueFireAndForgetAsync(connection, tracer, ResultProcessor.EstablishConnection).ForAwait();
 
             // Note: this **must** be the last thing on the subscription handshake, because after this

tests/StackExchange.Redis.Tests/RetryPolicyUnitTests.cs

@@ -19,7 +19,7 @@ public class RetryPolicyUnitTests(ITestOutputHelper log)
     [InlineData(FailureMode.SlowNonConnect)]
     [InlineData(FailureMode.NoResponses)]
     [InlineData(FailureMode.GarbageResponses)]
-    public async Task TestExponentialRetry(FailureMode failureMode)
+    public async Task RetryPolicyFailureCases(FailureMode failureMode)
     {
         using var server = new NonResponsiveServer(log);
         var options = server.GetClientConfig(withPubSub: false);