vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
67.5k stars 6.08k forks source link

Accept `async` middlewares #17421

Open benmccann opened 3 months ago

benmccann commented 3 months ago

Description

Most Vite middlewares need to be async especially because many of Vite's own APIs that you would be calling like ssrLoadModule are async. However, middlewares.use does not accept async methods. This makes for a pretty awkward dance to handle promise rejections and worse means that many users will overlook error handling altogether resulting in server crashes

Also, it would make sense for Vite's errorMiddleware to be called in the .catch when invoking the promise as the rejection handler, but that's impossible for users to do as it's not exported. However, this could be done internally in Vite.

Suggested solution

Override middlewares.use to accept async implementations or add a middlewares.useAsync method

Alternative

Additional context

No response

Validations

bluwy commented 3 months ago

Perhaps we could patch connect so that it handles .catch()?

IIUC here and here could have a trailing .catch(err => {}) which would call next(err), and it should properly work again. Not sure if they still accept changes upstream though given the time.

benmccann commented 3 months ago

Yeah, sadly there's been no commits to connect for three years, so I don't feel very optimistic we could get a change in.

benmccann commented 2 months ago

Express 5 has added support for this:

Request middleware and handlers that return rejected promises are now handled by forwarding the rejected value as an Error to the error handling middleware. This means that using async functions as middleware and handlers are easier than ever. When an error is thrown in an async function or a rejected promise is awaited inside an async function, those errors will be passed to the error handler as if calling next(err).

benmccann commented 2 months ago

I just tested and polka has support for it as well. If there's an unhandled error in an async handler then options.onError will be called. You can also see that its middleware returns a Promisable in its types.

Perhaps we should switch to polka 1.0.0-next.25 as SvelteKit has done since polka is both better maintained and supports async middleware.

benmccann commented 2 months ago

Ah, there's a PR to switch to polka. So this would be addressed by https://github.com/vitejs/vite/pull/17569