Fall through when the tray has gone

src/DiffEngine.Tests/PendingFilesFallbackTests.cs

@@ -0,0 +1,83 @@
+// DiffEngineTray is the obsolete public shim, but its IsRunning is still where the tray check
+// lives, and this test has to move it.
+#pragma warning disable CS0618
+
+/// <summary>
+/// Where a pending file goes when the tray check is stale.
+/// <para>
+/// DiffEngineTray.IsRunning is read once, when the type initialises. A tray that exits while a
+/// long lived host keeps running leaves that answer saying a tray is there, so the piper send went
+/// to a port nobody was listening on - and, returning nothing, was swallowed into a trace line.
+/// The move or delete was then pending in nothing at all: no fallback to the queue owner, no
+/// LaunchDelete.
+/// </para>
+/// </summary>
+[NotInParallel]
+public class PendingFilesFallbackTests
+{
+    [Test]
+    public async Task ADeadPiperFallsThroughToTheQueueOwner()
+    {
+        await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue();
+        using var server = bound!;
+        using var cancel = new CancelSource();
+
+        var heard = new ConcurrentBag<string>();
+        var listening = server.Listen(
+            _ =>
+            {
+                heard.Add($"{_.Verb}:{_.Key}");
+                return ViewerResponse.Success();
+            },
+            cancel.Token);
+
+        var previousPort = PiperClient.Port;
+        var previousViewerPort = Environment.GetEnvironmentVariable(ViewerClient.PortVariable);
+        var previousRunning = DiffEngineTray.IsRunning;
+        try
+        {
+            // A tray that says it is running, on a port nothing is listening on
+            PiperClient.Port = DeadPort();
+            DiffEngineTray.IsRunning = true;
+            Environment.SetEnvironmentVariable(ViewerClient.PortVariable, server.Port.ToString());
+
+            PendingFiles.AddMove("temp.txt", "target.txt", null, null, false, null);
+            await PendingFilesAddDelete("gone.txt");
+
+            await Assert.That(heard).Contains(_ => _.StartsWith("Move:", StringComparison.Ordinal));
+            await Assert.That(heard).Contains(_ => _.StartsWith("Delete:", StringComparison.Ordinal));
+        }
+        finally
+        {
+            PiperClient.Port = previousPort;
+            DiffEngineTray.IsRunning = previousRunning;
+            Environment.SetEnvironmentVariable(ViewerClient.PortVariable, previousViewerPort);
+            await cancel.CancelAsync();
+            try
+            {
+                await listening.WaitAsync(TimeSpan.FromSeconds(5));
+            }
+            catch (Exception exception)
+                when (exception is OperationCanceledException or TimeoutException)
+            {
+            }
+        }
+    }
+
+    // The delete path launches a viewer when nothing answers, so it is only safe to exercise with
+    // an owner bound - which is the point of the test.
+    static Task PendingFilesAddDelete(string file)
+    {
+        PendingFiles.AddDelete(file);
+        return Task.CompletedTask;
+    }
+
+    static int DeadPort()
+    {
+        var listener = new TcpListener(IPAddress.Loopback, 0);
+        listener.Start();
+        var port = ((IPEndPoint) listener.LocalEndpoint).Port;
+        listener.Stop();
+        return port;
+    }
+}

src/DiffEngine/Tray/PendingFiles.cs

@@ -24,14 +24,21 @@ namespace DiffEngine;
 /// process's life, and its moves and deletes arrive here instead — which a tray that owns the
 /// queue answers, so they end up tracked either way.
 /// </para>
+/// <para>
+/// The mirror of that case is a tray that exits while a long lived host keeps running, and it is
+/// why the piper send is asked whether it connected rather than told to get on with it. The cached
+/// answer still says a tray is there, so every later move and delete went to a port nobody was
+/// listening on and was swallowed into a trace line: pending in nothing, with no fallback and no
+/// LaunchDelete. A refused piper send now falls through to the same branch as no tray at all.
+/// </para>
 /// </summary>
 static class PendingFiles
 {
     public static void AddDelete(string file)
     {
-        if (DiffEngineTray.IsRunning)
+        if (DiffEngineTray.IsRunning &&
+            PiperClient.SendDelete(file))
         {
-            PiperClient.SendDelete(file);
             return;
         }
 
@@ -45,9 +52,9 @@ public static void AddDelete(string file)
 
     public static async Task AddDeleteAsync(string file, Cancel cancel)
     {
-        if (DiffEngineTray.IsRunning)
+        if (DiffEngineTray.IsRunning &&
+            await PiperClient.SendDeleteAsync(file, cancel))
         {
-            await PiperClient.SendDeleteAsync(file, cancel);
             return;
         }
 
@@ -67,9 +74,9 @@ public static void AddMove(
         bool canKill,
         int? processId)
     {
-        if (DiffEngineTray.IsRunning)
+        if (DiffEngineTray.IsRunning &&
+            PiperClient.SendMove(tempFile, targetFile, exe, arguments, canKill, processId))
         {
-            PiperClient.SendMove(tempFile, targetFile, exe, arguments, canKill, processId);
             return;
         }
 
@@ -85,9 +92,9 @@ public static async Task AddMoveAsync(
         int? processId,
         Cancel cancel)
     {
-        if (DiffEngineTray.IsRunning)
+        if (DiffEngineTray.IsRunning &&
+            await PiperClient.SendMoveAsync(tempFile, targetFile, exe, arguments, canKill, processId, cancel))
         {
-            await PiperClient.SendMoveAsync(tempFile, targetFile, exe, arguments, canKill, processId, cancel);
             return;
         }
 

src/DiffEngine/Tray/PiperClient.cs

@@ -2,10 +2,10 @@ static class PiperClient
 {
     public static int Port = 3492;
 
-    public static void SendDelete(string file) =>
+    public static bool SendDelete(string file) =>
         Send(BuildDeletePayload(file));
 
-    public static Task SendDeleteAsync(
+    public static Task<bool> SendDeleteAsync(
         string file,
         Cancel cancel = default)
     {
@@ -22,7 +22,7 @@ static string BuildDeletePayload(string file) =>
 
           """;
 
-    public static void SendMove(
+    public static bool SendMove(
         string tempFile,
         string targetFile,
         string? exe,
@@ -31,7 +31,7 @@ public static void SendMove(
         int? processId) =>
         Send(BuildMovePayload(tempFile, targetFile, exe, arguments, canKill, processId));
 
-    public static Task SendMoveAsync(
+    public static Task<bool> SendMoveAsync(
         string tempFile,
         string targetFile,
         string? exe,
@@ -79,28 +79,37 @@ public static string BuildMovePayload(string tempFile, string targetFile, string
         return builder.ToString();
     }
 
-    static void Send(string payload)
+    /// <summary>
+    /// True when the tray took it. False is not fatal on its own - the payload is traced either
+    /// way - but it is what lets the caller send the pending file somewhere else instead of
+    /// dropping it, which is what happened when this returned nothing.
+    /// </summary>
+    static bool Send(string payload)
     {
         try
         {
             InnerSend(payload);
+            return true;
         }
         catch (Exception exception)
         {
             HandleSendException(payload, exception);
+            return false;
         }
     }
 
-    static async Task SendAsync(string payload, Cancel cancel)
+    static async Task<bool> SendAsync(string payload, Cancel cancel)
     {
         try
         {
             await InnerSendAsync(payload, cancel);
+            return true;
         }
         // Let cancellation surface to the caller; only genuine send failures are swallowed.
         catch (Exception exception) when (exception is not OperationCanceledException)
         {
             HandleSendException(payload, exception);
+            return false;
         }
     }