feat: add env vars for keepAliveTimeout and headersTimeout

[theandrew168]

Hey there! First off, I want to say thanks for SvelteKit and all of the work done by contributors.

While working on some recent deployments, I noticed that SvelteKit's adapter-node doesn't expose a way to override Node's keepAliveTimeout and headersTimeout values. These often need tweaking when running Node behind a reverse proxy (since the upstream web server's timeouts should be longer than the proxy's). Otherwise, systems can find themselves dealing with intermittent 502 errors.

To that end, this PR adds checks for corresponding environment variables: KEEP_ALIVE_TIMEOUT and HEADERS_TIMEOUT. When present, they will override Node's default settings. Node expects these to be in milliseconds but I chose to have the vars be in seconds to match the other adapter-node timeouts (SHUTDOWN_TIMEOUT and IDLE_TIMEOUT).

Additionally, I slightly modified how and when polka's underlying HTTP server gets created. When not provided directly to polka, the server doesn't get created until calling .listen(). At that point, you can modify polka.server but it is already running. This has a chance of causing a race condition between the server accepting connections and the properties being changed. By manually creating the server ourselves and passing it to polka, this race is avoided. Plus, it solves some of the typing issues related to polka.server being a net.Server and not an http.Server (I've got a PR up to fix that, though).

Test Plan

I tested this locally (in playgrounds/basic and after running pnpm build) by running the following script (timeouts.js) with a few different env var configurations:

import { server } from './build/index.js';

console.log('keepAliveTimeout', server.server.keepAliveTimeout);
console.log('headersTimeout', server.server.headersTimeout);

Without any vars, we see the defaults:

$ node timeouts.js
keepAliveTimeout 5000
headersTimeout 60000
Listening on http://0.0.0.0:3000

With both new vars specified, we see that the values have been successfully overridden:

$ KEEP_ALIVE_TIMEOUT=60 HEADERS_TIMEOUT=65 node timeouts.js
keepAliveTimeout 60000
headersTimeout 65000
Listening on http://0.0.0.0:3000

With invalid vars, a meaningful error is thrown:

$ KEEP_ALIVE_TIMEOUT=foobar node timeouts.js
file:///Users/derz/Code/kit/playgrounds/basic/build/env.js:57
	throw new Error(
	      ^

Error: Invalid value for environment variable KEEP_ALIVE_TIMEOUT: "foobar" (should be a non-negative integer)
    at parsing_error (file:///Users/derz/Code/kit/playgrounds/basic/build/env.js:57:8)
    at timeout_env (file:///Users/derz/Code/kit/playgrounds/basic/build/env.js:75:3)
    at file:///Users/derz/Code/kit/playgrounds/basic/build/index.js:268:28

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1b2753d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/adapter-node	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-15125-svelte.vercel.app/

[teemingc]

cc: @Conduitry you might be interested in this one since it pertains to the Node adapter

[elliott-with-the-longest-name-on-github]

Let's add a better parsing abstraction here. In the env module, declare an int_env

const integer = /^\d+$/;

function parsing_error(name, value, description) {
  throw new Error(`Invalid value for environment variable ${name}: ${JSON.stringify(value)} (${description})`)
}

/**
 * @param {string} name
 * @param {number} [fallback]
 * @returns {number} | undefined
 */
function int_env(name, fallback) {
  const raw = env(name);
  if (!raw) {
    return fallback;
  }
  if (!integer.test(raw)) {
    parsing_error('value was a string but could not be parsed as an integer');
  }
  const parsed = Number.parseInt(raw, 10);
  // we don't technically need to check `Number.isNaN` because the integer regex will always parse, 
  // but just in case there's some new codepath introduced somewhere down the line, it's probably good to stick this in here
  if (Number.isNaN(parsed)) {
    parsing_error('should be a number');
  }
  if (parsed < 0) {
    parsing_error('should be non-negative');
  }
  return parsed;
}

Then we can use this for both of the variables here. It'll be a lot cleaner, as we can just do const keep_alive_timeout = int_env('KEEP_ALIVE_TIMEOUT');, check if it exists, and assign it if so.

[theandrew168]

Do you suppose it'd be worth being more specific and calling it non_negative_int_env? I'm fine either way and agree that a helper here will be better for readability.

[elliott-with-the-longest-name-on-github]

Maybe just timeout(name, fallback) instead? If we ever need more-specific int handling we can break it out to other functions, but timeouts can't be negative.

[theandrew168]

I added a new timeout_env helper in 1b2753d. I also made an attempt at adding some unit tests but hit a snag since the env.js file expects the ENV_PREFIX global to be defined. A vi.stubGlobal plus a dynamic import got things working, though. If that is too hacky, I'm happy to adjust or remove the tests altogether.