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

Resolve RSC / HTML rendering errors in error overlay #43332

Merged
merged 18 commits into from Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -39,6 +39,7 @@ export function getOriginalStackFrame(
const params = new URLSearchParams()
params.append('isServer', String(type === 'server'))
params.append('isEdgeServer', String(type === 'edge-server'))
params.append('isAppDirectory', 'true')
params.append('errorMessage', errorMessage)
for (const key in source) {
params.append(key, ((source as any)[key] ?? '').toString())
Expand Down
22 changes: 17 additions & 5 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -1053,18 +1053,24 @@ export default class DevServer extends Server {
({ file }) =>
!file?.startsWith('eval') &&
!file?.includes('web/adapter') &&
!file?.includes('sandbox/context')
!file?.includes('sandbox/context') &&
!file?.includes('<anonymous>')
)!

if (frame.lineNumber && frame?.file) {
const moduleId = frame.file!.replace(
/^(webpack-internal:\/\/\/|file:\/\/)/,
''
)
const modulePath = frame.file.replace(
/^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/,
''
)

const src = getErrorSource(err as Error)
const isEdgeCompiler = src === COMPILER_NAMES.edgeServer
const compilation = (
src === COMPILER_NAMES.edgeServer
isEdgeCompiler
? this.hotReloader?.edgeServerStats?.compilation
: this.hotReloader?.serverStats?.compilation
)!
Expand All @@ -1080,10 +1086,16 @@ export default class DevServer extends Server {
column: frame.column,
source,
frame,
modulePath: moduleId,
moduleId,
modulePath,
rootDirectory: this.dir,
errorMessage: err.message,
compilation,
serverCompilation: isEdgeCompiler
? undefined
: this.hotReloader?.serverStats?.compilation,
edgeCompilation: isEdgeCompiler
? this.hotReloader?.edgeServerStats?.compilation
: undefined,
})

if (originalFrame) {
Expand All @@ -1093,7 +1105,7 @@ export default class DevServer extends Server {
Log[type === 'warning' ? 'warn' : 'error'](
`${file} (${lineNumber}:${column}) @ ${methodName}`
)
if (src === COMPILER_NAMES.edgeServer) {
if (isEdgeCompiler) {
err = err.message
}
if (type === 'warning') {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/taskfile.js
Expand Up @@ -389,7 +389,7 @@ export async function ncc_next__react_dev_overlay(task, opts) {
precompiled: false,
packageName: '@next/react-dev-overlay',
externals: overlayExternals,
target: 'es5',
target: 'es2018',
})
.target('dist/compiled/@next/react-dev-overlay/dist')

Expand Down
145 changes: 111 additions & 34 deletions packages/react-dev-overlay/src/middleware.ts
Expand Up @@ -41,9 +41,10 @@ function getModuleById(
id: string | undefined,
compilation: webpack.Compilation
) {
return [...compilation.modules].find(
(searchModule) => getModuleId(compilation, searchModule) === id
)
return [...compilation.modules].find((searchModule) => {
const moduleId = getModuleId(compilation, searchModule)
return moduleId === id
})
}

function findModuleNotFoundFromError(errorMessage: string | undefined) {
Expand Down Expand Up @@ -112,45 +113,78 @@ async function findOriginalSourcePositionAndContent(
}

function findOriginalSourcePositionAndContentFromCompilation(
modulePath: string | undefined,
moduleId: string | undefined,
importedModule: string,
compilation: webpack.Compilation
) {
const module = getModuleById(modulePath, compilation)
const module = getModuleById(moduleId, compilation)
return module?.buildInfo?.importLocByPath?.get(importedModule) ?? null
}

export async function createOriginalStackFrame({
line,
column,
source,
moduleId,
modulePath,
rootDirectory,
frame,
errorMessage,
compilation,
clientCompilation,
serverCompilation,
edgeCompilation,
}: {
line: number
column: number | null
source: any
moduleId?: string
modulePath?: string
rootDirectory: string
frame: any
errorMessage?: string
compilation?: webpack.Compilation
clientCompilation?: webpack.Compilation
serverCompilation?: webpack.Compilation
edgeCompilation?: webpack.Compilation
}): Promise<OriginalStackFrameResponse | null> {
const moduleNotFound = findModuleNotFoundFromError(errorMessage)
const result =
moduleNotFound && compilation
? findOriginalSourcePositionAndContentFromCompilation(
modulePath,
moduleNotFound,
compilation
)
: await findOriginalSourcePositionAndContent(source, {
line,
column,
})
const result = await (async () => {
if (moduleNotFound) {
let moduleNotFoundResult = null

if (clientCompilation) {
moduleNotFoundResult =
findOriginalSourcePositionAndContentFromCompilation(
moduleId,
moduleNotFound,
clientCompilation
)
}

if (moduleNotFoundResult === null && serverCompilation) {
moduleNotFoundResult =
findOriginalSourcePositionAndContentFromCompilation(
moduleId,
moduleNotFound,
serverCompilation
)
}

if (moduleNotFoundResult === null && edgeCompilation) {
moduleNotFoundResult =
findOriginalSourcePositionAndContentFromCompilation(
moduleId,
moduleNotFound,
edgeCompilation
)
}

return moduleNotFoundResult
}
return await findOriginalSourcePositionAndContent(source, {
line,
column,
})
})()

if (result === null) {
return null
Expand All @@ -164,7 +198,12 @@ export async function createOriginalStackFrame({

const filePath = path.resolve(
rootDirectory,
modulePath || getSourcePath(sourcePosition.source)
getSourcePath(
// When sourcePosition.source is the loader path the modulePath is generally better.
(sourcePosition.source.includes('|')
? modulePath
: sourcePosition.source) || modulePath
)
)

const originalFrame: StackFrame = {
Expand All @@ -173,7 +212,11 @@ export async function createOriginalStackFrame({
: sourcePosition.source,
lineNumber: sourcePosition.line,
column: sourcePosition.column,
methodName: frame.methodName, // TODO: resolve original method name (?)
methodName:
sourcePosition.name ||
// default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
// Resolve it back to `default` for the method name if the source position didn't have the method.
frame.methodName?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default'),
arguments: [],
}

Expand Down Expand Up @@ -250,8 +293,14 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
const frame = query as unknown as StackFrame & {
isEdgeServer: 'true' | 'false'
isServer: 'true' | 'false'
isAppDirectory: 'true' | 'false'
errorMessage: string | undefined
}
const isAppDirectory = frame.isAppDirectory === 'true'
const isServerError = frame.isServer === 'true'
const isEdgeServerError = frame.isEdgeServer === 'true'
const isClientError = !isServerError && !isEdgeServerError

if (
!(
(frame.file?.startsWith('webpack-internal:///') ||
Expand All @@ -268,20 +317,45 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
/^(webpack-internal:\/\/\/|file:\/\/)/,
''
)
const modulePath = frame.file.replace(
/^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/,
''
)

let source: Source
const compilation =
frame.isEdgeServer === 'true'
? options.edgeServerStats()?.compilation
: frame.isServer === 'true'
? options.serverStats()?.compilation
: options.stats()?.compilation
let source: Source = null
const clientCompilation = options.stats()?.compilation
const serverCompilation = options.serverStats()?.compilation
const edgeCompilation = options.edgeServerStats()?.compilation
try {
source = await getSourceById(
frame.file.startsWith('file:'),
moduleId,
compilation
)
if (isClientError || isAppDirectory) {
// Try Client Compilation first
// In `pages` we leverage `isClientError` to check
// In `app` it depends on if it's a server / client component and when the code throws. E.g. during HTML rendering it's the server/edge compilation.
source = await getSourceById(
frame.file.startsWith('file:'),
moduleId,
clientCompilation
)
}
// Try Server Compilation
// In `pages` this could be something imported in getServerSideProps/getStaticProps as the code for those is tree-shaken.
// In `app` this finds server components and code that was imported from a server component. It also covers when client component code throws during HTML rendering.
if ((isServerError || isAppDirectory) && source === null) {
source = await getSourceById(
frame.file.startsWith('file:'),
moduleId,
serverCompilation
)
}
// Try Edge Server Compilation
// Both cases are the same as Server Compilation, main difference is that it covers `runtime: 'edge'` pages/app routes.
if ((isEdgeServerError || isAppDirectory) && source === null) {
source = await getSourceById(
frame.file.startsWith('file:'),
moduleId,
edgeCompilation
)
}
} catch (err) {
console.log('Failed to get source map:', err)
res.statusCode = 500
Expand Down Expand Up @@ -310,10 +384,13 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
column: frameColumn,
source,
frame,
modulePath: moduleId,
moduleId,
modulePath,
rootDirectory: options.rootDirectory,
errorMessage: frame.errorMessage,
compilation,
clientCompilation: isClientError ? clientCompilation : undefined,
serverCompilation: isServerError ? serverCompilation : undefined,
edgeCompilation: isEdgeServerError ? edgeCompilation : undefined,
})

if (originalStackFrameResponse === null) {
Expand Down
5 changes: 3 additions & 2 deletions packages/react-dev-overlay/tsconfig.json
Expand Up @@ -4,14 +4,15 @@
"sourceMap": true,
"strict": true,
"esModuleInterop": true,
"target": "es3",
"target": "es2020",
"lib": ["dom"],
"downlevelIteration": true,
"preserveWatchOutput": true,
"outDir": "dist",
"jsx": "react",
"noFallthroughCasesInSwitch": true,
"skipLibCheck": true
"skipLibCheck": true,
"moduleResolution": "Node16"
},
"include": ["src/**/*.ts", "src/**/*.tsx"],
"exclude": ["node_modules"]
Expand Down
56 changes: 48 additions & 8 deletions test/development/acceptance-app/ReactRefreshRegression.test.ts
Expand Up @@ -270,19 +270,15 @@ describe('ReactRefreshRegression app', () => {

// https://github.com/vercel/next.js/issues/11504
// TODO-APP: fix case where error is not resolved to source correctly.
test.skip('shows an overlay for a server-side error', async () => {
const { session, cleanup } = await sandbox(next)
test('shows an overlay for anonymous function server-side error', async () => {
const { session, browser, cleanup } = await sandbox(next)

await session.patch(
'app/page.js',
`export default function () { throw new Error('pre boom'); }`
)

const didNotReload = await session.patch(
'app/page.js',
`export default function () { throw new Error('boom'); }`
)
expect(didNotReload).toBe(false)

await browser.refresh()

expect(await session.hasRedbox(true)).toBe(true)

Expand All @@ -295,6 +291,50 @@ describe('ReactRefreshRegression app', () => {
await cleanup()
})

test('shows an overlay for server-side error in server component', async () => {
const { session, browser, cleanup } = await sandbox(next)

await session.patch(
'app/page.js',
`export default function Page() { throw new Error('boom'); }`
)

await browser.refresh()

expect(await session.hasRedbox(true)).toBe(true)

const source = await session.getRedboxSource()
expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
"> 1 | export default function Page() { throw new Error('boom'); }
| ^"
`)

await cleanup()
})

test('shows an overlay for server-side error in client component', async () => {
const { session, browser, cleanup } = await sandbox(next)

await session.patch(
'app/page.js',
`'use client'
export default function Page() { throw new Error('boom'); }`
)

await browser.refresh()

expect(await session.hasRedbox(true)).toBe(true)

const source = await session.getRedboxSource()
expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
" 1 | 'use client'
> 2 | export default function Page() { throw new Error('boom'); }
| ^"
`)

await cleanup()
})

// https://github.com/vercel/next.js/issues/13574
test('custom loader mdx should have Fast Refresh enabled', async () => {
const files = new Map()
Expand Down
Expand Up @@ -115,7 +115,7 @@ exports[`ReactRefreshLogBox app stuck error 1`] = `
`;
exports[`ReactRefreshLogBox app syntax > runtime error 1`] = `
"index.js (6:16) @ eval
"index.js (6:16) @ Error
4 | setInterval(() => {
5 | i++
Expand Down