Compare commits

...
Author SHA1 Message Date
Rob HoganandFacebook GitHub Bot 34078cab40 Remove HTTP keepAliveTimeout workaround not needed since Node.js 12
Summary:
The removed code comment is pretty clear that this workaround is in place for a specific bug, which was closed in a Node.js 8.x minor: https://github.com/nodejs/node/issues/13391

A related issue descibed a problem that still existed in Node 10:
https://github.com/nodejs/node/issues/27363

Subsequent comments reporting problems with newer Node.js versions all seem to relate to mismatched configuration with load balancers, not directly relevant to Metro.

Reproducers for the original bugs no longer repro on new Node.js (tested with 22.14), so I think it's safe to conclude this is fixed.

Changelog: [Internal]

Differential Revision: D78158427
2025-07-11 04:13:24 -07:00
@@ -147,7 +147,7 @@ async function runServer(
// $FlowIgnore[cannot-write] Assigning to readonly property
metroConfig.reporter = reporter;
const serverInstance = await Metro.runServer(metroConfig, {
await Metro.runServer(metroConfig, {
host: args.host,
secure: args.https,
secureCert: args.cert,
@@ -161,18 +161,6 @@ async function runServer(
reportEvent = eventsSocketEndpoint.reportEvent;
// In Node 8, the default keep-alive for an HTTP connection is 5 seconds. In
// early versions of Node 8, this was implemented in a buggy way which caused
// some HTTP responses (like those containing large JS bundles) to be
// terminated early.
//
// As a workaround, arbitrarily increase the keep-alive from 5 to 30 seconds,
// which should be enough to send even the largest of JS bundles.
//
// For more info: https://github.com/nodejs/node/issues/13391
//
serverInstance.keepAliveTimeout = 30000;
await version.logIfUpdateAvailable(cliConfig, terminalReporter);
}