Skip to content

Commit

Permalink
Resolve RSC / HTML rendering errors in error overlay (#43332)
Browse files Browse the repository at this point in the history
  • Loading branch information
timneutkens committed Nov 25, 2022
1 parent 1dd3462 commit d40ecb7
Show file tree
Hide file tree
Showing 15 changed files with 206 additions and 90 deletions.
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

0 comments on commit d40ecb7

Please sign in to comment.