Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: await requests to before server restart #13262

Merged
merged 15 commits into from Jun 8, 2023
8 changes: 6 additions & 2 deletions packages/vite/src/node/plugins/importAnalysis.ts
Expand Up @@ -54,6 +54,7 @@ import {
shouldExternalizeForSSR,
} from '../ssr/ssrExternal'
import { getDepsOptimizer, optimizedDepNeedsInterop } from '../optimizer'
import { ERR_CLOSED_SERVER } from '../server/pluginContainer'
import { checkPublicFile } from './asset'
import {
ERR_OUTDATED_OPTIMIZED_DEP,
Expand Down Expand Up @@ -650,8 +651,11 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
// by the deps optimizer
const url = removeImportQuery(hmrUrl)
server.transformRequest(url, { ssr }).catch((e) => {
if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) {
// This are expected errors
if (
e?.code === ERR_OUTDATED_OPTIMIZED_DEP ||
e?.code === ERR_CLOSED_SERVER
) {
// these are expected errors
return
}
// Unexpected error, log the issue but avoid an unhandled exception
Expand Down
7 changes: 7 additions & 0 deletions packages/vite/src/node/server/index.ts
Expand Up @@ -467,6 +467,13 @@ export async function _createServer(
getDepsOptimizer(server.config, true)?.close(),
closeHttpServer(),
])
while (server._pendingRequests.size > 0) {
await Promise.allSettled(
[...server._pendingRequests.values()].map(
(pending) => pending.request,
),
)
}
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
bluwy marked this conversation as resolved.
Show resolved Hide resolved
server.resolvedUrls = null
},
printUrls() {
Expand Down
9 changes: 9 additions & 0 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Expand Up @@ -36,6 +36,8 @@ import {
unwrapId,
wrapId,
} from '../../utils'
import { ERR_CLOSED_SERVER } from '../pluginContainer'
import { ERR_OUTDATED_OPTIMIZED_DEP } from '../../plugins/optimizedDeps'
import { isCSSRequest } from '../../plugins/css'
import { checkPublicFile } from '../../plugins/asset'
import { getCodeWithSourcemap, injectSourcesContent } from '../sourcemap'
Expand Down Expand Up @@ -350,6 +352,13 @@ function preTransformRequest(server: ViteDevServer, url: string, base: string) {
// transform all url as non-ssr as html includes client-side assets only
server.transformRequest(url).catch((e) => {
// Unexpected error, log the issue but avoid an unhandled exception
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
if (
e?.code === ERR_OUTDATED_OPTIMIZED_DEP ||
e?.code === ERR_CLOSED_SERVER
) {
// these are expected errors
return
}
server.config.logger.error(e.message)
})
}
18 changes: 18 additions & 0 deletions packages/vite/src/node/server/middlewares/transform.ts
Expand Up @@ -35,6 +35,7 @@ import {
ERR_OPTIMIZE_DEPS_PROCESSING_ERROR,
ERR_OUTDATED_OPTIMIZED_DEP,
} from '../../plugins/optimizedDeps'
import { ERR_CLOSED_SERVER } from '../pluginContainer'
import { getDepsOptimizer } from '../../optimizer'

const debugCache = createDebugger('vite:cache')
Expand All @@ -44,6 +45,8 @@ const knownIgnoreList = new Set(['/', '/favicon.ico'])
export function transformMiddleware(
server: ViteDevServer,
): Connect.NextHandleFunction {
// cache moduleGraph, as server.moduleGraph references a clean graph that this middleware shouldn't
// read or modify after a restart
bluwy marked this conversation as resolved.
Show resolved Hide resolved
const {
config: { root, logger },
moduleGraph,
Expand Down Expand Up @@ -234,6 +237,21 @@ export function transformMiddleware(
// 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_LOAD_URL) {
// Let other middleware handle if we can't load the url via transformRequest
return next()
Expand Down
71 changes: 55 additions & 16 deletions packages/vite/src/node/server/pluginContainer.ts
Expand Up @@ -84,6 +84,18 @@ import { createPluginHookUtils } from '../plugins'
import { buildErrorMessage } from './middlewares/error'
import type { ModuleGraph } from './moduleGraph'

export const ERR_CLOSED_SERVER = 'ERR_CLOSED_SERVER'

export function throwClosedServerError(): never {
const err: any = new Error(
'The server is being restarted or closed. Request is outdated',
)
err.code = ERR_CLOSED_SERVER
// This error will be caught by the transform middleware that will
// send a 504 status code request timeout
throw err
}

export interface PluginContainerOptions {
cwd?: string
output?: OutputOptions
Expand Down Expand Up @@ -195,6 +207,7 @@ export async function createPluginContainer(
): Promise<void> {
const parallelPromises: Promise<unknown>[] = []
for (const plugin of getSortedPlugins(hookName)) {
// Don't throw here if closed, so buildEnd and closeBundle hooks can finish running
const hook = plugin[hookName]
if (!hook) continue
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Expand Down Expand Up @@ -571,12 +584,27 @@ export async function createPluginContainer(
}

let closed = false
const processesing = new Set<Promise<any>>()
function awaitPromiseOnClose<T>(promise: Promise<T>) {
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
processesing.add(promise)
return promise.finally(() => processesing.delete(promise))
}
function handleHookPromise<T>(maybePromise: undefined | T | Promise<T>) {
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
if (!(maybePromise as any)?.then) {
return maybePromise
}
return awaitPromiseOnClose(maybePromise as Promise<T>)
}

const container: PluginContainer = {
options: await (async () => {
let options = rollupOptions
for (const optionsHook of getSortedPluginHooks('options')) {
options = (await optionsHook.call(minimalContext, options)) || options
if (closed) throwClosedServerError()
options =
(await handleHookPromise(
optionsHook.call(minimalContext, options),
)) || options
}
if (options.acornInjectPlugins) {
parser = acorn.Parser.extend(
Expand All @@ -593,10 +621,12 @@ export async function createPluginContainer(
getModuleInfo,

async buildStart() {
await hookParallel(
'buildStart',
(plugin) => new Context(plugin),
() => [container.options as NormalizedInputOptions],
await handleHookPromise(
hookParallel(
'buildStart',
(plugin) => new Context(plugin),
() => [container.options as NormalizedInputOptions],
),
)
},

Expand All @@ -609,10 +639,10 @@ export async function createPluginContainer(
ctx._scan = scan
ctx._resolveSkips = skip
const resolveStart = debugResolve ? performance.now() : 0

let id: string | null = null
const partial: Partial<PartialResolvedId> = {}
for (const plugin of getSortedPlugins('resolveId')) {
if (closed) throwClosedServerError()
if (!plugin.resolveId) continue
if (skip?.has(plugin)) continue

Expand All @@ -623,13 +653,15 @@ export async function createPluginContainer(
'handler' in plugin.resolveId
? plugin.resolveId.handler
: plugin.resolveId
const result = await handler.call(ctx as any, rawId, importer, {
assertions: options?.assertions ?? {},
custom: options?.custom,
isEntry: !!options?.isEntry,
ssr,
scan,
})
const result = await handleHookPromise(
handler.call(ctx as any, rawId, importer, {
assertions: options?.assertions ?? {},
custom: options?.custom,
isEntry: !!options?.isEntry,
ssr,
scan,
}),
)
if (!result) continue

if (typeof result === 'string') {
Expand Down Expand Up @@ -675,11 +707,14 @@ export async function createPluginContainer(
const ctx = new Context()
ctx.ssr = !!ssr
for (const plugin of getSortedPlugins('load')) {
if (closed) throwClosedServerError()
if (!plugin.load) continue
ctx._activePlugin = plugin
const handler =
'handler' in plugin.load ? plugin.load.handler : plugin.load
const result = await handler.call(ctx as any, id, { ssr })
const result = await handleHookPromise(
handler.call(ctx as any, id, { ssr }),
)
if (result != null) {
if (isObject(result)) {
updateModuleInfo(id, result)
Expand All @@ -696,6 +731,7 @@ export async function createPluginContainer(
const ctx = new TransformContext(id, code, inMap as SourceMap)
ctx.ssr = !!ssr
for (const plugin of getSortedPlugins('transform')) {
if (closed) throwClosedServerError()
if (!plugin.transform) continue
ctx._activePlugin = plugin
ctx._activeId = id
Expand All @@ -707,7 +743,9 @@ export async function createPluginContainer(
? plugin.transform.handler
: plugin.transform
try {
result = await handler.call(ctx as any, code, id, { ssr })
result = await handleHookPromise(
handler.call(ctx as any, code, id, { ssr }),
)
} catch (e) {
ctx.error(e)
}
Expand Down Expand Up @@ -741,6 +779,8 @@ export async function createPluginContainer(

async close() {
if (closed) return
closed = true
await Promise.allSettled(Array.from(processesing))
const ctx = new Context()
await hookParallel(
'buildEnd',
Expand All @@ -752,7 +792,6 @@ export async function createPluginContainer(
() => ctx,
() => [],
)
closed = true
},
}

Expand Down
11 changes: 9 additions & 2 deletions packages/vite/src/node/server/transformRequest.ts
Expand Up @@ -20,6 +20,7 @@ import { checkPublicFile } from '../plugins/asset'
import { getDepsOptimizer } from '../optimizer'
import { applySourcemapIgnoreList, injectSourcesContent } from './sourcemap'
import { isFileServingAllowed } from './middlewares/static'
import { throwClosedServerError } from './pluginContainer'

export const ERR_LOAD_URL = 'ERR_LOAD_URL'
export const ERR_LOAD_PUBLIC_URL = 'ERR_LOAD_PUBLIC_URL'
Expand All @@ -46,6 +47,8 @@ export function transformRequest(
server: ViteDevServer,
options: TransformOptions = {},
): Promise<TransformResult | null> {
if (server._restartPromise) throwClosedServerError()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worth having a method like server.throwIfClosed() to encapsulate this logic and potentially avoid needing to access a private variable. not sure if you'd want to expose this externally, but if so then middlewares could call it if necessary

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting idea. I think we better explore it in a separate PR, and once we have a use case from a middleware to justify exposing the function. transformRequest is internal to vite so I think it is fine reading _restartPromise here for now, and we may find a better way to stop the execution later. Throwing here feels a bit hacky to me to expose it.


const cacheKey = (options.ssr ? 'ssr:' : options.html ? 'html:' : '') + url

// This module may get invalidated while we are processing it. For example
Expand Down Expand Up @@ -108,9 +111,8 @@ export function transformRequest(
timestamp,
abort: clearCache,
})
request.then(clearCache, clearCache)

return request
return request.finally(clearCache)
}

async function doTransform(
Expand Down Expand Up @@ -242,6 +244,9 @@ async function loadAndTransform(
err.code = isPublicFile ? ERR_LOAD_PUBLIC_URL : ERR_LOAD_URL
throw err
}

if (server._restartPromise) throwClosedServerError()

// ensure module in graph after successful load
const mod = await moduleGraph.ensureEntryFromUrl(url, ssr)
ensureWatchedFile(watcher, mod.file, root)
Expand Down Expand Up @@ -303,6 +308,8 @@ async function loadAndTransform(
}
}

if (server._restartPromise) throwClosedServerError()

const result =
ssr && !server.config.experimental.skipSsrTransform
? await server.ssrTransform(code, map as SourceMap, url, originalCode)
Expand Down