Skip to content

Commit

Permalink
fix: await requests to before server restart (#13262)
Browse files Browse the repository at this point in the history
  • Loading branch information
patak-dev authored Jun 8, 2023
1 parent 43cbbcf commit 0464398
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 24 deletions.
16 changes: 10 additions & 6 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
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 @@ -255,10 +256,10 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
// have been loaded so its entry is guaranteed in the module graph.
const importerModule = moduleGraph.getModuleById(importer)!
if (!importerModule) {
// When the server is restarted, the module graph is cleared, so we
// return without transforming. This request is no longer valid, a full reload
// is going to request this id again. Throwing an outdated error so we
// properly finish the request with a 504 sent to the browser.
// This request is no longer valid. It could happen for optimized deps
// requests. A full reload is going to request this id again.
// Throwing an outdated error so we properly finish the request with a
// 504 sent to the browser.
throwOutdatedRequest(importer)
}

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
11 changes: 11 additions & 0 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,17 @@ export async function _createServer(
getDepsOptimizer(server.config, true)?.close(),
closeHttpServer(),
])
// Await pending requests. We throw early in transformRequest
// and in hooks if the server is closing, so the import analysis
// plugin stops pre-transforming static imports and this block
// is resolved sooner.
while (server._pendingRequests.size > 0) {
await Promise.allSettled(
[...server._pendingRequests.values()].map(
(pending) => pending.request,
),
)
}
server.resolvedUrls = null
},
printUrls() {
Expand Down
9 changes: 9 additions & 0 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
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 @@ -349,6 +351,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) => {
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
server.config.logger.error(e.message)
})
Expand Down
16 changes: 16 additions & 0 deletions packages/vite/src/node/server/middlewares/transform.ts
Original file line number Diff line number Diff line change
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 Down Expand Up @@ -234,6 +235,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
70 changes: 54 additions & 16 deletions packages/vite/src/node/server/pluginContainer.ts
Original file line number Diff line number Diff line change
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,26 @@ export async function createPluginContainer(
}

let closed = false
const processesing = new Set<Promise<any>>()
// keeps track of hook promises so that we can wait for them all to finish upon closing the server
function handleHookPromise<T>(maybePromise: undefined | T | Promise<T>) {
if (!(maybePromise as any)?.then) {
return maybePromise
}
const promise = maybePromise as Promise<T>
processesing.add(promise)
return promise.finally(() => processesing.delete(promise))
}

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 +620,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 +638,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 +652,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 +706,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 +730,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 +742,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 +778,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 +791,6 @@ export async function createPluginContainer(
() => ctx,
() => [],
)
closed = true
},
}

Expand Down
11 changes: 9 additions & 2 deletions packages/vite/src/node/server/transformRequest.ts
Original file line number Diff line number Diff line change
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()

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 @@ -253,6 +255,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
mod ??= await moduleGraph._ensureEntryFromUrl(url, ssr, undefined, resolved)
ensureWatchedFile(watcher, mod.file, root)
Expand Down Expand Up @@ -314,6 +319,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

0 comments on commit 0464398

Please sign in to comment.