fix(server): should close server after create new server

[sun0day]

Description

fix #12375
superseds close #11680

Additional context

After this PR, current server will not be closed if creating new server fails, so we could tell user that server restart failed explicitly.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[sapphi-red]

The following error happens when saving the config without any changes.

[ENOENT: no such file or directory, stat '/home/vitejs/vite/playground/assets/node_modules/.vite/deps_temp_c5a19aad'] {
  code: 'ENOENT',
  errno: -2,
  path: '/home/vitejs/vite/playground/assets/node_modules/.vite/deps_temp_c5a19aad',
  syscall: 'stat'
}

I guess it's happening because:

1. the deps_temp directory is created by createServer (but since it already exists the creation is skipped)
2. the deps_temp directory gets deleted by await server.close()
3. optimizer fails because deps_temp directory doesn't exist

[bluwy]

When starting the new server, with the old server still lingering, wouldn't there be a chance the port would conflict? Maybe we need a callback when the new server's resolveConfig() is finished and then we teardown the old server 🤔

[sun0day]

When starting the new server, with the old server still lingering, wouldn't there be a chance the port would conflict? Maybe we need a callback when the new server's resolveConfig() is finished and then we teardown the old server 🤔

server.listen is called after server.close, so the port wouldn't conflict. Ideally the restart logic will be that only when a new server instance is created successfully, then the old one should close. Adding resolveConfig() callback can certainly solve #12375, but I think it's better to decouple create and close.

[bluwy]

Ah I see, then we could go with this flow then once the issue sapphi brought up is resolved. Thanks for the explanation.

[sun0day]

I found that deps_temp will be recreated every time when vite creates a new server instance because of loading the new config(even if doesn't change) from vite.config.js. Not sure whether this is a bug or intended. @sapphi-red @bluwy

[sun0day]

tempDirPath maybe removed when server.close(), so add a catch to make clean up process work properly, this will solve sapphi-red's issue.

[sun0day]

https://github.com/vitejs/vite/pull/12379/files#diff-456b3c61f2f19b160b6a5016cb9a180500c4e3b3a9a41a192ca24670637e7706L1282

Error throwed by cleanupDepsCacheStaleDirs will confuse users, do they need to know this error?

If needed, maybe we can add more context info to this error and expose it in debug mode @patak-dev

[patak-dev]

This looks good to me. Let's merge it so it makes it in Vite 4.2, @sun0day already resolved the issue reported by @sapphi-red.

[patak-dev]

Error thrown by cleanupDepsCacheStaleDirs will confuse users, do they need to know this error?

I don't think users need to be aware of these errors 👍🏼