fix: handle warmup request error correctly

[sapphi-red]

Description

Alternative to #16216

From the stack trace, I guess this would catch the error correctly.

The reason why .catch(e => cannot catch the error is because transformRequest throws an error in sync instead of async.

vite/packages/vite/src/node/server/transformRequest.ts

Line 58 in d443428

if (server._restartPromise && !options.ssr) throwClosedServerError()

Sync errors can only be catched by try {} catch {}.

The fix for it would be to change transformRequest to an async function (that would change the sync error into async error), or, to use try { await transformRequest() } catch {}. Using both .catch and try {} catch {} is also an option.
https://stackblitz.com/edit/node-zucmtr?file=index.js

I went with try { await transformRequest() } catch {}.

Additional context

---

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, especially the Pull Request Guidelines.
• 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).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

Thanks for the alternative! I'm not sure if we should keep throwing the error though. Maybe it is ok to try first this PR. I'm afraid now about external code calling server.transformRequest and pluginContainer.resolve. We aren't exposing formally this error, and I don't think these are properly handled. It may be better to let the current processing to finish when restarting the server. If we bailout from warmup requests, the pending processing should clear rather quickly.

But I'm fine going with this one if you prefer that.

[sapphi-red]

I'm afraid now about external code calling server.transformRequest and pluginContainer.resolve. We aren't exposing formally this error, and I don't think these are properly handled.

Makes sense. But if we are going that way, I think we'll need to remove all of these.

vite/packages/vite/src/node/server/middlewares/transform.ts

Lines 216 to 269 in d443428

	if (e?.code === ERR_OPTIMIZE_DEPS_PROCESSING_ERROR) {
	// Skip if response has already been sent
	if (!res.writableEnded) {
	res.statusCode = 504 // status code request timeout
	res.statusMessage = 'Optimize Deps Processing Error'
	res.end()
	}
	// This timeout is unexpected
	server.config.logger.error(e.message)
	return
	}
	if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) {
	// Skip if response has already been sent
	if (!res.writableEnded) {
	res.statusCode = 504 // status code request timeout
	res.statusMessage = 'Outdated Optimize Dep'
	res.end()
	}
	// We don't need to log an error in this case, the request
	// is outdated because new dependencies were discovered and
	// the new pre-bundle dependencies have changed.
	// A full-page reload has been issued, and these old requests
	// can't be properly fulfilled. This isn't an unexpected
	// error but a normal part of the missing deps discovery flow
	return
	}
	if (e?.code === ERR_CLOSED_SERVER) {
	// Skip if response has already been sent
	if (!res.writableEnded) {
	res.statusCode = 504 // status code request timeout
	res.statusMessage = 'Outdated Request'
	res.end()
	}
	// We don't need to log an error in this case, the request
	// is outdated because new dependencies were discovered and
	// the new pre-bundle dependencies have changed.
	// A full-page reload has been issued, and these old requests
	// can't be properly fulfilled. This isn't an unexpected
	// error but a normal part of the missing deps discovery flow
	return
	}
	if (e?.code === ERR_FILE_NOT_FOUND_IN_OPTIMIZED_DEP_DIR) {
	// Skip if response has already been sent
	if (!res.writableEnded) {
	res.statusCode = 404
	res.end()
	}
	server.config.logger.warn(colors.yellow(e.message))
	return
	}
	if (e?.code === ERR_LOAD_URL) {
	// Let other middleware handle if we can't load the url via transformRequest
	return next()
	}

[patak-dev]

I think we could keep the ERR_OPTIMIZE_DEPS_PROCESSING_ERROR, but ya... I see your point about ERR_OUTDATED_OPTIMIZED_DEP. Ok, let's merge this version first and evaluate our options once we have real bug reports.

[cpojer]

Thank you 🙇‍♂️ Looking forward to trying this out.