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

This commit is contained in:
SteveSandersonMS
2017-01-20 17:32:26 +00:00
parent 351fe3d15c
commit d7d1a04751
3 changed files with 31 additions and 4 deletions

View File

@@ -13,6 +13,13 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels
/// </summary> /// </summary>
public bool NodeInstanceUnavailable { get; private set; } 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> /// <summary>
/// Creates a new instance of <see cref="NodeInvocationException"/>. /// Creates a new instance of <see cref="NodeInvocationException"/>.
/// </summary> /// </summary>
@@ -29,10 +36,20 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels
/// <param name="message">A description of the exception.</param> /// <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="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> /// <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) : 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; NodeInstanceUnavailable = nodeInstanceUnavailable;
AllowConnectionDraining = allowConnectionDraining;
} }
} }
} }

View File

@@ -42,6 +42,7 @@ If you haven't yet installed node-inspector, you can do so as follows:
private readonly StringAsTempFile _entryPointScript; private readonly StringAsTempFile _entryPointScript;
private FileSystemWatcher _fileSystemWatcher; private FileSystemWatcher _fileSystemWatcher;
private int _invocationTimeoutMilliseconds; private int _invocationTimeoutMilliseconds;
private bool _launchWithDebugging;
private readonly Process _nodeProcess; private readonly Process _nodeProcess;
private int? _nodeDebuggingPort; private int? _nodeDebuggingPort;
private bool _nodeProcessNeedsRestart; private bool _nodeProcessNeedsRestart;
@@ -78,9 +79,10 @@ If you haven't yet installed node-inspector, you can do so as follows:
OutputLogger = nodeOutputLogger; OutputLogger = nodeOutputLogger;
_entryPointScript = new StringAsTempFile(entryPointScript); _entryPointScript = new StringAsTempFile(entryPointScript);
_invocationTimeoutMilliseconds = invocationTimeoutMilliseconds; _invocationTimeoutMilliseconds = invocationTimeoutMilliseconds;
_launchWithDebugging = launchWithDebugging;
var startInfo = PrepareNodeProcessStartInfo(_entryPointScript.FileName, projectPath, commandLineArguments, var startInfo = PrepareNodeProcessStartInfo(_entryPointScript.FileName, projectPath, commandLineArguments,
environmentVars, launchWithDebugging, debuggingPort); environmentVars, _launchWithDebugging, debuggingPort);
_nodeProcess = LaunchNodeProcess(startInfo); _nodeProcess = LaunchNodeProcess(startInfo);
_watchFileExtensions = watchFileExtensions; _watchFileExtensions = watchFileExtensions;
_fileSystemWatcher = BeginFileWatcher(projectPath); _fileSystemWatcher = BeginFileWatcher(projectPath);
@@ -103,10 +105,17 @@ If you haven't yet installed node-inspector, you can do so as follows:
{ {
// This special kind of exception triggers a transparent retry - NodeServicesImpl will launch // This special kind of exception triggers a transparent retry - NodeServicesImpl will launch
// a new Node instance and pass the invocation to that one instead. // 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 var message = _nodeProcess.HasExited
? "The Node process has exited" ? "The Node process has exited"
: "The Node process needs to restart"; : "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 // Construct a new cancellation token that combines the supplied token with the configured invocation

View File

@@ -72,7 +72,8 @@ namespace Microsoft.AspNetCore.NodeServices
{ {
if (_currentNodeInstance == nodeInstance) if (_currentNodeInstance == nodeInstance)
{ {
DisposeNodeInstance(_currentNodeInstance, delay: ConnectionDrainingTimespan); var disposalDelay = ex.AllowConnectionDraining ? ConnectionDrainingTimespan : TimeSpan.Zero;
DisposeNodeInstance(_currentNodeInstance, disposalDelay);
_currentNodeInstance = null; _currentNodeInstance = null;
} }
} }