When Node is launched with a debug listener, disable connection draining on restart. Fixes #506.

Author: SteveSandersonMS

src/Microsoft.AspNetCore.NodeServices/HostingModels/NodeInvocationException.cs

@@ -13,6 +13,13 @@ public class NodeInvocationException : Exception
         /// </summary>
         public bool NodeInstanceUnavailable { get; private set; }
 
+        /// <summary>
+        /// If true, indicates that even though the invocation failed because the Node.js instance could not be reached
+        /// or needs to be restarted, that Node.js instance may remain alive for a period in order to complete any
+        /// outstanding requests.
+        /// </summary>
+        public bool AllowConnectionDraining { get; private set;}
+
         /// <summary>
         /// Creates a new instance of <see cref="NodeInvocationException"/>.
         /// </summary>
@@ -29,10 +36,20 @@ public NodeInvocationException(string message, string details)
         /// <param name="message">A description of the exception.</param>
         /// <param name="details">Additional information, such as a Node.js stack trace, representing the exception.</param>
         /// <param name="nodeInstanceUnavailable">Specifies a value for the <see cref="NodeInstanceUnavailable"/> flag.</param>
-        public NodeInvocationException(string message, string details, bool nodeInstanceUnavailable)
+        /// <param name="allowConnectionDraining">Specifies a value for the <see cref="AllowConnectionDraining"/> flag.</param>
+        public NodeInvocationException(string message, string details, bool nodeInstanceUnavailable, bool allowConnectionDraining)
             : this(message, details)
         {
+            // Reject a meaningless combination of flags
+            if (allowConnectionDraining && !nodeInstanceUnavailable)
+            {
+                throw new ArgumentException(
+                    $"The '${ nameof(allowConnectionDraining) }' parameter cannot be true " +
+                    $"unless the '${ nameof(nodeInstanceUnavailable) }' parameter is also true.");
+            }
+
             NodeInstanceUnavailable = nodeInstanceUnavailable;
+            AllowConnectionDraining = allowConnectionDraining;
         }
     }
 }

src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs

@@ -42,6 +42,7 @@ npm install -g node-inspector
         private readonly StringAsTempFile _entryPointScript;
         private FileSystemWatcher _fileSystemWatcher;
         private int _invocationTimeoutMilliseconds;
+        private bool _launchWithDebugging;
         private readonly Process _nodeProcess;
         private int? _nodeDebuggingPort;
         private bool _nodeProcessNeedsRestart;
@@ -78,9 +79,10 @@ public OutOfProcessNodeInstance(
             OutputLogger = nodeOutputLogger;
             _entryPointScript = new StringAsTempFile(entryPointScript);
             _invocationTimeoutMilliseconds = invocationTimeoutMilliseconds;
+            _launchWithDebugging = launchWithDebugging;
 
             var startInfo = PrepareNodeProcessStartInfo(_entryPointScript.FileName, projectPath, commandLineArguments,
-                environmentVars, launchWithDebugging, debuggingPort);
+                environmentVars, _launchWithDebugging, debuggingPort);
             _nodeProcess = LaunchNodeProcess(startInfo);
             _watchFileExtensions = watchFileExtensions;
             _fileSystemWatcher = BeginFileWatcher(projectPath);
@@ -103,10 +105,17 @@ public async Task<T> InvokeExportAsync<T>(
             {
                 // This special kind of exception triggers a transparent retry - NodeServicesImpl will launch
                 // a new Node instance and pass the invocation to that one instead.
+                // Note that if the Node process is listening for debugger connections, then we need it to shut
+                // down immediately and not stay open for connection draining (because if it did, the new Node
+                // instance wouldn't able to start, because the old one would still hold the debugging port).
                 var message = _nodeProcess.HasExited
                     ? "The Node process has exited"
                     : "The Node process needs to restart";
-                throw new NodeInvocationException(message, null, nodeInstanceUnavailable: true);
+                throw new NodeInvocationException(
+                    message,
+                    details: null,
+                    nodeInstanceUnavailable: true,
+                    allowConnectionDraining: !_launchWithDebugging);
             }
 
             // Construct a new cancellation token that combines the supplied token with the configured invocation

src/Microsoft.AspNetCore.NodeServices/NodeServicesImpl.cs

@@ -72,7 +72,8 @@ private async Task<T> InvokeExportWithPossibleRetryAsync<T>(string moduleName, s
                     {
                         if (_currentNodeInstance == nodeInstance)
                         {
-                            DisposeNodeInstance(_currentNodeInstance, delay: ConnectionDrainingTimespan);
+                            var disposalDelay = ex.AllowConnectionDraining ? ConnectionDrainingTimespan : TimeSpan.Zero;
+                            DisposeNodeInstance(_currentNodeInstance, disposalDelay);
                             _currentNodeInstance = null;
                         }
                     }