Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/StackExchange.Redis/PhysicalBridge.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,18 @@ private WriteResult QueueOrFailMessage(Message message)
}

// Anything else goes in the bin - we're just not ready for you yet
message.Cancel();
Multiplexer.OnMessageFaulted(message, null);
message.Complete(null);
return WriteResult.NoConnectionAvailable;
return FailDueToNoConnection(message);
}

private WriteResult FailDueToNoConnection(Message message)
{
message.Cancel();
Multiplexer.OnMessageFaulted(message, null);
message.Complete(null);
// fail with the exception the caller would otherwise construct from the result, rather than Cancel():
// a cancelled box surfaces as TaskCanceledException, and although ExecuteAsyncImpl then tries to
// fault the task with the real exception, the box's continuation can get there first (#3283) - and a
// batch has no caller translating the result at all
var ex = ExceptionFactory.NoConnectionAvailable(Multiplexer, message, ServerEndPoint);
Multiplexer.OnMessageFaulted(message, ex);
message.SetExceptionAndComplete(ex, null);
return WriteResult.NoConnectionAvailable;
}

Expand Down Expand Up @@ -1368,9 +1369,10 @@ public bool HasPendingCallerFacingItems()

private WriteResult TimedOutBeforeWrite(Message message)
{
message.Cancel();
Multiplexer.OnMessageFaulted(message, null);
message.Complete(null);
// as FailDueToNoConnection: a real exception, not Cancel()
var ex = ExceptionFactory.Timeout(Multiplexer, null, message, ServerEndPoint, WriteResult.TimeoutBeforeWrite, this);
Multiplexer.OnMessageFaulted(message, ex);
message.SetExceptionAndComplete(ex, null);
return WriteResult.TimeoutBeforeWrite;
}

Expand Down
8 changes: 8 additions & 0 deletions src/StackExchange.Redis/PhysicalConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,14 @@ void AddData(string? lk, string? sk, string? v)
}

var ex = innerException is RedisException ? innerException : outerException;
if (ex is not RedisException)
{
// the detailed exception above is built once, for the bridge's *current* connection; a repeat
// report for the same connection, or one the bridge has already replaced, must still fail
// whatever is queued - and with a RedisConnectionException, not the raw transport fault, nor
// (when there is no exception at all, e.g. EOF) a TaskCanceledException nobody asked for (#3283)
ex = new RedisConnectionException(failureType, CommandFlags.None, failureType + " on " + ToString(), innerException);
}

nextMessage = Interlocked.Exchange(ref _awaitingToken, null);
if (nextMessage is not null)
Expand Down
78 changes: 78 additions & 0 deletions tests/StackExchange.Redis.Tests/ConnectionFailureExceptionTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
using System;
using System.IO;
using System.Threading.Tasks;
using Xunit;

namespace StackExchange.Redis.Tests;

/// <summary>
/// A command that fails because its connection failed must fail with a <see cref="RedisConnectionException"/>
/// carrying the transport fault as its inner exception: never the raw transport exception, and never a
/// <see cref="TaskCanceledException"/> when no caller token was involved (#3283).
/// </summary>
public class ConnectionFailureExceptionTests(ITestOutputHelper log)
{
private static (Message Message, Task<bool> Task) CreateTrackedPing()
{
var message = Message.Create(-1, CommandFlags.None, RedisCommand.PING);
var box = TaskResultBox<bool>.Create(out var tcs, null);
message.SetSource(box, ResultProcessor.DemandOK);
return (message, tcs.Task);
}

private static async Task<Exception> AwaitFailure(Task task)
{
var completed = await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(10)));
Assert.Same(task, completed);
return await Assert.ThrowsAnyAsync<Exception>(() => task);
}

[Theory]
[InlineData(false)] // EOF-style report: no exception at all (used to surface as TaskCanceledException)
[InlineData(true)] // transport fault (used to surface raw when the detailed report was not built)
public async Task DrainedMessagesFailWithRedisConnectionException(bool withInner)
{
// a dummy connection has no bridge, so it is never the bridge's *current* connection and the detailed
// first-report exception is not built: this is exactly the path that handed out raw/null exceptions
using var stream = new MemoryStream();
using var connection = PhysicalConnection.Dummy(stream);
var (message, task) = CreateTrackedPing();
connection.EnqueueInsideWriteLock(message, enforceMuxer: false);

var inner = withInner ? new IOException("simulated transport fault") : null;
connection.RecordConnectionFailed(ConnectionFailureType.SocketClosed, inner);

var ex = await AwaitFailure(task);
log.WriteLine(ex.ToString());
var conn = Assert.IsType<RedisConnectionException>(ex);
Assert.Equal(ConnectionFailureType.SocketClosed, conn.FailureType);
Assert.Same(inner, conn.InnerException);
}

[Fact]
public async Task NoConnectionFailsWithRedisConnectionExceptionNotCancellation()
{
using var server = new InProcessTestServer(log);
var config = server.GetClientConfig(withPubSub: false, defaultOnly: true);
config.BacklogPolicy = BacklogPolicy.FailFast;
config.AbortOnConnectFail = false;
config.AllowSimulateConnectionFailure = true;
await using var conn = await ConnectionMultiplexer.ConnectAsync(config);
var endpoint = conn.GetServerSnapshot()[0];
var bridge = endpoint.GetBridge(ConnectionType.Interactive)!;

conn.AllowConnect = false;
endpoint.SimulateConnectionFailure(SimulatedFailureType.All);
Assert.False(conn.IsConnected);

// write at the bridge, so we observe the message's own outcome rather than the caller's translation of
// the write result (which is what hid this for single commands - most of the time)
var (message, task) = CreateTrackedPing();
Assert.Equal(WriteResult.NoConnectionAvailable, await bridge.TryWriteAsync(message, isReplica: false));

var ex = await AwaitFailure(task);
log.WriteLine(ex.ToString());
Assert.IsType<RedisConnectionException>(ex);
Assert.False(task.IsCanceled);
}
}
Loading