Skip to content

Commit

Permalink
handle server component replay error in error overlay
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi committed Oct 24, 2024
1 parent 5a953b6 commit 0d6a9c0
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@ export function patchConsoleError() {

window.console.error = (...args: any[]) => {
let maybeError: unknown
let isReplayed: boolean = false

if (process.env.NODE_ENV !== 'production') {
const replayedError = matchReplayedError(...args)
if (replayedError) {
maybeError = replayedError
isReplayed = true
} else {
// See https://github.com/facebook/react/blob/d50323eb845c5fde0d720cae888bf35dedd05506/packages/react-reconciler/src/ReactFiberErrorLogger.js#L78
maybeError = args[1]
Expand All @@ -33,10 +31,7 @@ export function patchConsoleError() {
handleClientError(
// replayed errors have their own complex format string that should be used,
// but if we pass the error directly, `handleClientError` will ignore it
//
// TODO: not passing an error here will make `handleClientError`
// create a new Error, so we'll lose the stack. we should make it smarter
isReplayed ? undefined : maybeError,
maybeError,
args
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
import type { OriginalStackFrameResponse } from '../../server/shared'
import { isWebpackBundled, formatFrameSourceFile } from './webpack-module-path'
import {
isWebpackInternalResource,
formatFrameSourceFile,
} from './webpack-module-path'
export interface OriginalStackFrame extends OriginalStackFrameResponse {
error: boolean
reason: string | null
Expand Down Expand Up @@ -106,7 +109,7 @@ export function getOriginalStackFrames(
export function getFrameSource(frame: StackFrame): string {
if (!frame.file) return ''

const isWebpackFrame = isWebpackBundled(frame.file)
const isWebpackFrame = isWebpackInternalResource(frame.file)

let str = ''
// Skip URL parsing for webpack internal file paths.
Expand Down Expand Up @@ -137,7 +140,7 @@ export function getFrameSource(frame: StackFrame): string {
}
}

if (!isWebpackBundled(frame.file) && frame.lineNumber != null) {
if (!isWebpackInternalResource(frame.file) && frame.lineNumber != null) {
if (frame.column != null) {
str += ` (${frame.lineNumber}:${frame.column})`
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,44 @@
import { formatFrameSourceFile, isWebpackBundled } from './webpack-module-path'
import {
formatFrameSourceFile,
isWebpackInternalResource,
} from './webpack-module-path'

describe('webpack-module-path', () => {
describe('isWebpackBundled', () => {
describe('isWebpackInternalResource', () => {
it('should return true for webpack-internal paths', () => {
expect(isWebpackBundled('webpack-internal:///./src/hello.tsx')).toBe(true)
expect(
isWebpackBundled(
isWebpackInternalResource('webpack-internal:///./src/hello.tsx')
).toBe(true)
expect(
isWebpackInternalResource(
'rsc://React/Server/webpack-internal:///(rsc)/./src/hello.tsx?42'
)
).toBe(true)
expect(
isWebpackBundled(
isWebpackInternalResource(
'rsc://React/Server/webpack:///(rsc)/./src/hello.tsx?42'
)
).toBe(true)
expect(
isWebpackBundled(
isWebpackInternalResource(
'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42'
)
).toBe(true)
expect(isWebpackBundled('webpack://_N_E/./src/hello.tsx')).toBe(true)
expect(isWebpackBundled('webpack://./src/hello.tsx')).toBe(true)
expect(isWebpackBundled('webpack:///./src/hello.tsx')).toBe(true)
expect(
isWebpackInternalResource(
'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42dc'
)
).toBe(true)
expect(isWebpackInternalResource('webpack://_N_E/./src/hello.tsx')).toBe(
true
)
expect(isWebpackInternalResource('webpack://./src/hello.tsx')).toBe(true)
expect(isWebpackInternalResource('webpack:///./src/hello.tsx')).toBe(true)
})

it('should return false for non-webpack-internal paths', () => {
expect(isWebpackBundled('<anonymous>')).toBe(false)
expect(isWebpackBundled('file:///src/hello.tsx')).toBe(false)
expect(isWebpackInternalResource('<anonymous>')).toBe(false)
expect(isWebpackInternalResource('file:///src/hello.tsx')).toBe(false)
})
})

Expand All @@ -50,6 +62,16 @@ describe('webpack-module-path', () => {
'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42'
)
).toBe('./src/hello.tsx')
expect(
formatFrameSourceFile(
'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42?0'
)
).toBe('./src/hello.tsx')
expect(
formatFrameSourceFile(
'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42dc'
)
).toBe('./src/hello.tsx')
expect(formatFrameSourceFile('webpack://_N_E/./src/hello.tsx')).toBe(
'./src/hello.tsx'
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ const replacementRegExes = [
/^(rsc:\/\/React\/[^/]+\/)/,
/^webpack-internal:\/\/\/(\([\w-]+\)\/)?/,
/^(webpack:\/\/\/|webpack:\/\/(_N_E\/)?)(\([\w-]+\)\/)?/,
/\?\w+(\?\d+)?$/, // React replay error query param, .e.g. ?c69d?0, ?c69d
/\?\d+$/, // React's fakeFunctionIdx query param
]

export function isWebpackBundled(file: string) {
export function isWebpackInternalResource(file: string) {
for (const regex of replacementRegExes) {
if (regex.test(file)) return true

Expand All @@ -19,6 +20,7 @@ export function isWebpackBundled(file: string) {
* Format the webpack internal id to original file path
* webpack-internal:///./src/hello.tsx => ./src/hello.tsx
* rsc://React/Server/webpack-internal:///(rsc)/./src/hello.tsx?42 => ./src/hello.tsx
* rsc://React/Server/webpack:///app/indirection.tsx?14cb?0 => app/indirection.tsx
* webpack://_N_E/./src/hello.tsx => ./src/hello.tsx
* webpack://./src/hello.tsx => ./src/hello.tsx
* webpack:///./src/hello.tsx => ./src/hello.tsx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const onCaughtError: HydrationOptions['onCaughtError'] = (
err,
errorInfo
) => {
console.log('onCaughtError', err)
// Skip certain custom errors which are not expected to be reported on client
if (isBailoutToCSRError(err) || isNextRouterError(err)) return

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default async function Page() {
await new Promise((r) => setTimeout(r, 200))
return <p>Page</p>
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { nextTestSetup } from 'e2e-utils'
import {
assertHasRedbox,
getRedboxCallStack,
getRedboxDescription,
hasErrorToast,
retry,
Expand Down Expand Up @@ -42,4 +44,28 @@ describe('Dynamic IO Dev Errors', () => {
)
})
})

it('should display error when component accessed data without suspense boundary', async () => {
const browser = await next.browser('/no-accessed-data')

await retry(async () => {
expect(await hasErrorToast(browser)).toBe(true)
await waitForAndOpenRuntimeError(browser)
await assertHasRedbox(browser)
})

const description = await getRedboxDescription(browser)
const stack = await getRedboxCallStack(browser)
const result = {
description,
stack,
}

expect(result).toMatchInlineSnapshot(`
{
"description": "Error: In Route "/no-accessed-data" this component accessed data without a Suspense boundary above it to provide a fallback UI. See more info: https://nextjs.org/docs/messages/next-prerender-data",
"stack": "",
}
`)
})
})

0 comments on commit 0d6a9c0

Please sign in to comment.