diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index 7ed60f8e..5b4f4d8e 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -733,6 +733,64 @@ public async Task AnUnresponsiveOwnerTimesOutRatherThanHanging() listener.Stop(); } } + /// + /// An owner that answers with an error is not an absent one. Collapsing the two into false + /// meant a refused inline was read as "nobody is there", so a second viewer was launched, it + /// could not bind the port, and the snapshot was reported as Queued while being held by + /// nothing at all. + /// + [Test] + public async Task ARefusedExchangeIsToldApartFromAnAbsentOwner() + { + await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue(); + using var server = bound!; + using var cancel = new CancelSource(); + var listening = server.Listen(_ => ViewerResponse.Error("no thanks"), cancel.Token); + + try + { + var refused = await ViewerClient.SendAsync(new(ViewerVerb.List), default, server.Port); + + await Assert.That(refused).IsEqualTo(SendOutcome.Refused); + } + finally + { + await cancel.CancelAsync(); + await Wait(listening); + } + } + + [Test] + public async Task AnAcceptedExchangeReportsAccepted() + { + await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue(); + using var server = bound!; + using var cancel = new CancelSource(); + var listening = server.Listen(_ => ViewerResponse.Success("fine"), cancel.Token); + + try + { + await Assert.That(await ViewerClient.SendAsync(new(ViewerVerb.List), default, server.Port)) + .IsEqualTo(SendOutcome.Accepted); + } + finally + { + await cancel.CancelAsync(); + await Wait(listening); + } + } + + [Test] + public async Task AnAbsentOwnerReportsNoOwner() + { + ViewerServer.TryBind(0, out var server); + var port = server!.Port; + server.Dispose(); + + await Assert.That(await ViewerClient.SendAsync(new(ViewerVerb.List), default, port, TimeSpan.FromSeconds(2))) + .IsEqualTo(SendOutcome.NoOwner); + } + [Test] public async Task AnAbsentOwnerIsNotAnError() { diff --git a/src/DiffEngine/DiffRunner_Inline.cs b/src/DiffEngine/DiffRunner_Inline.cs index c01cef60..6c247b25 100644 --- a/src/DiffEngine/DiffRunner_Inline.cs +++ b/src/DiffEngine/DiffRunner_Inline.cs @@ -14,7 +14,9 @@ public enum InlineResult Disabled, /// - /// No DiffEngineViewer could be resolved. Callers that want a fallback should use it here. + /// Nothing has the snapshot. No DiffEngineViewer could be resolved, or the owner of the queue + /// declined the payload - which is the same thing from the caller's side, since in both cases + /// the snapshot is pending nowhere. Callers that want a fallback should use it here. /// NoViewerFound } @@ -61,11 +63,22 @@ public static async Task AddInlineAsync(InlinePatch patch, Cancel // Onto the payload rather than onto the patch, which belongs to the caller and may be // held or sent again var payload = InlinePatchFile.Build(patch, patch.Framework ?? RuntimeMoniker.Current); - if (await ViewerClient.TrySendAsync(new(ViewerVerb.Inline, Body: payload), cancel)) + var outcome = await ViewerClient.SendAsync(new(ViewerVerb.Inline, Body: payload), cancel); + if (outcome == SendOutcome.Accepted) { return InlineResult.Queued; } + if (outcome == SendOutcome.Refused) + { + // An owner that is there and said no - an older one that does not understand the + // payload, or a handler that threw. Launching a second viewer cannot change that + // answer, and it would bind nothing, so reporting Queued would say the snapshot is + // somewhere when it is nowhere. Report it as no viewer, which is the answer that + // makes the caller stage the files instead + return InlineResult.NoViewerFound; + } + var launched = await ViewerLauncher.LaunchAsync(patch, payload, cancel); return launched ? InlineResult.Queued : InlineResult.NoViewerFound; } diff --git a/src/DiffEngine/Protocol/ViewerClient.cs b/src/DiffEngine/Protocol/ViewerClient.cs index 9f36941a..3d0aaa3a 100644 --- a/src/DiffEngine/Protocol/ViewerClient.cs +++ b/src/DiffEngine/Protocol/ViewerClient.cs @@ -5,6 +5,31 @@ namespace DiffEngine; /// caller turns into a launch (DiffEngine), "nothing pending" (the tray), or "the owner has gone" /// (an attached viewer). /// +/// +/// What came back from an exchange with the queue owner. Three outcomes rather than two, because +/// "nobody is there" and "the owner said no" call for opposite responses: the first is fixed by +/// launching a viewer, and the second is not. +/// +enum SendOutcome +{ + /// + /// Nobody answered. No owner, or one present but unresponsive - the caller cannot tell, and + /// for its purposes they are the same. + /// + NoOwner, + + /// + /// The owner answered and took it. + /// + Accepted, + + /// + /// The owner answered and declined it: a version it does not understand, or a handler that + /// threw. Launching another viewer will not change that answer. + /// + Refused +} + static class ViewerClient { public const int DefaultPort = 3493; @@ -114,6 +139,18 @@ public static bool TrySend( /// /// public static async Task TrySendAsync( + ViewerMessage message, + Cancel cancel, + int? port = null, + TimeSpan? wait = null) => + await SendAsync(message, cancel, port, wait) == SendOutcome.Accepted; + + /// + /// As , but says which of the two failures happened. A caller that + /// would launch a viewer on absence needs that: launching one because the owner refused the + /// payload leaves two processes and still no snapshot. + /// + public static async Task SendAsync( ViewerMessage message, Cancel cancel, int? port = null, @@ -154,8 +191,12 @@ public static async Task TrySendAsync( #else var text = await reader.ReadToEndAsync(); #endif - return ViewerResponse.TryParse(text, out var response) && - response.Ok; + if (!ViewerResponse.TryParse(text, out var response)) + { + return SendOutcome.NoOwner; + } + + return response.Ok ? SendOutcome.Accepted : SendOutcome.Refused; } // The deadline, rather than the caller cancelling. Whatever the abort surfaced as - a // cancellation, a closed socket, a torn down stream - the owner is present but not @@ -169,13 +210,13 @@ public static async Task TrySendAsync( Trace.WriteLine( $"Timed out after {timeToWait} waiting for the inline queue owner on port {endpointPort}. " + $"Verb: {message.Verb}. The owner is present but unresponsive. {exception.GetType().Name}"); - return false; + return SendOutcome.NoOwner; } // Cancellation is the caller's business; a missing owner is not. catch (Exception exception) when (exception is not OperationCanceledException && Ignorable(exception)) { - return false; + return SendOutcome.NoOwner; } }