Node-adapted app crashes on request if referenced ADDRESS_HEADER is missing

[Conduitry]

Describe the bug

Making a request to a non-prerendered page of a Node-adapter application with ADDRESS_HEADER set to a header that's not present in the request crashes the application. This happens even if the user code does not use getClientAddress anywhere.

Reproduction

1. Start with the default demo app
2. Swap out the auto adapter for the Node adapter
3. Build it with npm run build
4. Start it with ADDRESS_HEADER=blah node build
5. Run curl localhost:3000/sverdle

This crashes the server for me. Interestingly, curl localhost:3000 does not crash the server, presumably because that page is prerendered.

Logs

Error: Address header was specified with ADDRESS_HEADER=blah but is absent from request
    at Array.ssr (file:///.../build/handler.js:1198:9)

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
    CPU: (16) x64 AMD Ryzen 9 4900HS with Radeon Graphics
    Memory: 14.28 GB / 19.25 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 20.3.1
    Yarn: 1.22.19
    npm: 9.6.7
    pnpm: 8.6.3
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0 => 1.2.4
    @sveltejs/kit: ^1.20.4 => 1.21.0
    svelte: ^4.0.0 => 4.0.1
    vite: ^4.3.6 => 4.3.9

Severity

serious, but I can work around it

Additional Information

I think we should just move the check for the header referenced in ADDRESS_HEADER into the getClientAddress callback we pass back from the adapted output. We definitely don't want to crash the app. Another possibility would be to make that be an instant 500 without calling any of the underlying SvelteKit code (and without waiting for getClientAddress() to be called), but that doesn't seem quite necessary (maybe the user wants to handle that case!) and I'm not sure what a nice way to do that would be without duplicating SvelteKIt's error.html templating logic.

I'm working around this for now by simply using the appropriate headers directly - with my own fallbacks if they don't exist - rather than trying to use getClientAddress().

[Conduitry]

I was looking at too many crazy things and my brain had stopped working. The check that the header is present is not inside the getClientAddress() call like the other XFF_DEPTH checks are. I'll amend my issue description above accordingly.

This is still a bad bug. It shouldn't crash the server if these headers are missing. IMO we should just move this check inside the getClientAddress() function, so that that will throw if it's missing, which will either get handled by the user code, or will result in a 500 from the server.

[Conduitry]

On the other hand, I think it may very well make sense to make a zero/negative XFF_DEPTH value crash the server, since that's something that can be immediately determined at startup, and it's not going to be something that can bring down the server later. That would probably be a breaking change though, and we should think about whether there's other stuff (startup env var validation or otherwise) that we might want to do at the same time.