@@ -37,6 +37,7 @@ npm install -g node-inspector
3737 private bool _disposed ;
3838 private readonly StringAsTempFile _entryPointScript ;
3939 private FileSystemWatcher _fileSystemWatcher ;
40+ private int _invocationTimeoutMilliseconds ;
4041 private readonly Process _nodeProcess ;
4142 private int ? _nodeDebuggingPort ;
4243 private bool _nodeProcessNeedsRestart ;
@@ -49,6 +50,7 @@ public OutOfProcessNodeInstance(
4950 string commandLineArguments ,
5051 ILogger nodeOutputLogger ,
5152 IDictionary < string , string > environmentVars ,
53+ int invocationTimeoutMilliseconds ,
5254 bool launchWithDebugging ,
5355 int debuggingPort )
5456 {
@@ -59,6 +61,7 @@ public OutOfProcessNodeInstance(
5961
6062 OutputLogger = nodeOutputLogger ;
6163 _entryPointScript = new StringAsTempFile ( entryPointScript ) ;
64+ _invocationTimeoutMilliseconds = invocationTimeoutMilliseconds ;
6265
6366 var startInfo = PrepareNodeProcessStartInfo ( _entryPointScript . FileName , projectPath , commandLineArguments ,
6467 environmentVars , launchWithDebugging , debuggingPort ) ;
@@ -81,17 +84,74 @@ public async Task<T> InvokeExportAsync<T>(
8184 throw new NodeInvocationException ( message , null , nodeInstanceUnavailable : true ) ;
8285 }
8386
84- // Wait until the connection is established. This will throw if the connection fails to initialize,
85- // or if cancellation is requested first. Note that we can't really cancel the "establishing connection"
86- // task because that's shared with all callers, but we can stop waiting for it if this call is cancelled.
87- await _connectionIsReadySource . Task . OrThrowOnCancellation ( cancellationToken ) ;
88-
89- return await InvokeExportAsync < T > ( new NodeInvocationInfo
87+ // Construct a new cancellation token that combines the supplied token with the configured invocation
88+ // timeout. Technically we could avoid wrapping the cancellationToken if no timeout is configured,
89+ // but that's not really a major use case, since timeouts are enabled by default.
90+ using ( var timeoutSource = new CancellationTokenSource ( ) )
91+ using ( var combinedCancellationTokenSource = CancellationTokenSource . CreateLinkedTokenSource ( cancellationToken , timeoutSource . Token ) )
9092 {
91- ModuleName = moduleName ,
92- ExportedFunctionName = exportNameOrNull ,
93- Args = args
94- } , cancellationToken ) ;
93+ if ( _invocationTimeoutMilliseconds > 0 )
94+ {
95+ timeoutSource . CancelAfter ( _invocationTimeoutMilliseconds ) ;
96+ }
97+
98+ // By overwriting the supplied cancellation token, we ensure that it isn't accidentally used
99+ // below. We only want to pass through the token that respects timeouts.
100+ cancellationToken = combinedCancellationTokenSource . Token ;
101+ var connectionDidSucceed = false ;
102+
103+ try
104+ {
105+ // Wait until the connection is established. This will throw if the connection fails to initialize,
106+ // or if cancellation is requested first. Note that we can't really cancel the "establishing connection"
107+ // task because that's shared with all callers, but we can stop waiting for it if this call is cancelled.
108+ await _connectionIsReadySource . Task . OrThrowOnCancellation ( cancellationToken ) ;
109+ connectionDidSucceed = true ;
110+
111+ return await InvokeExportAsync < T > ( new NodeInvocationInfo
112+ {
113+ ModuleName = moduleName ,
114+ ExportedFunctionName = exportNameOrNull ,
115+ Args = args
116+ } , cancellationToken ) ;
117+ }
118+ catch ( TaskCanceledException )
119+ {
120+ if ( timeoutSource . IsCancellationRequested )
121+ {
122+ // It was very common for developers to report 'TaskCanceledException' when encountering almost any
123+ // trouble when using NodeServices. Now we have a default invocation timeout, and attempt to give
124+ // a more descriptive exception message if it happens.
125+ if ( ! connectionDidSucceed )
126+ {
127+ // This is very unlikely, but for debugging, it's still useful to differentiate it from the
128+ // case below.
129+ throw new NodeInvocationException (
130+ $ "Attempt to connect to Node timed out after { _invocationTimeoutMilliseconds } ms.",
131+ string . Empty ) ;
132+ }
133+ else
134+ {
135+ // Developers encounter this fairly often (if their Node code fails without invoking the callback,
136+ // all that the .NET side knows is that the invocation eventually times out). Previously, this surfaced
137+ // as a TaskCanceledException, but this led to a lot of issue reports. Now we throw the following
138+ // descriptive error.
139+ throw new NodeInvocationException (
140+ $ "The Node invocation timed out after { _invocationTimeoutMilliseconds } ms.",
141+ $ "You can change the timeout duration by setting the { NodeServicesOptions . TimeoutConfigPropertyName } "
142+ + $ "property on { nameof ( NodeServicesOptions ) } .\n \n "
143+ + "The first debugging step is to ensure that your Node.js function always invokes the supplied "
144+ + "callback (or throws an exception synchronously), even if it encounters an error. Otherwise, "
145+ + "the .NET code has no way to know that it is finished or has failed."
146+ ) ;
147+ }
148+ }
149+ else
150+ {
151+ throw ;
152+ }
153+ }
154+ }
95155 }
96156
97157 public void Dispose ( )
0 commit comments