diff --git a/Changelog.md b/Changelog.md index fbad1ba..c8666c3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,7 +3,11 @@ This project uses [semantic versioning](http://semver.org/spec/v2.0.0.html). Ref *[Semantic Versioning in Practice](https://www.jering.tech/articles/semantic-versioning-in-practice)* for an overview of semantic versioning. -## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.1...HEAD) +## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.2...HEAD) + +## [5.4.2](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.1...5.4.2) - Jun 25, 2020 +### Fixes +- Disabled unecessary Node.js HTTP timeouts, added logging for timeouts. ([#85](https://github.com/JeringTech/Javascript.NodeJS/pull/85)). ## [5.4.1](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.0...5.4.1) - Jun 23, 2020 ### Fixes diff --git a/src/NodeJS/Javascript/Servers/OutOfProcess/Http/HttpServer.ts b/src/NodeJS/Javascript/Servers/OutOfProcess/Http/HttpServer.ts index e926b39..1f59955 100644 --- a/src/NodeJS/Javascript/Servers/OutOfProcess/Http/HttpServer.ts +++ b/src/NodeJS/Javascript/Servers/OutOfProcess/Http/HttpServer.ts @@ -3,7 +3,7 @@ var Module = require('module'); import * as path from 'path'; import * as http from 'http'; import * as stream from 'stream'; -import { AddressInfo } from 'net'; +import { AddressInfo, Socket } from 'net'; import InvocationRequest from '../../../InvocationData/InvocationRequest'; import ModuleSourceType from '../../../InvocationData/ModuleSourceType'; @@ -26,9 +26,29 @@ let projectDir = process.cwd(); // Create server const server = http.createServer(serverOnRequestListener); -// In Node.js v13+ this is the default, however for earlier versions it is 120 seconds. +// The timeouts below are designed to manage network instability. Since we're using the HTTP protocol on a local machine, we can disable them +// to avoid their overhead and stability issues. + +// By default, on older versions of Node.js, request handling times out after 120 seconds. +// This timeout is disabled by default on Node.js v13+. +// Becuase of the older versions, we explicitly disable it. server.setTimeout(0); +// By default, a socket is destroyed if it receives no incoming data for 5 seconds: https://nodejs.org/api/http.html#http_server_keepalivetimeout. +// This is good practice when making external requests because DNS records may change: https://github.com/dotnet/runtime/issues/18348. +// Since we're using the HTTP protocol on a local machine, it's safe and more efficient to keep sockets alive indefinitely. +server.keepAliveTimeout = 0; + +// By default, a socket is destroyed if its incoming headers take longer than 60 seconds: https://nodejs.org/api/http.html#http_server_headerstimeout. +// In early versions of Node.js, even if setTimeout() was specified with a non-zero value, the server would wait indefinitely for headers. +// This timeout was added to deal with that issue. We specify setTimeout(0), so this timeout is of no use to us. +// +// Note that while 0 disables this timeout in node 12.17+, in earlier versions it causes requests to time out immediately, so set to 10 years. +server.headersTimeout = 10 * 365 * 24 * 60 * 60 * 1000; + +// Log timed out connections for debugging +server.on('timeout', serverOnTimeout); + // Send client error details to client for debugging server.on('clientError', serverOnClientError); @@ -182,6 +202,12 @@ function serverOnClientError(error: Error, socket: stream.Duplex) { socket.end(httpResponseMessage); } +// Send timeout details to client for debugging - this shouldn't fire but there have been various node http server timeout issues in the past. +// The socket won't actually get closed (the timeout function needs to do that manually). +function serverOnTimeout(socket: Socket) { + console.error(`Ignoring unexpected socket timeout for address ${socket.remoteAddress}, port ${socket.remotePort}`); +} + function serverOnListeningListener() { // Signal to HttpNodeHost which loopback IP address (IPv4 or IPv6) and port it should make its HTTP connections on // and that we are ready to process invocations.