Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable keep alive timeout, headers timeout and log any server timeouts #85

Merged
merged 3 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 28 additions & 2 deletions src/NodeJS/Javascript/Servers/OutOfProcess/Http/HttpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);

Expand Down Expand Up @@ -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.
Expand Down