Skip to content

Commit d7d1a04

Browse files
When Node is launched with a debug listener, disable connection draining on restart. Fixesaspnet#506.
1 parent 351fe3d commit d7d1a04

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ public class NodeInvocationException : Exception
1313
/// </summary>
1414
publicboolNodeInstanceUnavailable{get;privateset;}
1515

16+
/// <summary>
17+
/// If true, indicates that even though the invocation failed because the Node.js instance could not be reached
18+
/// or needs to be restarted, that Node.js instance may remain alive for a period in order to complete any
19+
/// outstanding requests.
20+
/// </summary>
21+
publicboolAllowConnectionDraining{get;privateset;}
22+
1623
/// <summary>
1724
/// Creates a new instance of <see cref="NodeInvocationException"/>.
1825
/// </summary>
@@ -29,10 +36,20 @@ public NodeInvocationException(string message, string details)
2936
/// <param name="message">A description of the exception.</param>
3037
/// <param name="details">Additional information, such as a Node.js stack trace, representing the exception.</param>
3138
/// <param name="nodeInstanceUnavailable">Specifies a value for the <see cref="NodeInstanceUnavailable"/> flag.</param>
32-
publicNodeInvocationException(stringmessage,stringdetails,boolnodeInstanceUnavailable)
39+
/// <param name="allowConnectionDraining">Specifies a value for the <see cref="AllowConnectionDraining"/> flag.</param>
40+
publicNodeInvocationException(stringmessage,stringdetails,boolnodeInstanceUnavailable,boolallowConnectionDraining)
3341
:this(message,details)
3442
{
43+
// Reject a meaningless combination of flags
44+
if(allowConnectionDraining&&!nodeInstanceUnavailable)
45+
{
46+
thrownewArgumentException(
47+
$"The '${nameof(allowConnectionDraining) }' parameter cannot be true "+
48+
$"unless the '${nameof(nodeInstanceUnavailable) }' parameter is also true.");
49+
}
50+
3551
NodeInstanceUnavailable=nodeInstanceUnavailable;
52+
AllowConnectionDraining=allowConnectionDraining;
3653
}
3754
}
3855
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ npm install -g node-inspector
4242
privatereadonlyStringAsTempFile_entryPointScript;
4343
privateFileSystemWatcher_fileSystemWatcher;
4444
privateint_invocationTimeoutMilliseconds;
45+
privatebool_launchWithDebugging;
4546
privatereadonlyProcess_nodeProcess;
4647
privateint?_nodeDebuggingPort;
4748
privatebool_nodeProcessNeedsRestart;
@@ -78,9 +79,10 @@ public OutOfProcessNodeInstance(
7879
OutputLogger=nodeOutputLogger;
7980
_entryPointScript=newStringAsTempFile(entryPointScript);
8081
_invocationTimeoutMilliseconds=invocationTimeoutMilliseconds;
82+
_launchWithDebugging=launchWithDebugging;
8183

8284
varstartInfo=PrepareNodeProcessStartInfo(_entryPointScript.FileName,projectPath,commandLineArguments,
83-
environmentVars,launchWithDebugging,debuggingPort);
85+
environmentVars,_launchWithDebugging,debuggingPort);
8486
_nodeProcess=LaunchNodeProcess(startInfo);
8587
_watchFileExtensions=watchFileExtensions;
8688
_fileSystemWatcher=BeginFileWatcher(projectPath);
@@ -103,10 +105,17 @@ public async Task<T> InvokeExportAsync<T>(
103105
{
104106
// This special kind of exception triggers a transparent retry - NodeServicesImpl will launch
105107
// a new Node instance and pass the invocation to that one instead.
108+
// Note that if the Node process is listening for debugger connections, then we need it to shut
109+
// down immediately and not stay open for connection draining (because if it did, the new Node
110+
// instance wouldn't able to start, because the old one would still hold the debugging port).
106111
varmessage=_nodeProcess.HasExited
107112
?"The Node process has exited"
108113
:"The Node process needs to restart";
109-
thrownewNodeInvocationException(message,null,nodeInstanceUnavailable:true);
114+
thrownewNodeInvocationException(
115+
message,
116+
details:null,
117+
nodeInstanceUnavailable:true,
118+
allowConnectionDraining:!_launchWithDebugging);
110119
}
111120

112121
// Construct a new cancellation token that combines the supplied token with the configured invocation

‎src/Microsoft.AspNetCore.NodeServices/NodeServicesImpl.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ private async Task<T> InvokeExportWithPossibleRetryAsync<T>(string moduleName, s
7272
{
7373
if(_currentNodeInstance==nodeInstance)
7474
{
75-
DisposeNodeInstance(_currentNodeInstance,delay:ConnectionDrainingTimespan);
75+
vardisposalDelay=ex.AllowConnectionDraining?ConnectionDrainingTimespan:TimeSpan.Zero;
76+
DisposeNodeInstance(_currentNodeInstance,disposalDelay);
7677
_currentNodeInstance=null;
7778
}
7879
}

0 commit comments

Comments
(0)