Aspnetcore: Unable to configure server timeout of Node HTTP server using ASP.NET Core NodeServices

[Jaffacakes82]

Descriptions

The current implementation of NodeServices doesn't allow for configuration of the server.timeout property of the Node HTTP server created at runtime. This causes issues if there's a long running process within the Node application (e.g. PDF generation) whereby the Node HTTP server closes the connection to the calling service (in our case an Azure Function) before the process completes.

Background and reproduction

_Versions:_

• Microsoft.AspNetCore.NodeServices 2.2.0
• .NET Core 2.2

_Reproduction:_

• Invoke a Node.js module from AspNetCore.NodeServices which takes longer than 2 minutes to execute
• Observe System.Net.Http.HttpRequestException - The server returned an invalid or unrecognized response. in the .NET Core service that calls the

_Example:_

[FunctionName("PDFExport")]
public void Run(
[QueueTrigger("pdf", Connection = "QueueConnectionString")]
INodeServices nodeServices,
ILogger log)
{
    try 
    {
        var pdfBase64 = nodeServices.InvokeAsync<string>(rootDir + "Modules/jsModuleThatTakesOverTwoMinutes").Result;
    }
    catch (Exception ex)
    {
        // observe exception after exactly two minutes of execution time above
        throw ex;
    }
}

Expected behaviour

The Node HTTP server should not close the connection until all processing is completed. One way to resolve this might be to add a NodeHttpTimeout property to the NodeServicesOptions class which gets passed into the Node runtime as a command line argument and set using server.setTimeout(val);

[mkArtakMSFT]

Thanks for contacting us, @Jaffacakes82.
We would happily consider a PR for this.

[SteveSandersonMS]

If this is something you're interested in adding a PR to address, I think we'd certainly be open to it. My suggestion would be:

• Use the existing InvocationTimeoutMilliseconds option value. Developers can set this when configuring the service, e.g. services.AddNodeServices(options => { options.InvocationTimeoutMilliseconds = 10000 };.
• OutOfProcessNodeInstance receives this as a constructor parameter here.
• You can modify the PrepareNodeProcessStartInfo logic so that it also passes the invocationTimeoutMilliseconds value as a command-line argument to the launched Node process
• Then, in HttpNodeInstanceEntryPoint.ts, see how it already parses incoming command-line args, and get your new timeout param value if supplied (remember to convert to a numeric value using parseInt or similar). Be sure to tolerate no value being passed for that argument. If some value was passed, then you can assign it to server.timeout.

[Jaffacakes82]

@SteveSandersonMS @mkArtakMSFT thanks both. I'll pick this up.

[Jaffacakes82]

@SteveSandersonMS quick question regarding the following:

Then, in HttpNodeInstanceEntryPoint.ts, see how it already parses incoming command-line args, and get your new timeout param value if supplied (remember to convert to a numeric value using parseInt or similar). Be sure to tolerate no value being passed for that argument. If some value was passed, then you can assign it to server.timeout

InvocationTimeoutMilliseconds gets a default value of 60 seconds if it's not set, meaning there's no reliable way to check whether it's set or not. If I were to pass this in to HttpNodeInstanceEntryPoint.ts and set the timeout off the default value, I'm essentially reducing the default timeout of the Node HTTP server by 60 seconds.

What would you expect the behavior to be here? Is it not easier to add an additional, optional property to the options?

[Jaffacakes82]

@SteveSandersonMS @mkArtakMSFT any view on my comment above?

[SteveSandersonMS]

If it's my choice, I'd recommend only having a single "timeout" option, and it being the existing one (InvocationTimeoutMilliseconds). I think it would be strange to have two different timeouts: one to control when the .NET side stops waiting, and a different one to control when the JS side aborts.

If I were to pass this in to HttpNodeInstanceEntryPoint.ts and set the timeout off the default value, I'm essentially reducing the default timeout of the Node HTTP server by 60 seconds.

Sure, but is that a problem? Apps are already giving up and throwing a timeout exception after 60 seconds, so this doesn't break any scenario that wasn't already throwing timeout exceptions.

[impworks]

@Jaffacakes82 I have a similar problem. Did you manage to get this working?

[Jaffacakes82]

@impworks I didn't get it working - it requires a code change (see above comments). It's likely MSFT are still open to PRs.

[impworks]

@Jaffacakes82 OK thanks. The approach by @SteveSandersonMS seems pretty straightforward, I will make a PR in a couple of days.