Skip to content

Commit 90bbac4

Browse files
authored
fix(edge): error handling for edge route and middleware is inconsistent (vercel#38401)
## What’s in there? This PR brings more consistency in how errors and warnings are reported when running code in the Edge Runtime: - Dynamic code evaluation (`eval()`, `new Function()`, `WebAssembly.instantiate()`, `WebAssembly.compile()`…) - Usage of Node.js global APIs (`BroadcastChannel`, `Buffer`, `TextDecoderStream`, `setImmediate()`...) - Usage of Node.js modules (`fs`, `path`, `child_process`…) The new error messages should mention *Edge Runtime* instead of *Middleware*, so they are valid in both cases. It also fixes a bug where the process polyfill would issue a warning for `process.cwd` (which is `undefined` but legit). Now, one has to invoke the function `process.cwd()` to trigger the error. It finally fixes the react-dev-overlay, where links from middleware and Edge API route files could not be opened because of the `(middleware)/` prefix in their name. About the later, please note that we can’t easily remove the prefix or change it for Edge API routes. It comes from the Webpack layer, which is the same for both. We may consider renaming it to *edge* instead in the future. ## How to test? These changes are almost fully covered with tests: ```bash pnpm testheadless --testPathPattern runtime-dynamic pnpm testheadless --testPathPattern runtime-with-node pnpm testheadless --testPathPattern runtime-module pnpm testheadless --testPathPattern middleware-dev-errors ``` To try them out manually, you can write a middleware and Edge route files like these: ```jsx // middleware.js import { NextResponse } from 'next/server' import { basename } from 'path' export default async function middleware() { eval('2+2') setImmediate(() => {}) basename() return NextResponse.next() } export const config = { matcher: '/' } ``` ```jsx // pages/api/route.js import { basename } from 'path' export default async function handle() { eval('2+2') setImmediate(() => {}) basename() return Response.json({ ok: true }) } export const config = { runtime: 'experimental-edge' } ``` The expected behaviours are: - [x] dev, middleware/edge route is using a node.js module: error at runtime (logs + read-dev-overlay): ```bash error - (middleware)/pages/api/route.js (1:0) @ Object.handle [as handler] error - The edge runtime does not support Node.js 'path' module. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime > 1 | import { basename } from "path"; 2 | export default async function handle() { ``` - [x] build, middleware/edge route is using a node.js module: warning but succeeds ```bash warn - Compiled with warnings ./middleware.js A Node.js module is loaded ('path' at line 4) which is not supported in the Edge Runtime. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime ./pages/api/route.js A Node.js module is loaded ('path' at line 1) which is not supported in the Edge Runtime. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime ``` - [x] production, middleware/edge route is using a node.js module: error at runtime (logs + 500 error) ```bash Error: The edge runtime does not support Node.js 'path' module. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime at <unknown> (file:///Users/damien/dev/next.js/packages/next/server/web/sandbox/context.ts:149) ``` - [x] dev, middleware/edge route is using a node.js global API: error at runtime (logs + read-dev-overlay): ```bash error - (middleware)/pages/api/route.js (4:2) @ Object.handle [as handler] error - A Node.js API is used (setImmediate) which is not supported in the Edge Runtime. Learn more: https://nextjs.org/docs/api-reference/edge-runtime 2 | 3 | export default async function handle() { > 4 | setImmediate(() => {}) | ^ ``` - [x] build, middleware/edge route is using a node.js global API: warning but succeeds ```bash warn - Compiled with warnings ./middleware.js A Node.js API is used (setImmediate at line: 6) which is not supported in the Edge Runtime. Learn more: https://nextjs.org/docs/api-reference/edge-runtime ./pages/api/route.js A Node.js API is used (setImmediate at line: 3) which is not supported in the Edge Runtime. Learn more: https://nextjs.org/docs/api-reference/edge-runtime ``` - [x] production, middleware/edge route is using a node.js module: error at runtime (logs + 500 error) ```bash Error: A Node.js API is used (setImmediate) which is not supported in the Edge Runtime. Learn more: https://nextjs.org/docs/api-reference/edge-runtime at <unknown> (file:///Users/damien/dev/next.js/packages/next/server/web/sandbox/context.ts:330) ``` - [x] dev, middleware/edge route is loading dynamic code: warning at runtime (logs + read-dev-overlay) and request succeeds (we allow dynamic code in dev only): ```bash warn - (middleware)/middleware.js (7:2) @ Object.middleware [as handler] warn - Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime 5 | 6 | export default async function middleware() { > 7 | eval('2+2') ``` - [x] build, middleware/edge route is loading dynamic code: build fails with error: ```bash Failed to compile. ./middleware.js Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Edge Runtime Used by default ./pages/api/route.js Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Edge Runtime Used by default ``` ## Notes to reviewers Edge-related errors are either issued from `next/server/web/sandbox/context.ts` file (runtime errors) or from `next/build/webpack/plugins/middleware-plugin.ts` (webpack compilation). The previous implementation (I’m pleading guilty here) was way too verbose: some errors (Node.js global APIs like using `process.cwd()`) could be reported several times, and the previous mechanism to dedupe them (in middleware-plugin) wasn’t really effective. Changes in tests are due to renaming existing tests such as `test/integration/middleware-with-node.js-apis` into `test/integration/edge-runtime-with-node.js-apis`. I extended them to cover Edge API route. @hanneslund I’ve pushed the improvement you did in vercel#38289 one step further to avoid duplication.
1 parent 6486e12 commit 90bbac4

File tree

26 files changed

+403
-470
lines changed

26 files changed

+403
-470
lines changed

packages/next/build/webpack/plugins/middleware-plugin.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,10 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`,
380380
}
381381
}
382382
})
383-
registerUnsupportedApiHooks(parser, compilation)
383+
if (!dev) {
384+
// do not issue compilation warning on dev: invoking code will provide details
385+
registerUnsupportedApiHooks(parser, compilation)
386+
}
384387
}
385388
}
386389

@@ -445,7 +448,7 @@ function getExtractMetadata(params: {
445448

446449
compilation.errors.push(
447450
buildWebpackError({
448-
message: `Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Middleware ${entryName}${
451+
message: `Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Edge Runtime ${
449452
typeof buildInfo.usingIndirectEval !== 'boolean'
450453
? `\nUsed by ${Array.from(buildInfo.usingIndirectEval).join(
451454
', '

packages/next/client/dev/error-overlay/format-webpack-messages.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function formatMessage(message, verbose) {
4444
message.moduleTrace &&
4545
message.moduleTrace.filter(
4646
(trace) =>
47-
!/next-(middleware|client-pages|flight-(client|server))-loader\.js/.test(
47+
!/next-(middleware|client-pages|edge-function|flight-(client|server))-loader\.js/.test(
4848
trace.originName
4949
)
5050
)

packages/next/server/dev/next-dev-server.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type { RoutingItem } from '../base-server'
1313

1414
import crypto from 'crypto'
1515
import fs from 'fs'
16-
import chalk from 'next/dist/compiled/chalk'
1716
import { Worker } from 'next/dist/compiled/jest-worker'
1817
import findUp from 'next/dist/compiled/find-up'
1918
import { join as pathJoin, relative, resolve as pathResolve, sep } from 'path'
@@ -714,7 +713,12 @@ export default class DevServer extends Server {
714713
page: string
715714
}) {
716715
try {
717-
return super.runEdgeFunction(params)
716+
return super.runEdgeFunction({
717+
...params,
718+
onWarning: (warn) => {
719+
this.logErrorWithOriginalStack(warn, 'warning')
720+
},
721+
})
718722
} catch (error) {
719723
if (error instanceof DecodeError) {
720724
throw error
@@ -797,7 +801,9 @@ export default class DevServer extends Server {
797801
const frames = parseStack(err.stack!)
798802
const frame = frames.find(
799803
({ file }) =>
800-
!file?.startsWith('eval') && !file?.includes('web/adapter')
804+
!file?.startsWith('eval') &&
805+
!file?.includes('web/adapter') &&
806+
!file?.includes('sandbox/context')
801807
)!
802808

803809
if (frame.lineNumber && frame?.file) {
@@ -838,12 +844,9 @@ export default class DevServer extends Server {
838844
`${file} (${lineNumber}:${column}) @ ${methodName}`
839845
)
840846
if (src === 'edge-server') {
841-
console[type === 'warning' ? 'warn' : 'error'](
842-
`${(type === 'warning' ? chalk.yellow : chalk.red)(
843-
err.name
844-
)}: ${err.message}`
845-
)
846-
} else if (type === 'warning') {
847+
err = err.message
848+
}
849+
if (type === 'warning') {
847850
Log.warn(err)
848851
} else if (type) {
849852
Log.error(`${type}:`, err)

packages/next/server/next-server.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,12 +1247,7 @@ export default class NextNodeServer extends BaseServer {
12471247
body: originalBody?.cloneBodyStream(),
12481248
},
12491249
useCache: !this.nextConfig.experimental.runtime,
1250-
onWarning: (warning: Error) => {
1251-
if (params.onWarning) {
1252-
warning.message += ` "./${middlewareInfo.name}"`
1253-
params.onWarning(warning)
1254-
}
1255-
},
1250+
onWarning: params.onWarning,
12561251
})
12571252

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

@@ -1584,12 +1580,7 @@ export default class NextNodeServer extends BaseServer {
15841580
: requestToBodyStream(nodeReq.originalRequest),
15851581
},
15861582
useCache: !this.nextConfig.experimental.runtime,
1587-
onWarning: (_warning: Error) => {
1588-
// if (params.onWarning) {
1589-
// warning.message += ` "./${middlewareInfo.name}"`
1590-
// params.onWarning(warning)
1591-
// }
1592-
},
1583+
onWarning: params.onWarning,
15931584
})
15941585

15951586
params.res.statusCode = result.response.status

packages/next/server/web/sandbox/context.ts

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ async function createModuleContext(options: ModuleContextOptions) {
120120
if (!warnedEvals.has(key)) {
121121
const warning = getServerError(
122122
new Error(
123-
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Middleware`
123+
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime`
124124
),
125125
'edge-server'
126126
)
@@ -137,7 +137,7 @@ async function createModuleContext(options: ModuleContextOptions) {
137137
const key = fn.toString()
138138
if (!warnedWasmCodegens.has(key)) {
139139
const warning = getServerError(
140-
new Error(`Dynamic WASM code generation (e. g. 'WebAssembly.compile') not allowed in Middleware.
140+
new Error(`Dynamic WASM code generation (e. g. 'WebAssembly.compile') not allowed in Edge Runtime.
141141
Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation`),
142142
'edge-server'
143143
)
@@ -164,7 +164,7 @@ Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation
164164
const key = fn.toString()
165165
if (instantiatedFromBuffer && !warnedWasmCodegens.has(key)) {
166166
const warning = getServerError(
167-
new Error(`Dynamic WASM code generation ('WebAssembly.instantiate' with a buffer parameter) not allowed in Middleware.
167+
new Error(`Dynamic WASM code generation ('WebAssembly.instantiate' with a buffer parameter) not allowed in Edge Runtime.
168168
Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation`),
169169
'edge-server'
170170
)
@@ -240,7 +240,7 @@ Learn More: https://nextjs.org/docs/messages/middleware-dynamic-wasm-compilation
240240
}
241241

242242
for (const name of EDGE_UNSUPPORTED_NODE_APIS) {
243-
addStub(context, name, options)
243+
addStub(context, name)
244244
}
245245

246246
Object.assign(context, wasm)
@@ -285,9 +285,7 @@ function buildEnvironmentVariablesFrom(
285285
return env
286286
}
287287

288-
function createProcessPolyfill(
289-
options: Pick<ModuleContextOptions, 'env' | 'onWarning'>
290-
) {
288+
function createProcessPolyfill(options: Pick<ModuleContextOptions, 'env'>) {
291289
const env = buildEnvironmentVariablesFrom(options.env)
292290

293291
const processPolyfill = { env }
@@ -296,8 +294,13 @@ function createProcessPolyfill(
296294
if (key === 'env') continue
297295
Object.defineProperty(processPolyfill, key, {
298296
get() {
299-
emitWarning(`process.${key}`, options)
300-
return overridenValue[key]
297+
if (overridenValue[key]) {
298+
return overridenValue[key]
299+
}
300+
if (typeof (process as any)[key] === 'function') {
301+
return () => throwUnsupportedAPIError(`process.${key}`)
302+
}
303+
return undefined
301304
},
302305
set(value) {
303306
overridenValue[key] = value
@@ -308,35 +311,23 @@ function createProcessPolyfill(
308311
return processPolyfill
309312
}
310313

311-
const warnedAlready = new Set<string>()
312-
313-
function addStub(
314-
context: Primitives,
315-
name: string,
316-
contextOptions: Pick<ModuleContextOptions, 'onWarning'>
317-
) {
314+
function addStub(context: Primitives, name: string) {
318315
Object.defineProperty(context, name, {
319316
get() {
320-
emitWarning(name, contextOptions)
321-
return undefined
317+
return function () {
318+
throwUnsupportedAPIError(name)
319+
}
322320
},
323321
enumerable: false,
324322
})
325323
}
326324

327-
function emitWarning(
328-
name: string,
329-
contextOptions: Pick<ModuleContextOptions, 'onWarning'>
330-
) {
331-
if (!warnedAlready.has(name)) {
332-
const warning =
333-
new Error(`A Node.js API is used (${name}) which is not supported in the Edge Runtime.
325+
function throwUnsupportedAPIError(name: string) {
326+
const error =
327+
new Error(`A Node.js API is used (${name}) which is not supported in the Edge Runtime.
334328
Learn more: https://nextjs.org/docs/api-reference/edge-runtime`)
335-
warning.name = 'NodejsRuntimeApiInMiddlewareWarning'
336-
contextOptions.onWarning(warning)
337-
console.warn(warning.message)
338-
warnedAlready.add(name)
339-
}
329+
decorateServerError(error, 'edge-server')
330+
throw error
340331
}
341332

342333
function decorateUnhandledError(error: any) {

packages/next/server/web/sandbox/sandbox.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const ErrorSource = Symbol('SandboxError')
88
type RunnerFn = (params: {
99
name: string
1010
env: string[]
11-
onWarning: (warn: Error) => void
11+
onWarning?: (warn: Error) => void
1212
paths: string[]
1313
request: RequestData
1414
useCache: boolean
@@ -19,7 +19,7 @@ type RunnerFn = (params: {
1919
export const run = withTaggedErrors(async (params) => {
2020
const { runtime, evaluateInContext } = await getModuleContext({
2121
moduleName: params.name,
22-
onWarning: params.onWarning,
22+
onWarning: params.onWarning ?? (() => {}),
2323
useCache: params.useCache !== false,
2424
env: params.env,
2525
edgeFunctionEntry: params.edgeFunctionEntry,

packages/react-dev-overlay/src/middleware.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,11 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
344344
return res.end()
345345
}
346346

347-
const filePath = path.resolve(options.rootDirectory, frameFile)
347+
// frame files may start with their webpack layer, like (middleware)/middleware.js
348+
const filePath = path.resolve(
349+
options.rootDirectory,
350+
frameFile.replace(/^\([^)]+\)\//, '')
351+
)
348352
const fileExists = await fs.access(filePath, FS.F_OK).then(
349353
() => true,
350354
() => false

test/integration/middleware-dynamic-code/lib/utils.js renamed to test/integration/edge-runtime-dynamic-code/lib/utils.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
export const useCases = {
2+
eval: 'using-eval',
3+
noEval: 'not-using-eval',
4+
wasmCompile: 'using-webassembly-compile',
5+
wasmInstanciate: 'using-webassembly-instantiate',
6+
wasmBufferInstanciate: 'using-webassembly-instantiate-with-buffer',
7+
}
8+
19
export async function usingEval() {
210
// eslint-disable-next-line no-eval
311
return { value: eval('100') }

0 commit comments

Comments
 (0)