Skip to content

Commit a6ff1d3

Browse files
committed
Fix review findings: logger forwarding, exception logging, degraded mode warning
- Log t.Exception (AggregateException) instead of t.Exception.InnerException to prevent null reference when handler throws AggregateException - Forward ILogger<RedisDatabase> from RedisClient via LoggerFactory so PubSub error logging works in production deployments - Add warning log when all pool connections are disconnected and a degraded-mode fallback is returned - Replace Task.Delay(200) with signaling (TaskCompletionSource) in PubSub handler-throws test to eliminate timing flakiness
1 parent 84ef764 commit a6ff1d3

File tree

4 files changed

+19
-3
lines changed

4 files changed

+19
-3
lines changed

src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisClient.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Copyright (c) Ugo Lattanzi. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
22

3+
using Microsoft.Extensions.Logging;
4+
35
using StackExchange.Redis.Extensions.Core.Abstractions;
46
using StackExchange.Redis.Extensions.Core.Configuration;
57

@@ -9,6 +11,7 @@ namespace StackExchange.Redis.Extensions.Core.Implementations;
911
public class RedisClient : IRedisClient
1012
{
1113
private readonly RedisConfiguration redisConfiguration;
14+
private readonly ILogger<RedisDatabase>? databaseLogger;
1215

1316
/// <summary>
1417
/// Initializes a new instance of the <see cref="RedisClient"/> class.
@@ -24,6 +27,7 @@ public RedisClient(
2427
ConnectionPoolManager = connectionPoolManager;
2528
Serializer = serializer;
2629
this.redisConfiguration = redisConfiguration;
30+
databaseLogger = redisConfiguration.LoggerFactory?.CreateLogger<RedisDatabase>();
2731
}
2832

2933
/// <inheritdoc/>
@@ -92,7 +96,8 @@ public IRedisDatabase GetDb(int dbNumber, string? keyPrefix = null)
9296
redisConfiguration.ServerEnumerationStrategy,
9397
dbNumber,
9498
redisConfiguration.MaxValueLength,
95-
keyPrefix);
99+
keyPrefix,
100+
databaseLogger);
96101
}
97102

98103
/// <inheritdoc/>

src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisConnectionPoolManager.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ var nextIdx
145145
throw new InvalidEnumArgumentException(nameof(redisConfiguration.ConnectionSelectionStrategy), (int)redisConfiguration.ConnectionSelectionStrategy, typeof(ConnectionSelectionStrategy));
146146
}
147147

148+
if (!connection.IsConnected() && logger.IsEnabled(LogLevel.Warning))
149+
logger.LogWarning("All Redis connections are disconnected. Using connection {HashCode} in degraded mode.", connection.Connection.GetHashCode().ToString(CultureInfo.InvariantCulture));
150+
148151
if (logger.IsEnabled(LogLevel.Debug))
149152
logger.LogDebug("Using connection {HashCode} with {OutStanding} outstanding!", connection.Connection.GetHashCode().ToString(CultureInfo.InvariantCulture), connection.TotalOutstanding().ToString(CultureInfo.InvariantCulture));
150153

src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.PubSub.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ void Handler(RedisChannel redisChannel, RedisValue value)
4343
});
4444

4545
task.ContinueWith(
46-
t => logger.LogError(t.Exception!.InnerException, "Error processing subscription message on channel {Channel}", (string?)redisChannel),
46+
t => logger.LogError(t.Exception, "Error processing subscription message on channel {Channel}", (string?)redisChannel),
4747
CancellationToken.None,
4848
TaskContinuationOptions.OnlyOnFaulted,
4949
TaskScheduler.Default);

tests/StackExchange.Redis.Extensions.Core.Tests/CacheClientTestBase.PubSub.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,30 @@ public async Task SubscribeAsync_HandlerThrows_ShouldNotCrashProcess_Async()
3939
{
4040
var channel = new RedisChannel(Guid.NewGuid().ToString(), RedisChannel.PatternMode.Literal);
4141
var secondMessageReceived = new TaskCompletionSource<bool>();
42+
var firstHandlerCompleted = new TaskCompletionSource<bool>();
4243
var callCount = 0;
4344

4445
await Sut.GetDefaultDatabase().SubscribeAsync<string>(channel, _ =>
4546
{
4647
var count = Interlocked.Increment(ref callCount);
4748

4849
if (count == 1)
50+
{
51+
firstHandlerCompleted.TrySetResult(true);
4952
throw new InvalidOperationException("Simulated handler failure");
53+
}
5054

5155
secondMessageReceived.TrySetResult(true);
5256
return Task.CompletedTask;
5357
});
5458

5559
// First message — handler throws, should be logged not crash
5660
await Sut.GetDefaultDatabase().PublishAsync(channel, "msg1");
57-
await Task.Delay(200);
61+
62+
// Wait for first handler to complete before sending second message
63+
using var cts1 = new CancellationTokenSource(TimeSpan.FromSeconds(5));
64+
cts1.Token.Register(() => firstHandlerCompleted.TrySetCanceled());
65+
await firstHandlerCompleted.Task;
5866

5967
// Second message — handler should still be active
6068
await Sut.GetDefaultDatabase().PublishAsync(channel, "msg2");

0 commit comments

Comments
 (0)