fix: EADDRINUSE: address already in use - when a stale SOCKET_PATH file already exists

[marko-rbn]

Occasionally socket file is not removed when server crashes/shutdown. This patch attempts to delete the file before restarting the server. Applies similar solution as was pulled into adapter-deno package recently:

https://github.com/NextLegacy/sveltekit-adapter-deno
NextLegacy/sveltekit-adapter-deno#1
NextLegacy/sveltekit-adapter-deno#2

To reproduce, create a sock file, and attempt to start the server:

> node -r dotenv/config index.js
node:events:486
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use /var/home/.../app/.run/stage.sock
    at Server.setupListenHandle [as _listen2] (node:net:1994:21)
    at listenInCluster (node:net:2073:12)
    at Server.listen (node:net:2195:5)
    at Polka.listen (file:///var/home/.../app/stage/index.js:206:22)
    at file:///var/home/.../app/stage/index.js:310:9
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:2052:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
  code: 'EADDRINUSE',
  errno: -48,
  syscall: 'listen',
  address: '/var/home/.../app/.run/stage.sock',
  port: -1
}

After adding this patch, starting the server succeeds:

> node -r dotenv/config index.js
Listening on /var/home/.../app/.run/stage.sock

---

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: f1cce34

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	Patch

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

[Conduitry]

This feels dangerous. If someone accidentally passes the wrong path in the environment variable, we'd be deleting some random file on their system. I'd prefer it to be the caller's responsibility to make sure that the spot for the socket is clear, the way this works in a number of other tools.

If we do decide we want to do this, I'd be in favor of using rm(path, { force: true }) rather than catching and suppressing one specific error.

[yds]

This feels dangerous. If someone accidentally passes the wrong path in the environment variable, we'd be deleting some random file on their system.

yes, it "feels" dangerous. however this is the only way to ensure the node server starts up if there's already an existing file of any sort at the same path as SOCKET_PATH

I'd prefer it to be the caller's responsibility to make sure that the spot for the socket is clear, the way this works in a number of other tools.

clearing the SOCKET_PATH before node startup does not fix the edge case where node is running under a supervisor such as BSD daemon(8) and node shuts down or crashes and gets restarted by the supervisor. in that case node goes into a loop unable to start up cuz there's a stale socket file it never cleared because it crashed.

as for "other tools", servers such as OpenLDAP's slapd do always leave a stale socket file laying around and upon startup, slapd always deletes whatever it finds at that path to make room for the socket file it needs to create to be able to start up. which also "feels" dangerous, exactly as you mentioned, but I'd argue it's the caller's responsibility to not shoot oneself in the foot, so to speak. ;)

there's plenty of prior art where servers deal with this issue in different ways. however this is a real world issue where node.js has not yet applied the same fix that was fixed in deno v2.6.7:
fix(ext/net): remove socket file when dropping unix listener (denoland/deno#31947)

but even that fix does not address the edge case where a stale socket file gets left behind cuz node crashes and never gets a chance to clean up the socket file -- running under a supervisor the node proc will go into a loop unable to fully start up cuz there's a stale socket file in the way.

most of the arguments about this have already been hashed out in the denoland thread

If we do decide we want to do this, I'd be in favor of using rm(path, { force: true }) rather than catching and suppressing one specific error.

agreed -- that does look better.

P.S. I'm the one who submitted a similar fix to the NextLegacy/sveltekit-adapter-deno -- I'll make another PR there to do rm(path, { force: true }) as you suggest.

[Conduitry]

If something genuinely is a socket file from a previous run, it will have size zero now, correct? What do folks think about statting the file and, if it exists and is zero bytes, automatically deleting it? I think/hope that would be just as usable and low-friction as the current unconditional removal - and it reduces my concerns about deleting the wrong file considerably, as it will just be something with zero bytes.

[Rich-Harris]

implemented that change

[Conduitry]

LGTM!