From 28a1a53bb6a087d94e5bec8add705381c16b7aa4 Mon Sep 17 00:00:00 2001 From: mgravell Date: Tue, 13 Oct 2026 10:31:25 +0100 Subject: [PATCH 1/2] Tolerate refused CLUSTER SLOTS/NODES during topology updates (#3274) Proxied deployments (Azure Managed Redis / Redis Enterprise) can identify as a cluster while refusing CLUSTER SLOTS ("ERR command is not allowed"; Enterprise exposes SLOTS only under the OSS cluster API). That refusal surfaced as a logged *error* from GetEndpointsFromClusterNodes at connect and on every reconfigure, including the periodic topology refresh. A refusal is a posture, not a fault: catch RedisServerException per task - SLOTS refused falls back to the NODES-only path (as the method's own comment always intended), NODES refused means no topology view at all, so return null; both are logged at Information (EventIds 128/129). Tests: ClusterCommandsRefusedUnitTests drives the in-process test server with SLOTS (and SLOTS+NODES) refused, asserting discovery still works and no error is logged; CapturingLogger promoted from ConnectFailureRefreshTests to a shared test helper. --- .../ConnectionMultiplexer.cs | 29 ++++++- src/StackExchange.Redis/LoggerExtensions.cs | 12 +++ .../ClusterCommandsRefusedUnitTests.cs | 75 +++++++++++++++++++ .../ConnectFailureRefreshTests.cs | 30 -------- .../Helpers/CapturingLogger.cs | 39 ++++++++++ 5 files changed, 153 insertions(+), 32 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs create mode 100644 tests/StackExchange.Redis.Tests/Helpers/CapturingLogger.cs diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs index b2d38cdc1..3473629d6 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -2162,8 +2162,33 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => var nodesTask = ExecuteAsyncImpl( RedisServer.GetClusterNodesMessage(CommandFlags.None), ResultProcessor.ClusterNodes, null, server); - var slots = await slotsTask.ForAwait(); - var clusterConfig = await nodesTask.ForAwait(); + ClusterSlotsResult? slots; + try + { + slots = await slotsTask.ForAwait(); + } + catch (RedisServerException ex) + { + // the server *refused* the command - a posture, not a fault: proxied deployments + // commonly answer NODES while blocking SLOTS (Redis Enterprise exposes SLOTS only + // under the OSS cluster API, for example; see #3274). Fall back to the NODES-only + // path, exactly as for a pre-4.0 server that has no SLOTS view to give + log?.LogInformationClusterSlotsRefused(new(server), ex.Message); + slots = null; + } + + ClusterConfiguration? clusterConfig; + try + { + clusterConfig = await nodesTask.ForAwait(); + } + catch (RedisServerException ex) + { + // likewise; a server refusing NODES as well exposes no topology at all, however it + // announced itself (a proxy's HELLO can say "cluster" while CLUSTER stays blocked) + log?.LogInformationClusterNodesRefused(new(server), ex.Message); + return null; + } if (clusterConfig is null) { return null; diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index 6617f3df6..b8abcea8f 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -328,6 +328,18 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) Message = "Encountered error while updating cluster config: {ErrorMessage}")] internal static partial void LogErrorEncounteredErrorWhileUpdatingClusterConfig(this ILogger logger, Exception exception, string errorMessage); + [LoggerMessage( + Level = LogLevel.Information, + EventId = 128, + Message = "{Server}: CLUSTER SLOTS was refused ({ErrorMessage}); continuing with CLUSTER NODES alone")] + internal static partial void LogInformationClusterSlotsRefused(this ILogger logger, ServerEndPointLogValue server, string errorMessage); + + [LoggerMessage( + Level = LogLevel.Information, + EventId = 129, + Message = "{Server}: CLUSTER NODES was refused ({ErrorMessage}); no cluster topology is available")] + internal static partial void LogInformationClusterNodesRefused(this ILogger logger, ServerEndPointLogValue server, string errorMessage); + [LoggerMessage( Level = LogLevel.Information, EventId = 111, diff --git a/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs b/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs new file mode 100644 index 000000000..1f6e34de5 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs @@ -0,0 +1,75 @@ +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using StackExchange.Redis.Server; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Proxied deployments can refuse some or all CLUSTER subcommands while still identifying as a cluster +/// (Redis Enterprise exposes CLUSTER SLOTS only under the OSS cluster API, and its HELLO/INFO +/// can say "cluster" while CLUSTER stays blocked entirely). A refusal is a posture, not a fault: the +/// topology pass must fall back to whatever view it can get, quietly - not log a server error on every +/// reconfigure. See #3274. +/// +[RunPerProtocol] +public class ClusterCommandsRefusedUnitTests(ITestOutputHelper log) +{ + private sealed class RefusingServer(ITestOutputHelper log, bool refuseNodes) : InProcessTestServer(log) + { + protected override TypedRedisValue ClusterSlots(RedisClient client, in RedisRequest request) + => TypedRedisValue.Error("ERR command is not allowed"); + + protected override TypedRedisValue ClusterNodes(RedisClient client, in RedisRequest request) + => refuseNodes ? TypedRedisValue.Error("ERR command is not allowed") : base.ClusterNodes(client, request); + } + + [Fact] + public async Task SlotsRefusedFallsBackToNodes() + { + using var server = new RefusingServer(log, refuseNodes: false) { ServerType = ServerType.Cluster }; + var a = server.DefaultEndPoint; + var b = server.AddEmptyNode(); + + var config = server.GetClientConfig(defaultOnly: true); // only connect to a initially + var logger = new CapturingLogger(); + config.LoggerFactory = logger; + await using var conn = await ConnectionMultiplexer.ConnectAsync(config); + + // discovery still happened, from the NODES view alone + var endpoints = conn.GetEndPoints(); + Assert.Contains(a, endpoints); + Assert.Contains(b, endpoints); + + log.WriteLine(logger.All); + Assert.Empty(logger.Matching("Encountered error while updating cluster config")); + Assert.NotEmpty(logger.Matching("CLUSTER SLOTS was refused")); + + await conn.GetDatabase().PingAsync(); + } + + [Fact] + public async Task AllClusterCommandsRefusedIsQuiet() + { + // the server still *identifies* as a cluster (HELLO "mode", INFO "redis_mode"), so the client + // interrogates it; both commands being refused must leave a working connection, not a logged error + using var server = new RefusingServer(log, refuseNodes: true) { ServerType = ServerType.Cluster }; + + var config = server.GetClientConfig(defaultOnly: true); + var logger = new CapturingLogger(); + config.LoggerFactory = logger; + await using var conn = await ConnectionMultiplexer.ConnectAsync(config); + + log.WriteLine(logger.All); + Assert.Empty(logger.Matching("Encountered error while updating cluster config")); + Assert.NotEmpty(logger.Matching("CLUSTER NODES was refused")); + + await conn.GetDatabase().PingAsync(); + + // ...and a later explicit reconfigure stays quiet too + Assert.True(await conn.ReconfigureAsync("test")); + Assert.Empty(logger.Matching("Encountered error while updating cluster config")); + } +} diff --git a/tests/StackExchange.Redis.Tests/ConnectFailureRefreshTests.cs b/tests/StackExchange.Redis.Tests/ConnectFailureRefreshTests.cs index 48ae8b973..0bc6beb2b 100644 --- a/tests/StackExchange.Redis.Tests/ConnectFailureRefreshTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectFailureRefreshTests.cs @@ -145,34 +145,4 @@ public async Task TheReadIsRateLimitedRatherThanOncePerFailure() Assert.True(refreshes <= permitted, $"expected at most {permitted} rate-limited reads, but saw {refreshes}"); } } - - private sealed class CapturingLogger : ILoggerFactory, ILogger - { - private readonly List _messages = []; - - public ILogger CreateLogger(string categoryName) => this; - - public void AddProvider(ILoggerProvider provider) { } - - public IDisposable? BeginScope(TState state) where TState : notnull => null; - - public bool IsEnabled(LogLevel logLevel) => true; - - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) - { - lock (_messages) _messages.Add(formatter(state, exception)); - } - - public List Matching(string fragment) - { - lock (_messages) return _messages.FindAll(x => x.Contains(fragment, StringComparison.Ordinal)); - } - - public string All - { - get { lock (_messages) return string.Join("\n", _messages); } - } - - public void Dispose() { } - } } diff --git a/tests/StackExchange.Redis.Tests/Helpers/CapturingLogger.cs b/tests/StackExchange.Redis.Tests/Helpers/CapturingLogger.cs new file mode 100644 index 000000000..eec70187e --- /dev/null +++ b/tests/StackExchange.Redis.Tests/Helpers/CapturingLogger.cs @@ -0,0 +1,39 @@ +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Logging; + +namespace StackExchange.Redis.Tests; + +/// +/// An that records every formatted message, so a test can assert on what +/// was (or was not) logged. +/// +internal sealed class CapturingLogger : ILoggerFactory, ILogger +{ + private readonly List _messages = []; + + public ILogger CreateLogger(string categoryName) => this; + + public void AddProvider(ILoggerProvider provider) { } + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + lock (_messages) _messages.Add(formatter(state, exception)); + } + + public List Matching(string fragment) + { + lock (_messages) return _messages.FindAll(x => x.Contains(fragment, StringComparison.Ordinal)); + } + + public string All + { + get { lock (_messages) return string.Join("\n", _messages); } + } + + public void Dispose() { } +} From 79cbfd8f29f79b13924b03db412b8a2047885713 Mon Sep 17 00:00:00 2001 From: mgravell Date: Tue, 13 Oct 2026 10:38:46 +0100 Subject: [PATCH 2/2] Observe both topology tasks, so an abandoned one never faults unobserved SLOTS and NODES are issued as a pair; if one await throws anything other than a server refusal, the other task is abandoned mid-flight - and both ride the same connection, so the orphan is likely to fault too. Attach ObserveErrors at creation so it can never surface as an UnobservedTaskException in the host. (Pre-existing gap: the bare sequential awaits had the same shape.) --- src/StackExchange.Redis/ConnectionMultiplexer.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs index 3473629d6..970561773 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -2157,10 +2157,13 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => // node including those serving nothing. Asked as a pair for symmetry - trusting the topology // cached from autoconfigure here would mean acting on possibly-stale data while deliberately // re-reading the other half + // ObserveErrors: if either await below throws, the *other* task is abandoned mid-flight, + // and both ride the same connection - a faulted orphan must never surface as an + // UnobservedTaskException in the host var slotsTask = ExecuteAsyncImpl( - RedisServer.GetClusterSlotsMessage(CommandFlags.None), ResultProcessor.ClusterSlots, null, server); + RedisServer.GetClusterSlotsMessage(CommandFlags.None), ResultProcessor.ClusterSlots, null, server).ObserveErrors(); var nodesTask = ExecuteAsyncImpl( - RedisServer.GetClusterNodesMessage(CommandFlags.None), ResultProcessor.ClusterNodes, null, server); + RedisServer.GetClusterNodesMessage(CommandFlags.None), ResultProcessor.ClusterNodes, null, server).ObserveErrors(); ClusterSlotsResult? slots; try