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(edge): error handling for edge route and middleware is inconsistent #38401

Merged
merged 10 commits into from Jul 21, 2022
7 changes: 5 additions & 2 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
Expand Up @@ -380,7 +380,10 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`,
}
}
})
registerUnsupportedApiHooks(parser, compilation)
if (!dev) {
// do not issue compilation warning on dev: invoking code will provide details
registerUnsupportedApiHooks(parser, compilation)
}
}
}

Expand Down Expand Up @@ -445,7 +448,7 @@ function getExtractMetadata(params: {

compilation.errors.push(
buildWebpackError({
message: `Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Middleware ${entryName}${
message: `Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Edge Runtime ${
typeof buildInfo.usingIndirectEval !== 'boolean'
? `\nUsed by ${Array.from(buildInfo.usingIndirectEval).join(
', '
Expand Down
Expand Up @@ -44,7 +44,7 @@ function formatMessage(message, verbose) {
message.moduleTrace &&
message.moduleTrace.filter(
(trace) =>
!/next-(middleware|client-pages|flight-(client|server))-loader\.js/.test(
!/next-(middleware|client-pages|edge-function|flight-(client|server))-loader\.js/.test(
trace.originName
)
)
Expand Down
21 changes: 12 additions & 9 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -13,7 +13,6 @@ import type { RoutingItem } from '../base-server'

import crypto from 'crypto'
import fs from 'fs'
import chalk from 'next/dist/compiled/chalk'
import { Worker } from 'next/dist/compiled/jest-worker'
import findUp from 'next/dist/compiled/find-up'
import { join as pathJoin, relative, resolve as pathResolve, sep } from 'path'
Expand Down Expand Up @@ -714,7 +713,12 @@ export default class DevServer extends Server {
page: string
}) {
try {
return super.runEdgeFunction(params)
return super.runEdgeFunction({
...params,
onWarning: (warn) => {
this.logErrorWithOriginalStack(warn, 'warning')
},
})
} catch (error) {
if (error instanceof DecodeError) {
throw error
Expand Down Expand Up @@ -797,7 +801,9 @@ export default class DevServer extends Server {
const frames = parseStack(err.stack!)
const frame = frames.find(
({ file }) =>
!file?.startsWith('eval') && !file?.includes('web/adapter')
!file?.startsWith('eval') &&
!file?.includes('web/adapter') &&
!file?.includes('sandbox/context')
)!

if (frame.lineNumber && frame?.file) {
Expand Down Expand Up @@ -838,12 +844,9 @@ export default class DevServer extends Server {
`${file} (${lineNumber}:${column}) @ ${methodName}`
)
if (src === 'edge-server') {
console[type === 'warning' ? 'warn' : 'error'](
`${(type === 'warning' ? chalk.yellow : chalk.red)(
err.name
)}: ${err.message}`
)
} else if (type === 'warning') {
err = err.message
}
if (type === 'warning') {
Log.warn(err)
} else if (type) {
Log.error(`${type}:`, err)
Expand Down
15 changes: 3 additions & 12 deletions packages/next/server/next-server.ts
Expand Up @@ -1247,12 +1247,7 @@ export default class NextNodeServer extends BaseServer {
body: originalBody?.cloneBodyStream(),
},
useCache: !this.nextConfig.experimental.runtime,
onWarning: (warning: Error) => {
if (params.onWarning) {
warning.message += ` "./${middlewareInfo.name}"`
params.onWarning(warning)
}
},
onWarning: params.onWarning,
})

for (let [key, value] of result.response.headers) {
Expand Down Expand Up @@ -1531,6 +1526,7 @@ export default class NextNodeServer extends BaseServer {
query: ParsedUrlQuery
params: Params | undefined
page: string
onWarning?: (warning: Error) => void
}): Promise<FetchEventResult | null> {
let middlewareInfo: ReturnType<typeof this.getEdgeFunctionInfo> | undefined

Expand Down Expand Up @@ -1584,12 +1580,7 @@ export default class NextNodeServer extends BaseServer {
: requestToBodyStream(nodeReq.originalRequest),
},
useCache: !this.nextConfig.experimental.runtime,
onWarning: (_warning: Error) => {
// if (params.onWarning) {
// warning.message += ` "./${middlewareInfo.name}"`
// params.onWarning(warning)
// }
},
onWarning: params.onWarning,
})

params.res.statusCode = result.response.status
Expand Down
51 changes: 21 additions & 30 deletions packages/next/server/web/sandbox/context.ts
Expand Up @@ -120,7 +120,7 @@ async function createModuleContext(options: ModuleContextOptions) {
if (!warnedEvals.has(key)) {
const warning = getServerError(
new Error(
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Middleware`
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime`
),
'edge-server'
)
Expand All @@ -137,7 +137,7 @@ async function createModuleContext(options: ModuleContextOptions) {
const key = fn.toString()
if (!warnedWasmCodegens.has(key)) {
const warning = getServerError(
new Error(`Dynamic WASM code generation (e. g. 'WebAssembly.compile') not allowed in Middleware.
new Error(`Dynamic WASM code generation (e. g. 'WebAssembly.compile') not allowed in Edge Runtime.
Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation`),
'edge-server'
)
Expand All @@ -164,7 +164,7 @@ Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation
const key = fn.toString()
if (instantiatedFromBuffer && !warnedWasmCodegens.has(key)) {
const warning = getServerError(
new Error(`Dynamic WASM code generation ('WebAssembly.instantiate' with a buffer parameter) not allowed in Middleware.
new Error(`Dynamic WASM code generation ('WebAssembly.instantiate' with a buffer parameter) not allowed in Edge Runtime.
Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation`),
'edge-server'
)
Expand Down Expand Up @@ -240,7 +240,7 @@ Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation
}

for (const name of EDGE_UNSUPPORTED_NODE_APIS) {
addStub(context, name, options)
addStub(context, name)
}

Object.assign(context, wasm)
Expand Down Expand Up @@ -285,9 +285,7 @@ function buildEnvironmentVariablesFrom(
return env
}

function createProcessPolyfill(
options: Pick<ModuleContextOptions, 'env' | 'onWarning'>
) {
function createProcessPolyfill(options: Pick<ModuleContextOptions, 'env'>) {
const env = buildEnvironmentVariablesFrom(options.env)

const processPolyfill = { env }
Expand All @@ -296,8 +294,13 @@ function createProcessPolyfill(
if (key === 'env') continue
Object.defineProperty(processPolyfill, key, {
get() {
emitWarning(`process.${key}`, options)
return overridenValue[key]
if (overridenValue[key]) {
return overridenValue[key]
}
if (typeof (process as any)[key] === 'function') {
return () => throwUnsupportedAPIError(`process.${key}`)
}
return undefined
},
set(value) {
overridenValue[key] = value
Expand All @@ -308,35 +311,23 @@ function createProcessPolyfill(
return processPolyfill
}

const warnedAlready = new Set<string>()

function addStub(
context: Primitives,
name: string,
contextOptions: Pick<ModuleContextOptions, 'onWarning'>
) {
function addStub(context: Primitives, name: string) {
Object.defineProperty(context, name, {
get() {
emitWarning(name, contextOptions)
return undefined
return function () {
throwUnsupportedAPIError(name)
}
},
enumerable: false,
})
}

function emitWarning(
name: string,
contextOptions: Pick<ModuleContextOptions, 'onWarning'>
) {
if (!warnedAlready.has(name)) {
const warning =
new Error(`A Node.js API is used (${name}) which is not supported in the Edge Runtime.
function throwUnsupportedAPIError(name: string) {
const error =
new Error(`A Node.js API is used (${name}) which is not supported in the Edge Runtime.
Learn more: https://nextjs.org/docs/api-reference/edge-runtime`)
warning.name = 'NodejsRuntimeApiInMiddlewareWarning'
contextOptions.onWarning(warning)
console.warn(warning.message)
warnedAlready.add(name)
}
decorateServerError(error, 'edge-server')
throw error
}

function decorateUnhandledError(error: any) {
Expand Down
4 changes: 2 additions & 2 deletions packages/next/server/web/sandbox/sandbox.ts
Expand Up @@ -8,7 +8,7 @@ export const ErrorSource = Symbol('SandboxError')
type RunnerFn = (params: {
name: string
env: string[]
onWarning: (warn: Error) => void
onWarning?: (warn: Error) => void
paths: string[]
request: RequestData
useCache: boolean
Expand All @@ -19,7 +19,7 @@ type RunnerFn = (params: {
export const run = withTaggedErrors(async (params) => {
const { runtime, evaluateInContext } = await getModuleContext({
moduleName: params.name,
onWarning: params.onWarning,
onWarning: params.onWarning ?? (() => {}),
useCache: params.useCache !== false,
env: params.env,
edgeFunctionEntry: params.edgeFunctionEntry,
Expand Down
6 changes: 5 additions & 1 deletion packages/react-dev-overlay/src/middleware.ts
Expand Up @@ -344,7 +344,11 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
return res.end()
}

const filePath = path.resolve(options.rootDirectory, frameFile)
// frame files may start with their webpack layer, like (middleware)/middleware.js
const filePath = path.resolve(
options.rootDirectory,
frameFile.replace(/^\([^)]+\)\//, '')
)
const fileExists = await fs.access(filePath, FS.F_OK).then(
() => true,
() => false
Expand Down
@@ -1,3 +1,11 @@
export const useCases = {
eval: 'using-eval',
noEval: 'not-using-eval',
wasmCompile: 'using-webassembly-compile',
wasmInstanciate: 'using-webassembly-instantiate',
wasmBufferInstanciate: 'using-webassembly-instantiate-with-buffer',
}

export async function usingEval() {
// eslint-disable-next-line no-eval
return { value: eval('100') }
Expand Down
@@ -1,42 +1,47 @@
import { notUsingEval, usingEval } from './lib/utils'
import { NextResponse } from 'next/server'
import { useCases, notUsingEval, usingEval } from './lib/utils'
import {
usingWebAssemblyCompile,
usingWebAssemblyInstantiate,
usingWebAssemblyInstantiateWithBuffer,
} from './lib/wasm'

export async function middleware(request) {
if (request.nextUrl.pathname === '/using-eval') {
if (request.nextUrl.pathname === `/${useCases.eval}`) {
return new Response(null, {
headers: { data: JSON.stringify(await usingEval()) },
})
}

if (request.nextUrl.pathname === '/not-using-eval') {
if (request.nextUrl.pathname === `/${useCases.noEval}`) {
return new Response(null, {
headers: { data: JSON.stringify(await notUsingEval()) },
})
}

if (request.nextUrl.pathname === '/using-webassembly-compile') {
if (request.nextUrl.pathname === `/${useCases.wasmCompile}`) {
return new Response(null, {
headers: { data: JSON.stringify(await usingWebAssemblyCompile(9)) },
})
}

if (request.nextUrl.pathname === '/using-webassembly-instantiate') {
if (request.nextUrl.pathname === `/${useCases.wasmInstanciate}`) {
return new Response(null, {
headers: { data: JSON.stringify(await usingWebAssemblyInstantiate(9)) },
})
}

if (
request.nextUrl.pathname === '/using-webassembly-instantiate-with-buffer'
) {
if (request.nextUrl.pathname === `/${useCases.wasmBufferInstanciate}`) {
return new Response(null, {
headers: {
data: JSON.stringify(await usingWebAssemblyInstantiateWithBuffer(9)),
},
})
}

return NextResponse.next()
}

export const config = {
matcher: Object.values(useCases).map((route) => `/${route}`),
}
26 changes: 26 additions & 0 deletions test/integration/edge-runtime-dynamic-code/pages/api/route.js
@@ -0,0 +1,26 @@
import { useCases, notUsingEval, usingEval } from '../../lib/utils'
import {
usingWebAssemblyCompile,
usingWebAssemblyInstantiate,
usingWebAssemblyInstantiateWithBuffer,
} from '../../lib/wasm'

export default async function handler(request) {
const useCase = request.nextUrl.searchParams.get('case')

return Response.json(
useCase === useCases.eval
? await usingEval()
: useCase === useCases.noEval
? await notUsingEval()
: useCase === useCases.wasmCompile
? await usingWebAssemblyCompile(9)
: useCase === useCases.wasmInstanciate
? await usingWebAssemblyInstantiate(9)
: useCase === useCases.wasmBufferInstanciate
? await usingWebAssemblyInstantiateWithBuffer(9)
: { ok: true }
)
}

export const config = { runtime: 'experimental-edge' }