archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixes for middleware compilation errors (#37984)

Browse source 
* Refactor `runDependingOnPageType`

* Throw specific error when the compiled middleware cannot be found

* Do not render `MiddlewareNotFoundError` on dev

* Allow to invalidate compilers by type

* Show compile errors when middleware fails to build

* Add tests
This commit is contained in:
Javi Velasco 2022-06-24 20:50:49 +02:00 committed by GitHub
parent a590c4e7f3
commit a8757135e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 254 additions and 87 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
packages/next/build/entries.ts
View file

@ -442,25 +442,24 @@ export function runDependingOnPageType<T>(params: {
pageRuntime: PageRuntime
}) {
if (isMiddlewareFile(params.page)) {
return [params.onEdgeServer()]
return { edgeServer: params.onEdgeServer() }
} else if (params.page.match(API_ROUTE)) {
return params.pageRuntime === 'edge'
? [params.onEdgeServer()]
: [params.onServer()]
? { edgeServer: params.onEdgeServer() }
: { server: params.onServer() }
} else if (params.page === '/_document') {
return [params.onServer()]
return { server: params.onServer() }
} else if (
params.page === '/_app' ||
params.page === '/_error' ||
params.page === '/404' ||
params.page === '/500'
) {
return [params.onClient(), params.onServer()]
return { client: params.onClient(), server: params.onServer() }
} else {
return [
params.onClient(),
params.pageRuntime === 'edge' ? params.onEdgeServer() : params.onServer(),
]
return params.pageRuntime === 'edge'
? { client: params.onClient(), edgeServer: params.onEdgeServer() }
: { client: params.onClient(), server: params.onServer() }
}
}

43
packages/next/build/output/index.ts
View file

@ -39,7 +39,7 @@ export type AmpPageStatus = {
type BuildStatusStore = {
client: WebpackStatus
server: WebpackStatus
edgeServer?: WebpackStatus
edgeServer: WebpackStatus
trigger: string | undefined
amp: AmpPageStatus
}
@ -122,11 +122,10 @@ buildStore.subscribe((state) => {
clientWasLoading = (!buildWasDone && clientWasLoading) || client.loading
serverWasLoading = (!buildWasDone && serverWasLoading) || server.loading
edgeServerWasLoading =
(!buildWasDone && serverWasLoading) || !!edgeServer?.loading
(!buildWasDone && edgeServerWasLoading) || edgeServer.loading
buildWasDone = false
return
}
if (edgeServer?.loading) return
buildWasDone = true
@ -145,7 +144,7 @@ buildStore.subscribe((state) => {
(edgeServerWasLoading ? edgeServer?.modules || 0 : 0),
hasEdgeServer: !!edgeServer,
}
if (client.errors) {
if (client.errors && clientWasLoading) {
// Show only client errors
consoleStore.setState(
{
@ -155,8 +154,7 @@ buildStore.subscribe((state) => {
} as OutputState,
true
)
} else if (server.errors) {
// Show only server errors
} else if (server.errors && serverWasLoading) {
consoleStore.setState(
{
...partialState,
@ -165,8 +163,7 @@ buildStore.subscribe((state) => {
} as OutputState,
true
)
} else if (edgeServer && edgeServer.errors) {
// Show only edge server errors
} else if (edgeServer.errors && edgeServerWasLoading) {
consoleStore.setState(
{
...partialState,
@ -180,7 +177,7 @@ buildStore.subscribe((state) => {
const warnings = [
...(client.warnings || []),
...(server.warnings || []),
...((edgeServer && edgeServer.warnings) || []),
...(edgeServer.warnings || []),
].concat(formatAmpMessages(amp) || [])
consoleStore.setState(
@ -236,13 +233,13 @@ export function watchCompilers(
buildStore.setState({
client: { loading: true },
server: { loading: true },
edgeServer: edgeServer ? { loading: true } : undefined,
edgeServer: { loading: true },
trigger: 'initial',
})
function tapCompiler(
key: string,
compiler: any,
key: 'client' | 'server' | 'edgeServer',
compiler: webpack5.Compiler,
onEvent: (status: WebpackStatus) => void
) {
compiler.hooks.invalid.tap(`NextJsInvalid-${key}`, () => {
@ -272,7 +269,11 @@ export function watchCompilers(
}
tapCompiler('client', client, (status) => {
if (!status.loading && !buildStore.getState().server.loading) {
if (
!status.loading &&
!buildStore.getState().server.loading &&
!buildStore.getState().edgeServer.loading
) {
buildStore.setState({
client: status,
trigger: undefined,
@ -284,7 +285,11 @@ export function watchCompilers(
}
})
tapCompiler('server', server, (status) => {
if (!status.loading && !buildStore.getState().client.loading) {
if (
!status.loading &&
!buildStore.getState().client.loading &&
!buildStore.getState().edgeServer.loading
) {
buildStore.setState({
server: status,
trigger: undefined,
@ -295,14 +300,22 @@ export function watchCompilers(
})
}
})
if (edgeServer) {
tapCompiler('edgeServer', edgeServer, (status) => {
if (
!status.loading &&
!buildStore.getState().client.loading &&
!buildStore.getState().server.loading
) {
buildStore.setState({
edgeServer: status,
trigger: undefined,
})
} else {
buildStore.setState({
edgeServer: status,
})
}
})
previousClient = client
previousServer = server

130
packages/next/server/dev/hot-middleware.ts
View file

@ -23,19 +23,21 @@
// SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
import type { webpack5 as webpack } from 'next/dist/compiled/webpack/webpack'
import type ws from 'ws'
import { isMiddlewareFilename } from '../../build/utils'
import { nonNullable } from '../../lib/non-nullable'
export class WebpackHotMiddleware {
eventStream: EventStream
latestStats: webpack.Stats | null
clientLatestStats: webpack.Stats | null
clientLatestStats: { ts: number; stats: webpack.Stats } | null
middlewareLatestStats: { ts: number; stats: webpack.Stats } | null
serverLatestStats: { ts: number; stats: webpack.Stats } | null
closed: boolean
serverError: boolean
constructor(compilers: webpack.Compiler[]) {
this.eventStream = new EventStream()
this.latestStats = null
this.clientLatestStats = null
this.serverError = false
this.middlewareLatestStats = null
this.serverLatestStats = null
this.closed = false
compilers[0].hooks.invalid.tap(
@ -43,57 +45,91 @@ export class WebpackHotMiddleware {
this.onClientInvalid
)
compilers[0].hooks.done.tap('webpack-hot-middleware', this.onClientDone)
compilers[1].hooks.invalid.tap(
'webpack-hot-middleware',
this.onServerInvalid
)
compilers[1].hooks.done.tap('webpack-hot-middleware', this.onServerDone)
compilers[2].hooks.done.tap('webpack-hot-middleware', this.onEdgeServerDone)
compilers[2].hooks.invalid.tap(
'webpack-hot-middleware',
this.onEdgeServerInvalid
)
}
onClientInvalid = () => {
if (this.closed || this.serverLatestStats?.stats.hasErrors()) return
this.eventStream.publish({ action: 'building' })
}
onClientDone = (statsResult: webpack.Stats) => {
this.clientLatestStats = { ts: Date.now(), stats: statsResult }
if (this.closed || this.serverLatestStats?.stats.hasErrors()) return
this.publishStats('built', statsResult)
}
onServerInvalid = () => {
if (!this.serverError) return
if (!this.serverLatestStats?.stats.hasErrors()) return
this.serverLatestStats = null
if (this.clientLatestStats?.stats) {
this.publishStats('built', this.clientLatestStats.stats)
}
}
this.serverError = false
if (this.clientLatestStats) {
this.latestStats = this.clientLatestStats
this.publishStats('built', this.latestStats)
}
}
onClientInvalid = () => {
if (this.closed || this.serverError) return
this.latestStats = null
this.eventStream.publish({ action: 'building' })
}
onServerDone = (statsResult: webpack.Stats) => {
if (this.closed) return
// Keep hold of latest stats so they can be propagated to new clients
// this.latestStats = statsResult
// this.publishStats('built', this.latestStats)
this.serverError = statsResult.hasErrors()
if (this.serverError) {
this.latestStats = statsResult
this.publishStats('built', this.latestStats)
if (statsResult.hasErrors()) {
this.serverLatestStats = { ts: Date.now(), stats: statsResult }
this.publishStats('built', statsResult)
}
}
onClientDone = (statsResult: webpack.Stats) => {
this.clientLatestStats = statsResult
if (this.closed || this.serverError) return
// Keep hold of latest stats so they can be propagated to new clients
this.latestStats = statsResult
this.publishStats('built', this.latestStats)
onEdgeServerInvalid = () => {
if (!this.middlewareLatestStats?.stats.hasErrors()) return
this.middlewareLatestStats = null
if (this.clientLatestStats?.stats) {
this.publishStats('built', this.clientLatestStats.stats)
}
}
onEdgeServerDone = (statsResult: webpack.Stats) => {
if (!isMiddlewareStats(statsResult)) {
this.onServerDone(statsResult)
return
}
if (statsResult.hasErrors()) {
this.middlewareLatestStats = { ts: Date.now(), stats: statsResult }
this.publishStats('built', statsResult)
}
}
/**
* To sync we use the most recent stats but also we append middleware
* errors. This is because it is possible that middleware fails to compile
* and we still want to show the client overlay with the error while
* the error page should be rendered just fine.
*/
onHMR = (client: ws) => {
if (this.closed) return
this.eventStream.handler(client)
if (this.latestStats) {
// Explicitly not passing in `log` fn as we don't want to log again on
// the server
this.publishStats('sync', this.latestStats)
const [latestStats] = [this.clientLatestStats, this.serverLatestStats]
.filter(nonNullable)
.sort((statsA, statsB) => statsB.ts - statsA.ts)
if (latestStats?.stats) {
const stats = statsToJson(latestStats.stats)
const middlewareStats = statsToJson(this.middlewareLatestStats?.stats)
this.eventStream.publish({
action: 'sync',
hash: stats.hash,
errors: [...(stats.errors || []), ...(middlewareStats.errors || [])],
warnings: [
...(stats.warnings || []),
...(middlewareStats.warnings || []),
],
})
}
}
@ -158,3 +194,23 @@ class EventStream {
})
}
}
function isMiddlewareStats(stats: webpack.Stats) {
for (const key of stats.compilation.entrypoints.keys()) {
if (isMiddlewareFilename(key)) {
return true
}
}
return false
}
function statsToJson(stats?: webpack.Stats | null) {
if (!stats) return {}
return stats.toJson({
all: false,
errors: true,
hash: true,
warnings: true,
})
}

1
packages/next/server/dev/hot-reloader.ts
View file

@ -901,7 +901,6 @@ export default class HotReloader {
this.onDemandEntries = onDemandEntryHandler({
multiCompiler,
watcher: this.watcher,
pagesDir: this.pagesDir,
appDir: this.appDir,
rootDir: this.dir,

11
packages/next/server/dev/next-dev-server.ts
View file

@ -48,7 +48,7 @@ import { findPageFile } from '../lib/find-page-file'
import { getNodeOptionsWithoutInspect } from '../lib/utils'
import { withCoalescedInvoke } from '../../lib/coalesced-function'
import { loadDefaultErrorComponents } from '../load-components'
import { DecodeError } from '../../shared/lib/utils'
import { DecodeError, MiddlewareNotFoundError } from '../../shared/lib/utils'
import {
createOriginalStackFrame,
getErrorSource,
@ -675,7 +675,16 @@ export default class DevServer extends Server {
if (error instanceof DecodeError) {
throw error
}
/**
* We only log the error when it is not a MiddlewareNotFound error as
* in that case we should be already displaying a compilation error
* which is what makes the module not found.
*/
if (!(error instanceof MiddlewareNotFoundError)) {
this.logErrorWithOriginalStack(error)
}
const err = getProperError(error)
;(err as any).middleware = true
const { request, response, parsedUrl } = params

35
packages/next/server/dev/on-demand-entry-handler.ts
View file

@ -66,7 +66,6 @@ export function onDemandEntryHandler({
pagesDir,
rootDir,
appDir,
watcher,
}: {
maxInactiveAge: number
multiCompiler: webpack.MultiCompiler
@ -75,9 +74,8 @@ export function onDemandEntryHandler({
pagesDir: string
rootDir: string
appDir?: string
watcher: any
}) {
invalidator = new Invalidator(watcher)
invalidator = new Invalidator(multiCompiler)
const doneCallbacks: EventEmitter | null = new EventEmitter()
const lastClientAccessPages = ['']
@ -245,7 +243,7 @@ export function onDemandEntryHandler({
nextConfig,
})
const promises = runDependingOnPageType({
const result = runDependingOnPageType({
page: pagePathData.page,
pageRuntime: staticInfo.runtime,
onClient: () => addPageEntry('client'),
@ -253,13 +251,14 @@ export function onDemandEntryHandler({
onEdgeServer: () => addPageEntry('edge-server'),
})
const promises = Object.values(result)
if (entryAdded) {
reportTrigger(
!clientOnly && promises.length > 1
? `${pagePathData.page} (client and server)`
: pagePathData.page
)
invalidator.invalidate()
invalidator.invalidate(Object.keys(result))
}
return Promise.all(promises)
@ -315,18 +314,18 @@ function disposeInactiveEntries(
// Make sure only one invalidation happens at a time
// Otherwise, webpack hash gets changed and it'll force the client to reload.
class Invalidator {
private watcher: any
private multiCompiler: webpack.MultiCompiler
private building: boolean
public rebuildAgain: boolean
constructor(watcher: any) {
this.watcher = watcher
constructor(multiCompiler: webpack.MultiCompiler) {
this.multiCompiler = multiCompiler
// contains an array of types of compilers currently building
this.building = false
this.rebuildAgain = false
}
invalidate() {
invalidate(keys: string[] = []) {
// If there's a current build is processing, we won't abort it by invalidating.
// (If aborted, it'll cause a client side hard reload)
// But let it to invalidate just after the completion.
@ -337,7 +336,23 @@ class Invalidator {
}
this.building = true
this.watcher.invalidate()
if (!keys || keys.length === 0) {
this.multiCompiler.compilers[0].watching.invalidate()
this.multiCompiler.compilers[1].watching.invalidate()
this.multiCompiler.compilers[2].watching.invalidate()
return
}
for (const key of keys) {
if (key === 'client') {
this.multiCompiler.compilers[0].watching.invalidate()
} else if (key === 'server') {
this.multiCompiler.compilers[1].watching.invalidate()
} else if (key === 'edgeServer') {
this.multiCompiler.compilers[2].watching.invalidate()
}
}
}
startBuilding() {

31
packages/next/server/next-server.ts
View file

@ -2,7 +2,12 @@ import './node-polyfill-fetch'
import './node-polyfill-web-streams'
import type { Route } from './router'
import { CacheFs, DecodeError, PageNotFoundError } from '../shared/lib/utils'
import {
CacheFs,
DecodeError,
PageNotFoundError,
MiddlewareNotFoundError,
} from '../shared/lib/utils'
import type { MiddlewareManifest } from '../build/webpack/plugins/middleware-plugin'
import type RenderResult from './render-result'
import type { FetchEventResult } from './web/types'
@ -1091,15 +1096,19 @@ export default class NextNodeServer extends BaseServer {
try {
foundPage = denormalizePagePath(normalizePagePath(params.page))
} catch (err) {
throw new PageNotFoundError(params.page)
return null
}
let pageInfo = params.middleware
? manifest.middleware[foundPage]
: manifest.functions[foundPage]
if (!pageInfo) {
if (!params.middleware) {
throw new PageNotFoundError(foundPage)
}
return null
}
return {
name: pageInfo.name,
@ -1121,14 +1130,8 @@ export default class NextNodeServer extends BaseServer {
pathname: string,
_isSSR?: boolean
): Promise<boolean> {
try {
return (
this.getEdgeFunctionInfo({ page: pathname, middleware: true }).paths
.length > 0
)
} catch (_) {}
return false
const info = this.getEdgeFunctionInfo({ page: pathname, middleware: true })
return Boolean(info && info.paths.length > 0)
}
/**
@ -1209,6 +1212,10 @@ export default class NextNodeServer extends BaseServer {
middleware: !middleware.ssr,
})
if (!middlewareInfo) {
throw new MiddlewareNotFoundError()
}
result = await run({
name: middlewareInfo.name,
paths: middlewareInfo.paths,
@ -1524,6 +1531,10 @@ export default class NextNodeServer extends BaseServer {
return null
}
if (!middlewareInfo) {
return null
}
// For middleware to "fetch" we must always provide an absolute URL
const url = getRequestMeta(params.req, '__NEXT_INIT_URL')!
if (!url.startsWith('http')) {

9
packages/next/shared/lib/utils.ts
View file

@ -413,6 +413,15 @@ export class PageNotFoundError extends Error {
}
}
export class MiddlewareNotFoundError extends Error {
code: string
constructor() {
super()
this.code = 'ENOENT'
this.message = `Cannot find the middleware module`
}
}
export interface CacheFs {
readFile(f: string): Promise<string>
readFileSync(f: string): string

56
test/integration/middleware-dev-errors/test/index.test.js
View file

@ -1,4 +1,5 @@
import {
check,
fetchViaHTTP,
File,
findPort,
@ -241,4 +242,59 @@ describe('Middleware development errors', () => {
expect(await browser.elementByCss('#page-title')).toBeTruthy()
})
})
describe('when there is a compilation error from boot', () => {
beforeEach(() => {
context.middleware.write(`export default function () }`)
})
it('logs the error correctly', async () => {
await fetchViaHTTP(context.appPort, '/')
expect(context.logs.output).toContain(`Expected '{', got '}'`)
expect(context.logs.output.split(`Expected '{', got '}'`).length).toEqual(
2
)
})
it('renders the error correctly and recovers', async () => {
const browser = await webdriver(context.appPort, '/')
expect(await hasRedbox(browser, true)).toBe(true)
expect(
await browser
.elementByCss('#nextjs__container_build_error_label')
.text()
).toEqual('Failed to compile')
context.middleware.write(`export default function () {}`)
await hasRedbox(browser, false)
expect(await browser.elementByCss('#page-title')).toBeTruthy()
})
})
describe('when there is a compilation error after boot', () => {
beforeEach(() => {
context.middleware.write(`export default function () {}`)
})
it('logs the error correctly', async () => {
await fetchViaHTTP(context.appPort, '/')
context.middleware.write(`export default function () }`)
await check(() => {
expect(context.logs.output).toContain(`Expected '{', got '}'`)
expect(
context.logs.output.split(`Expected '{', got '}'`).length
).toEqual(2)
return 'success'
}, 'success')
})
it('renders the error correctly and recovers', async () => {
const browser = await webdriver(context.appPort, '/')
expect(await hasRedbox(browser, false)).toBe(false)
context.middleware.write(`export default function () }`)
expect(await hasRedbox(browser, true)).toBe(true)
context.middleware.write(`export default function () {}`)
expect(await hasRedbox(browser, false)).toBe(false)
expect(await browser.elementByCss('#page-title')).toBeTruthy()
})
})
})