fix(client): GetServers() throws FormatException on DNS hostnames

rust/src/ffi.rs

@@ -375,6 +375,7 @@ pub(crate) unsafe fn create_connection_request(
         tcp_nodelay: false,
         periodic_checks: None, // TODO #485: Expose periodic_checks in ClusterClientConfiguration.
         inflight_requests_limit: None, // TODO #484: Expose inflight_requests_limit in ConnectionConfiguration.
+        recovery_requests_queue_size: None,
         address_resolver: None,
         client_circuit_breaker: None,
     })

sources/Valkey.Glide/Abstract/ConnectionMultiplexer.cs

@@ -127,8 +127,19 @@ public IServer[] GetServers()
         // run INFO on all nodes, but disregard the node responses, we need node addresses only
         if (_db!.IsCluster)
         {
-            Dictionary<string, string> info = _db.Command(Request.Info([]).ToMultiNodeValue(), Route.AllNodes).GetAwaiter().GetResult();
-            return [.. info.Keys.Select(addr => new ValkeyServer(_db, IPEndPoint.Parse(addr)))];
+            var servers = new List<ValkeyServer>();
+
+            foreach (var addr in _db.Command(Request.Info([]).ToMultiNodeValue(), Route.AllNodes).GetAwaiter().GetResult().Keys)
+            {
+                if (!Format.TryParseEndPoint(addr, out EndPoint? ep))
+                {
+                    throw new FormatException($"Could not parse endpoint address: '{addr}'");
+                }
+
+                servers.Add(new ValkeyServer(_db, ep));
+            }
+
+            return [.. servers];
         }
         else
         {

tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs

@@ -360,7 +360,7 @@ public async Task TestClientId_WithRoute(GlideClusterClient client)
         // Test CLIENT ID with all nodes routing
         var allNodesResult = await client.ClientIdAsync(AllNodes);
         Assert.True(allNodesResult.HasMultiData);
-        Assert.True(allNodesResult.MultiValue.Count > 0);
+        Assert.NotEmpty(allNodesResult.MultiValue);
 
         foreach (var kvp in allNodesResult.MultiValue)
         {
@@ -414,7 +414,7 @@ public async Task TestClientGetName_WithRoute(GlideClusterClient client)
         // Test CLIENT GETNAME with all nodes routing
         var allNodesResult = await client.ClientGetNameAsync(AllNodes);
         Assert.True(allNodesResult.HasMultiData);
-        Assert.True(allNodesResult.MultiValue.Count > 0);
+        Assert.NotEmpty(allNodesResult.MultiValue);
 
         foreach (var kvp in allNodesResult.MultiValue)
         {

tests/Valkey.Glide.IntegrationTests/DnsTests.cs

@@ -1,8 +1,9 @@
 // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0
 
+using System.Net;
+
 using Valkey.Glide.TestUtils;
 
-using static Valkey.Glide.ConnectionConfiguration;
 using static Valkey.Glide.Errors;
 using static Valkey.Glide.TestUtils.Client;
 using static Valkey.Glide.TestUtils.Constants;
@@ -22,100 +23,131 @@ public class DnsTests(DnsTestsFixture fixture) : IClassFixture<DnsTestsFixture>
     /// Environment variable for enabling DNS tests.
     /// See <see href="../../DEVELOPER.md#dns-tests">DEVELOPER.md</see> for more details.
     /// </summary>
-    private const string DnsEnabledEnvVar = "VALKEY_GLIDE_DNS_TESTS_ENABLED";
+    internal const string DnsEnabledEnvVar = "VALKEY_GLIDE_DNS_TESTS_ENABLED";
 
     #endregion
-    #region Tests
+    #region Test Data
 
-    [Theory]
-    [MemberData(nameof(ClusterMode), MemberType = typeof(Data))]
-    public async Task ConnectWithValidHostname_Succeeds(bool useCluster)
+    public static TheoryData<bool> TlsMode => [true, false];
+
+    public static TheoryData<bool, bool> ClusterAndTlsMode => new()
     {
-        SkipIfDnsTestsNotEnabled();
-        await using var client = await BuildClient(useCluster, useTls: false, HostnameNoTls);
-        await AssertConnected(client);
-    }
+        { true, true },
+        { true, false },
+        { false, true },
+        { false, false },
+    };
 
-    [Theory]
-    [MemberData(nameof(ClusterMode), MemberType = typeof(Data))]
-    public async Task ConnectWithInvalidHostname_Fails(bool useCluster)
+    #endregion
+    #region Constructor
+
+    static DnsTests()
     {
-        SkipIfDnsTestsNotEnabled();
-        _ = await Assert.ThrowsAsync<ConnectionException>(async ()
-            => await BuildClient(useCluster, useTls: false, "NONEXISTENT.INVALID"));
+        Assert.SkipWhen(
+            string.IsNullOrEmpty(Environment.GetEnvironmentVariable(DnsEnabledEnvVar)),
+            "DNS tests are disabled. See DEVELOPER.md for setup instructions.");
     }
 
+    #endregion
+    #region Tests
+
     [Theory]
-    [MemberData(nameof(ClusterMode), MemberType = typeof(Data))]
-    public async Task Tls_WithHostnameInCertificate_Succeeds(bool useCluster)
+    [MemberData(nameof(ClusterAndTlsMode))]
+    public async Task Connect_WithValidHostname_Succeeds(bool useCluster, bool useTls)
     {
-        SkipIfDnsTestsNotEnabled();
-        await using var client = await BuildClient(useCluster, useTls: true, HostnameTls);
+        var server = GetServer(useCluster, useTls);
+        var host = useTls ? HostnameTls : HostnameNoTls;
+        await using var client = await server.CreateClientAsync(host);
+
         await AssertConnected(client);
     }
 
     [Theory]
-    [MemberData(nameof(ClusterMode), MemberType = typeof(Data))]
-    public async Task Tls_WithHostnameNotInCertificate_Fails(bool useCluster)
-    {
-        SkipIfDnsTestsNotEnabled();
-        _ = await Assert.ThrowsAsync<ConnectionException>(async ()
-            => await BuildClient(useCluster, useTls: true, HostnameNoTls));
-    }
+    [MemberData(nameof(ClusterAndTlsMode))]
+    public async Task Connect_WithInvalidHostname_Fails(bool useCluster, bool useTls)
+        => _ = await Assert.ThrowsAsync<ConnectionException>(async ()
+            => await GetServer(useCluster, useTls).CreateClientAsync("NONEXISTENT.INVALID"));
 
-    #endregion
-    #region Helpers
+    [Theory]
+    [MemberData(nameof(ClusterMode), MemberType = typeof(Data))]
+    public async Task Connect_WithHostnameNotInCertificate_Fails(bool useCluster)
+        => _ = await Assert.ThrowsAsync<ConnectionException>(async ()
+            => await GetServer(useCluster, useTls: true).CreateClientAsync(HostnameNoTls));
 
     /// <summary>
-    /// Returns true if DNS tests are enabled.
+    /// Verifies that <see cref="ConnectionMultiplexer.GetServers"/> returns servers with <see cref="DnsEndPoint"/>
+    /// instances when the cluster topology reports DNS hostnames (via <c>cluster-announce-hostname</c>).
     /// </summary>
-    public static bool IsDnsTestsEnabled()
-        => !string.IsNullOrEmpty(Environment.GetEnvironmentVariable(DnsEnabledEnvVar));
+    /// <seealso href="https://github.com/valkey-io/valkey-glide-csharp/issues/419">#419</seealso>
+    [Theory]
+    [MemberData(nameof(TlsMode))]
+    public void GetServers_WithDnsHostname_ReturnsDnsEndPoints(bool useTls)
+    {
+        var server = GetServer(useCluster: true, useTls);
+        var config = new ConfigurationOptions { Ssl = useTls };
+        config.EndPoints.Add(server.Address.Host, server.Address.Port);
 
-    /// <summary>
-    /// Skips the current test if DNS tests are not enabled.
-    /// </summary>
-    private static void SkipIfDnsTestsNotEnabled()
-        => Assert.SkipWhen(
-            !IsDnsTestsEnabled(),
-            $"DNS tests are disabled. See DEVELOPER.md for setup instructions.");
+        if (useTls)
+        {
+            config.TrustIssuer(ServerManager.ServerCertificatePath);
+        }
+
+        using var conn = ConnectionMultiplexer.Connect(config);
+
+        var servers = conn.GetServers();
+        Assert.NotEmpty(servers);
+
+        foreach (var s in servers)
+        {
+            _ = Assert.IsType<DnsEndPoint>(s.EndPoint);
+            Assert.Contains(HostnameTls, s.EndPoint.ToString());
+        }
+    }
 
     /// <summary>
-    /// Builds and returns a client configured with the specified parameters.
+    /// Verifies that <see cref="ConnectionMultiplexer.GetEndPoints"/> returns <see cref="DnsEndPoint"/>
+    /// instances when the cluster topology reports DNS hostnames (via <c>cluster-announce-hostname</c>).
     /// </summary>
-    private async Task<BaseClient> BuildClient(bool useCluster, bool useTls, string host)
+    /// <seealso href="https://github.com/valkey-io/valkey-glide-csharp/issues/419">#419</seealso>
+    [Theory]
+    [MemberData(nameof(TlsMode))]
+    public void GetEndPoints_WithDnsHostname_ReturnsDnsEndPoints(bool useTls)
     {
-        if (useCluster)
+        var server = GetServer(useCluster: true, useTls);
+        var config = new ConfigurationOptions { Ssl = useTls };
+        config.EndPoints.Add(server.Address.Host, server.Address.Port);
+
+        if (useTls)
         {
-            var server = useTls ? fixture.TlsClusterServer! : fixture.ClusterServer!;
-            var builder = new ClusterClientConfigurationBuilder()
-                .WithAddress(host, server.Address.Port);
+            config.TrustIssuer(ServerManager.ServerCertificatePath);
+        }
 
-            if (useTls)
-            {
-                _ = builder.WithTls();
-                _ = builder.WithTrustedCertificate(server.CertificateData!);
-            }
+        using var conn = ConnectionMultiplexer.Connect(config);
 
-            return await GlideClusterClient.CreateClient(builder.Build());
-        }
+        var endpoints = conn.GetEndPoints(false);
+        Assert.NotEmpty(endpoints);
 
-        else
+        foreach (var endpoint in endpoints)
         {
-            var server = useTls ? fixture.TlsStandaloneServer! : fixture.StandaloneServer!;
-            var builder = new StandaloneClientConfigurationBuilder()
-                .WithAddress(host, server.Address.Port);
+            _ = Assert.IsType<DnsEndPoint>(endpoint);
+            Assert.Contains(HostnameTls, endpoint.ToString());
 
-            if (useTls)
-            {
-                _ = builder.WithTls();
-                _ = builder.WithTrustedCertificate(server.CertificateData!);
-            }
-
-            return await GlideClient.CreateClient(builder.Build());
+            var s = conn.GetServer(endpoint);
+            Assert.Equal(endpoint, s.EndPoint);
         }
     }
 
+    #endregion
+    #region Helpers
+
+    private Server GetServer(bool useCluster, bool useTls) => (useCluster, useTls) switch
+    {
+        (true, true) => fixture.TlsClusterServer!,
+        (true, false) => fixture.ClusterServer!,
+        (false, true) => fixture.TlsStandaloneServer!,
+        _ => fixture.StandaloneServer!,
+    };
+
     #endregion
 }
 
@@ -131,14 +163,10 @@ public class DnsTestsFixture : IAsyncLifetime
 
     public ValueTask InitializeAsync()
     {
-        // Only start the servers if DNS tests are enabled.
-        if (DnsTests.IsDnsTestsEnabled())
-        {
-            ClusterServer = new(useTls: false);
-            StandaloneServer = new(useTls: false);
-            TlsClusterServer = new(useTls: true);
-            TlsStandaloneServer = new(useTls: true);
-        }
+        ClusterServer = new(host: HostnameTls);
+        StandaloneServer = new(host: HostnameTls);
+        TlsClusterServer = new(useTls: true, host: HostnameTls);
+        TlsStandaloneServer = new(useTls: true, host: HostnameTls);
 
         return ValueTask.CompletedTask;
     }

tests/Valkey.Glide.IntegrationTests/ScriptingCommandTests.cs

@@ -1043,7 +1043,7 @@ public async Task FunctionStatsAsync_ReturnsStatistics(GlideClient client)
 
         Assert.NotNull(stats);
         Assert.NotNull(stats.Engines);
-        Assert.True(stats.Engines.Count > 0);
+        Assert.NotEmpty(stats.Engines);
 
         // Check LUA engine stats
         Assert.True(stats.Engines.ContainsKey("LUA"));
@@ -1119,7 +1119,7 @@ public async Task FunctionDumpAsync_CreatesBackup(GlideClient client)
         byte[] backup = await client.FunctionDumpAsync();
 
         Assert.NotNull(backup);
-        Assert.True(backup.Length > 0);
+        Assert.NotEmpty(backup);
     }
 
     [Theory(DisableDiscoveryEnumeration = true)]
@@ -1246,7 +1246,7 @@ public async Task FCallAsync_WithAllPrimariesRouting_ExecutesOnAllPrimaries(Glid
         // Verify load succeeded (may be single or multi-value depending on cluster configuration)
         if (loadResult.HasMultiData)
         {
-            Assert.True(loadResult.MultiValue.Count > 0);
+            Assert.NotEmpty(loadResult.MultiValue);
             Assert.All(loadResult.MultiValue.Values, name => Assert.Equal(libName, name));
         }
         else
@@ -1260,7 +1260,7 @@ public async Task FCallAsync_WithAllPrimariesRouting_ExecutesOnAllPrimaries(Glid
         // Verify execution (may be single or multi-value depending on cluster configuration)
         if (result.HasMultiData)
         {
-            Assert.True(result.MultiValue.Count > 0);
+            Assert.NotEmpty(result.MultiValue);
             Assert.All(result.MultiValue.Values, r => Assert.Equal("Hello from primary", r.ToString()));
         }
         else
@@ -1294,7 +1294,7 @@ public async Task FCallAsync_WithAllNodesRouting_ExecutesOnAllNodes(GlideCluster
         // Verify load succeeded (may be single or multi-value depending on cluster configuration)
         if (loadResult.HasMultiData)
         {
-            Assert.True(loadResult.MultiValue.Count > 0);
+            Assert.NotEmpty(loadResult.MultiValue);
         }
         else
         {
@@ -1307,7 +1307,7 @@ public async Task FCallAsync_WithAllNodesRouting_ExecutesOnAllNodes(GlideCluster
         // Verify execution (may be single or multi-value depending on cluster configuration)
         if (result.HasMultiData)
         {
-            Assert.True(result.MultiValue.Count > 0);
+            Assert.NotEmpty(result.MultiValue);
             Assert.All(result.MultiValue.Values, r => Assert.Equal("Hello from node", r.ToString()));
         }
         else
@@ -1425,7 +1425,7 @@ public async Task FunctionListAsync_WithRouting_ReturnsLibrariesFromSpecifiedNod
         // Verify list returned (may be single or multi-value depending on cluster configuration)
         if (result.HasMultiData)
         {
-            Assert.True(result.MultiValue.Count > 0);
+            Assert.NotEmpty(result.MultiValue);
             // Verify each node has the library
             foreach (var (node, libraries) in result.MultiValue)
             {
@@ -1464,7 +1464,7 @@ public async Task FunctionStatsAsync_WithRouting_ReturnsPerNodeStats(GlideCluste
         // Verify stats returned (may be single or multi-value depending on cluster configuration)
         if (result.HasMultiData)
         {
-            Assert.True(result.MultiValue.Count > 0);
+            Assert.NotEmpty(result.MultiValue);
             // Verify each node has stats
             foreach (var (_, stats) in result.MultiValue)
             {
@@ -1610,7 +1610,7 @@ public async Task ClusterValue_MultiNodeResults_HandlesCorrectly(GlideClusterCli
         {
             Assert.False(loadResult.HasSingleData);
             Assert.NotNull(loadResult.MultiValue);
-            Assert.True(loadResult.MultiValue.Count > 0);
+            Assert.NotEmpty(loadResult.MultiValue);
 
             // Verify each node address is a key in the dictionary
             foreach (var (nodeAddress, libraryName) in loadResult.MultiValue)