[dotnet-watch] Notify DCP of terminated session when process exits on its own (#50992)

Author: marcpopMSFT

src/BuiltInTools/AspireService/AspireServerService.cs

@@ -123,6 +123,7 @@ public List<KeyValuePair<string, string>> GetServerConnectionEnvironment()
             new(DebugSessionServerCertEnvVar, _certificateEncodedBytes),
         ];
 
+    /// <exception cref="OperationCanceledException"/>
     public ValueTask NotifySessionEndedAsync(string dcpId, string sessionId, int processId, int? exitCode, CancellationToken cancelationToken)
         => SendNotificationAsync(
             new SessionTerminatedNotification()
@@ -136,6 +137,7 @@ public ValueTask NotifySessionEndedAsync(string dcpId, string sessionId, int pro
             sessionId,
             cancelationToken);
 
+    /// <exception cref="OperationCanceledException"/>
     public ValueTask NotifySessionStartedAsync(string dcpId, string sessionId, int processId, CancellationToken cancelationToken)
         => SendNotificationAsync(
             new ProcessRestartedNotification()
@@ -148,6 +150,7 @@ public ValueTask NotifySessionStartedAsync(string dcpId, string sessionId, int p
             sessionId,
             cancelationToken);
 
+    /// <exception cref="OperationCanceledException"/>
     public ValueTask NotifyLogMessageAsync(string dcpId, string sessionId, bool isStdErr, string data, CancellationToken cancelationToken)
         => SendNotificationAsync(
             new ServiceLogsNotification()
@@ -161,23 +164,28 @@ public ValueTask NotifyLogMessageAsync(string dcpId, string sessionId, bool isSt
             sessionId,
             cancelationToken);
 
-    private async ValueTask SendNotificationAsync<TNotification>(TNotification notification, string dcpId, string sessionId, CancellationToken cancelationToken)
+    /// <exception cref="OperationCanceledException"/>
+    private async ValueTask SendNotificationAsync<TNotification>(TNotification notification, string dcpId, string sessionId, CancellationToken cancellationToken)
         where TNotification : SessionNotification
     {
         try
         {
-            Log($"[#{sessionId}] Sending '{notification.NotificationType}'");
+            Log($"[#{sessionId}] Sending '{notification.NotificationType}': {notification}");
             var jsonSerialized = JsonSerializer.SerializeToUtf8Bytes(notification, JsonSerializerOptions);
-            await SendMessageAsync(dcpId, jsonSerialized, cancelationToken);
-        }
-        catch (Exception e) when (e is not OperationCanceledException && LogAndPropagate(e))
-        {
-        }
+            var success = await SendMessageAsync(dcpId, jsonSerialized, cancellationToken);
 
-        bool LogAndPropagate(Exception e)
+            if (!success)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                Log($"[#{sessionId}] Failed to send message: Connection not found (dcpId='{dcpId}').");
+            }
+        }
+        catch (Exception e) when (e is not OperationCanceledException)
         {
-            Log($"[#{sessionId}] Sending '{notification.NotificationType}' failed: {e.Message}");
-            return false;
+            if (!cancellationToken.IsCancellationRequested)
+            {
+                Log($"[#{sessionId}] Failed to send message: {e.Message}");
+            }
         }
     }
 
@@ -373,15 +381,13 @@ private async Task WriteResponseTextAsync(HttpResponse response, Exception ex, b
         }
     }
 
-    private async Task SendMessageAsync(string dcpId, byte[] messageBytes, CancellationToken cancellationToken)
+    private async ValueTask<bool> SendMessageAsync(string dcpId, byte[] messageBytes, CancellationToken cancellationToken)
     {
         // Find the connection for the passed in dcpId
         WebSocketConnection? connection = _socketConnectionManager.GetSocketConnection(dcpId);
         if (connection is null)
         {
-            // Most likely the connection has already gone away
-            Log($"Send message failure: Connection with the following dcpId was not found {dcpId}");
-            return;
+            return false;
         }
 
         var success = false;
@@ -405,6 +411,8 @@ private async Task SendMessageAsync(string dcpId, byte[] messageBytes, Cancellat
 
             _webSocketAccess.Release();
         }
+
+        return success;
     }
 
     private async ValueTask HandleStopSessionRequestAsync(HttpContext context, string sessionId)

src/BuiltInTools/AspireService/Models/SessionChangeNotification.cs

@@ -56,6 +56,9 @@ internal sealed class SessionTerminatedNotification : SessionNotification
     [Required]
     [JsonPropertyName("exit_code")]
     public required int? ExitCode { get; init; }
+
+    public override string ToString()
+        => $"pid={Pid}, exit_code={ExitCode}";
 }
 
 /// <summary>
@@ -70,6 +73,9 @@ internal sealed class ProcessRestartedNotification : SessionNotification
     [Required]
     [JsonPropertyName("pid")]
     public required int PID { get; init; }
+
+    public override string ToString()
+        => $"pid={PID}";
 }
 
 /// <summary>
@@ -91,4 +97,7 @@ internal sealed class ServiceLogsNotification : SessionNotification
     [Required]
     [JsonPropertyName("log_message")]
     public required string LogMessage { get; init; }
+
+    public override string ToString()
+        => $"log_message='{LogMessage}', is_std_err={IsStdErr}";
 }

src/BuiltInTools/HotReloadClient/DefaultHotReloadClient.cs

@@ -35,9 +35,14 @@ public override void Dispose()
 
         private void DisposePipe()
         {
-            Logger.LogDebug("Disposing agent communication pipe");
-            _pipe?.Dispose();
-            _pipe = null;
+            if (_pipe != null)
+            {
+                Logger.LogDebug("Disposing agent communication pipe");
+
+                // Dispose the pipe but do not set it to null, so that any in-progress 
+                // operations throw the appropriate exception type.
+                _pipe.Dispose();
+            }
         }
 
         // for testing
@@ -101,8 +106,7 @@ private void RequireReadyForUpdates()
             // should only be called after connection has been created:
             _ = GetCapabilitiesTask();
 
-            if (_pipe == null)
-                throw new InvalidOperationException("Pipe has been disposed.");
+            Debug.Assert(_pipe != null);
         }
 
         public override void ConfigureLaunchEnvironment(IDictionary<string, string> environmentBuilder)
@@ -152,7 +156,13 @@ public override async Task<ApplyStatus> ApplyManagedCodeUpdatesAsync(ImmutableAr
             {
                 if (!success)
                 {
-                    Logger.LogWarning("Further changes won't be applied to this process.");
+                    // Don't report a warning when cancelled. The process has terminated or the host is shutting down in that case.
+                    // Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
+                    if (!cancellationToken.IsCancellationRequested)
+                    {
+                        Logger.LogWarning("Further changes won't be applied to this process.");
+                    }
+
                     _managedCodeUpdateFailedOrCancelled = true;
                     DisposePipe();
                 }
@@ -216,7 +226,7 @@ public async override Task<ApplyStatus> ApplyStaticAssetUpdatesAsync(ImmutableAr
         private ValueTask<bool> SendAndReceiveUpdateAsync<TRequest>(TRequest request, bool isProcessSuspended, CancellationToken cancellationToken)
             where TRequest : IUpdateRequest
         {
-            // Should not be disposed:
+            // Should not initialized:
             Debug.Assert(_pipe != null);
 
             return SendAndReceiveUpdateAsync(
@@ -241,8 +251,10 @@ async ValueTask<bool> SendAndReceiveAsync(int batchId, CancellationToken cancell
 
                     Logger.LogDebug("Update batch #{UpdateId} failed.", batchId);
                 }
-                catch (Exception e) when (e is not OperationCanceledException || isProcessSuspended)
+                catch (Exception e)
                 {
+                    // Don't report an error when cancelled. The process has terminated or the host is shutting down in that case.
+                    // Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
                     if (cancellationToken.IsCancellationRequested)
                     {
                         Logger.LogDebug("Update batch #{UpdateId} canceled.", batchId);
@@ -267,7 +279,7 @@ async ValueTask WriteRequestAsync(CancellationToken cancellationToken)
 
         private async ValueTask<bool> ReceiveUpdateResponseAsync(CancellationToken cancellationToken)
         {
-            // Should not be disposed:
+            // Should be initialized:
             Debug.Assert(_pipe != null);
 
             var (success, log) = await UpdateResponse.ReadAsync(_pipe, cancellationToken);
@@ -296,10 +308,12 @@ public override async Task InitialUpdatesAppliedAsync(CancellationToken cancella
             }
             catch (Exception e) when (e is not OperationCanceledException)
             {
-                // pipe might throw another exception when forcibly closed on process termination:
+                // Pipe might throw another exception when forcibly closed on process termination.
+                // Don't report an error when cancelled. The process has terminated or the host is shutting down in that case.
+                // Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
                 if (!cancellationToken.IsCancellationRequested)
                 {
-                    Logger.LogError("Failed to send InitialUpdatesCompleted: {Message}", e.Message);
+                    Logger.LogError("Failed to send {RequestType}: {Message}", nameof(RequestType.InitialUpdatesCompleted), e.Message);
                 }
             }
         }

src/BuiltInTools/HotReloadClient/HotReloadClient.cs

@@ -41,23 +41,42 @@ internal Task PendingUpdates
     /// <summary>
     /// Initiates connection with the agent in the target process.
     /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract void InitiateConnection(CancellationToken cancellationToken);
 
     /// <summary>
     /// Waits until the connection with the agent is established.
     /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract Task WaitForConnectionEstablishedAsync(CancellationToken cancellationToken);
 
+    /// <summary>
+    /// Returns update capabilities of the target process.
+    /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract Task<ImmutableArray<string>> GetUpdateCapabilitiesAsync(CancellationToken cancellationToken);
 
+    /// <summary>
+    /// Applies managed code updates to the target process.
+    /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract Task<ApplyStatus> ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Applies static asset updates to the target process.
+    /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract Task<ApplyStatus> ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStaticAssetUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken);
 
     /// <summary>
     /// Notifies the agent that the initial set of updates has been applied and the user code in the process can start executing.
     /// </summary>
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public abstract Task InitialUpdatesAppliedAsync(CancellationToken cancellationToken);
 
+    /// <summary>
+    /// Disposes the client. Can occur unexpectedly whenever the process exits.
+    /// </summary>
     public abstract void Dispose();
 
     public static void ReportLogEntry(ILogger logger, string message, AgentMessageSeverity severity)
@@ -72,7 +91,7 @@ public static void ReportLogEntry(ILogger logger, string message, AgentMessageSe
         logger.Log(level, message);
     }
 
-    public async Task<IReadOnlyList<HotReloadManagedCodeUpdate>> FilterApplicableUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, CancellationToken cancellationToken)
+    protected async Task<IReadOnlyList<HotReloadManagedCodeUpdate>> FilterApplicableUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, CancellationToken cancellationToken)
     {
         var availableCapabilities = await GetUpdateCapabilitiesAsync(cancellationToken);
         var applicableUpdates = new List<HotReloadManagedCodeUpdate>();

src/BuiltInTools/HotReloadClient/HotReloadClients.cs

@@ -23,6 +23,9 @@ public HotReloadClients(HotReloadClient client, AbstractBrowserRefreshServer? br
     {
     }
 
+    /// <summary>
+    /// Disposes all clients. Can occur unexpectedly whenever the process exits.
+    /// </summary>
     public void Dispose()
     {
         foreach (var (client, _) in clients)
@@ -56,6 +59,7 @@ internal void ConfigureLaunchEnvironment(IDictionary<string, string> environment
         browserRefreshServer?.ConfigureLaunchEnvironment(environmentBuilder, enableHotReload: true);
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     internal void InitiateConnection(CancellationToken cancellationToken)
     {
         foreach (var (client, _) in clients)
@@ -64,11 +68,13 @@ internal void InitiateConnection(CancellationToken cancellationToken)
         }
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     internal async ValueTask WaitForConnectionEstablishedAsync(CancellationToken cancellationToken)
     {
         await Task.WhenAll(clients.Select(c => c.client.WaitForConnectionEstablishedAsync(cancellationToken)));
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public async ValueTask<ImmutableArray<string>> GetUpdateCapabilitiesAsync(CancellationToken cancellationToken)
     {
         if (clients is [var (singleClient, _)])
@@ -83,6 +89,7 @@ public async ValueTask<ImmutableArray<string>> GetUpdateCapabilitiesAsync(Cancel
         return [.. results.SelectMany(r => r).Distinct(StringComparer.Ordinal).OrderBy(c => c)];
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public async ValueTask ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken)
     {
         var anyFailure = false;
@@ -139,6 +146,7 @@ public async ValueTask ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadMana
         }
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public async ValueTask InitialUpdatesAppliedAsync(CancellationToken cancellationToken)
     {
         if (clients is [var (singleClient, _)])
@@ -151,6 +159,7 @@ public async ValueTask InitialUpdatesAppliedAsync(CancellationToken cancellation
         }
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public async Task ApplyStaticAssetUpdatesAsync(IEnumerable<(string filePath, string relativeUrl, string assemblyName, bool isApplicationProject)> assets, CancellationToken cancellationToken)
     {
         if (browserRefreshServer != null)
@@ -190,6 +199,7 @@ public async Task ApplyStaticAssetUpdatesAsync(IEnumerable<(string filePath, str
         }
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public async ValueTask ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStaticAssetUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken)
     {
         if (clients is [var (singleClient, _)])
@@ -202,6 +212,7 @@ public async ValueTask ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStat
         }
     }
 
+    /// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
     public ValueTask ReportCompilationErrorsInApplicationAsync(ImmutableArray<string> compilationErrors, CancellationToken cancellationToken)
         => browserRefreshServer?.ReportCompilationErrorsInBrowserAsync(compilationErrors, cancellationToken) ?? ValueTask.CompletedTask;
 }