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]

⚠️ No Changeset found

Latest commit: 6f7cbe5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a 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.