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

Compare error stack to dedupe error #71798

Merged
merged 11 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import isError from '../../../lib/is-error'
import { isNextRouterError } from '../is-next-router-error'
import { stripStackByFrame } from '../react-dev-overlay/internal/helpers/strip-stack-frame'
import { handleClientError } from '../react-dev-overlay/internal/helpers/use-error-handler'

const NEXT_CONSOLE_STACK_FRAME = 'next-console-stack-frame'

const stripBeforeNextConsoleFrame = (stack: string) =>
stripStackByFrame(stack, NEXT_CONSOLE_STACK_FRAME, false)

export const originConsoleError = window.console.error

// Patch console.error to collect information about hydration errors
Expand All @@ -11,34 +17,49 @@ export function patchConsoleError() {
return
}

window.console.error = (...args: any[]) => {
let maybeError: unknown
const namedLoggerInstance = {
[NEXT_CONSOLE_STACK_FRAME](...args: any[]) {
let maybeError: unknown
let isReplayedError = false

if (process.env.NODE_ENV !== 'production') {
const replayedError = matchReplayedError(...args)
if (replayedError) {
maybeError = replayedError
if (process.env.NODE_ENV !== 'production') {
const replayedError = matchReplayedError(...args)
if (replayedError) {
isReplayedError = true
maybeError = replayedError
} else {
// See https://github.com/facebook/react/blob/d50323eb845c5fde0d720cae888bf35dedd05506/packages/react-reconciler/src/ReactFiberErrorLogger.js#L78
maybeError = args[1]
}
} else {
// See https://github.com/facebook/react/blob/d50323eb845c5fde0d720cae888bf35dedd05506/packages/react-reconciler/src/ReactFiberErrorLogger.js#L78
maybeError = args[1]
maybeError = args[0]
}
} else {
maybeError = args[0]
}

if (!isNextRouterError(maybeError)) {
if (process.env.NODE_ENV !== 'production') {
handleClientError(
// replayed errors have their own complex format string that should be used,
// but if we pass the error directly, `handleClientError` will ignore it
maybeError,
args
)
}
if (!isNextRouterError(maybeError)) {
if (process.env.NODE_ENV !== 'production') {
// Create an origin stack that pointing to the origin location of the error
const captureStackErrorStackTrace = new Error().stack || ''
const strippedStack = stripBeforeNextConsoleFrame(
captureStackErrorStackTrace
)

originConsoleError.apply(window.console, args)
}
handleClientError(
// replayed errors have their own complex format string that should be used,
// but if we pass the error directly, `handleClientError` will ignore it
maybeError,
args,
isReplayedError ? '' : strippedStack
huozhi marked this conversation as resolved.
Show resolved Hide resolved
)
}

originConsoleError.apply(window.console, args)
}
},
}

window.console.error = namedLoggerInstance[NEXT_CONSOLE_STACK_FRAME].bind(
window.console
)
}

function matchReplayedError(...args: unknown[]): Error | null {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export function enqueueConsecutiveDedupedError(
) {
const isFront = isHydrationError(error)
const previousError = isFront ? queue[0] : queue[queue.length - 1]
// Only check message to see if it's the same error, as message is representative display in the console.
if (previousError && previousError.message === error.message) {
// Compare the error stack to dedupe the consecutive errors
if (previousError && previousError.stack === error.stack) {
return
}
// TODO: change all to push error into errorQueue,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react'
import isError from '../../../../../lib/is-error'
import { stripStackByFrame } from './strip-stack-frame'

const REACT_ERROR_STACK_BOTTOM_FRAME = 'react-stack-bottom-frame'
const REACT_ERROR_STACK_BOTTOM_FRAME_REGEX = new RegExp(
`(at ${REACT_ERROR_STACK_BOTTOM_FRAME} )|(${REACT_ERROR_STACK_BOTTOM_FRAME}\\@)`
)

const stripAfterReactBottomFrame = (stack: string) =>
stripStackByFrame(stack, REACT_ERROR_STACK_BOTTOM_FRAME, true)

export function getReactStitchedError<T = unknown>(err: T): Error | T {
if (typeof (React as any).captureOwnerStack !== 'function') {
Expand All @@ -14,14 +15,7 @@ export function getReactStitchedError<T = unknown>(err: T): Error | T {
const isErrorInstance = isError(err)
const originStack = isErrorInstance ? err.stack || '' : ''
const originMessage = isErrorInstance ? err.message : ''
const stackLines = originStack.split('\n')
const indexOfSplit = stackLines.findIndex((line) =>
REACT_ERROR_STACK_BOTTOM_FRAME_REGEX.test(line)
)
const isOriginalReactError = indexOfSplit >= 0 // has the react-stack-bottom-frame
let newStack = isOriginalReactError
? stackLines.slice(0, indexOfSplit).join('\n')
: originStack
let newStack = stripAfterReactBottomFrame(originStack)

const newError = new Error(originMessage)
// Copy all enumerable properties, e.g. digest
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { stripStackByFrame } from './strip-stack-frame'

describe('stripStackByFrame', () => {
it('strips stack after frame', () => {
const stripStackByFrameBefore = (stack: string) =>
stripStackByFrame(stack, 'special-stack-frame', true)

const stack = `Error: test
at page (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
at special-stack-frame (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
at foo (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
`

const strippedStack = stripStackByFrameBefore(stack)
expect(strippedStack).toMatchInlineSnapshot(`
"Error: test
at page (http://localhost:3000/_next/static/chunks/webpack.js:1:1)"
`)
})

it('strips stack before frame', () => {
const stripStackByFrameAfter = (stack: string) =>
stripStackByFrame(stack, 'special-stack-frame', false)

const stack = `Error: test
at page (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
at special-stack-frame (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
at foo (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
`

const strippedStack = stripStackByFrameAfter(stack)
expect(strippedStack).toMatchInlineSnapshot(`
" at foo (http://localhost:3000/_next/static/chunks/webpack.js:1:1)
"
`)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
export function stripStackByFrame(
stack: string,
framePivot: string,
stripAfter: boolean
): string {
const framePivotRegex = new RegExp(`(at ${framePivot} )|(${framePivot}\\@)`)
const stackLines = stack.split('\n')
const indexOfSplit = stackLines.findIndex((line) =>
framePivotRegex.test(line)
)
const isOriginalReactError = indexOfSplit >= 0 // has the frame pivot
const strippedStack = isOriginalReactError
? stripAfter
? // Keep the frames before pivot, from 1st line of stack (error.message) to the pivot
stackLines.slice(0, indexOfSplit).join('\n')
: // Keep the frames after pivot
stackLines.slice(indexOfSplit + 1).join('\n')
: stack

return strippedStack
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@ const rejectionHandlers: Array<ErrorHandler> = []

export function handleClientError(
originError: unknown,
consoleErrorArgs: any[]
consoleErrorArgs: any[],
originStack?: string
) {
let error: Error
if (!originError || !isError(originError)) {
// If it's not an error, format the args into an error
const formattedErrorMessage = formatConsoleArgs(consoleErrorArgs)
error = createUnhandledError(formattedErrorMessage)
// When the originStack is provided, strip the stack after the react-bottom-stack-frame
if (originStack) {
error.stack = originStack
}
} else {
error = originError
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use client'

export default function Page() {
for (let i = 0; i < 3; i++) {
console.error('trigger an console.error in loop of render')
}
return <p>render</p>
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,15 @@ describe('app-dir - capture-console-error', () => {
10 | click to error",
}
`)
} else if (isReactExperimental) {
} else {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "button
app/browser/event/page.js (5:6)",
"callStacks": ${
isReactExperimental
? `"button
app/browser/event/page.js (5:6)"`
: `""`
},
"count": 1,
"description": "trigger an console <error>",
"source": "app/browser/event/page.js (7:17) @ error
Expand All @@ -73,27 +77,55 @@ describe('app-dir - capture-console-error', () => {
10 | click to error",
}
`)
}
})

it('should capture browser console error in render and dedupe if necessary', async () => {
const browser = await next.browser('/browser/render')

await waitForAndOpenRuntimeError(browser)
await assertHasRedbox(browser)

const result = await getRedboxResult(browser)

if (process.env.TURBOPACK) {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": ${isReactExperimental ? 1 : 2},
"description": "trigger an console.error in render",
"source": "app/browser/render/page.js (4:11) @ Page
2 |
3 | export default function Page() {
> 4 | console.error('trigger an console.error in render')
| ^
5 | return <p>render</p>
6 | }
7 |",
}
`)
} else {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": 1,
"description": "trigger an console <error>",
"source": "app/browser/event/page.js (7:17) @ error
"count": ${isReactExperimental ? 1 : 2},
"description": "trigger an console.error in render",
"source": "app/browser/render/page.js (4:11) @ error
5 | <button
6 | onClick={() => {
> 7 | console.error('trigger an console <%s>', 'error')
| ^
8 | }}
9 | >
10 | click to error",
2 |
3 | export default function Page() {
> 4 | console.error('trigger an console.error in render')
| ^
5 | return <p>render</p>
6 | }
7 |",
}
`)
}
})

it('should capture browser console error in render and dedupe if necessary', async () => {
it('should capture browser console error in render and dedupe when multi same errors logged', async () => {
const browser = await next.browser('/browser/render')

await waitForAndOpenRuntimeError(browser)
Expand All @@ -105,7 +137,7 @@ describe('app-dir - capture-console-error', () => {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": 1,
"count": ${isReactExperimental ? 1 : 2},
"description": "trigger an console.error in render",
"source": "app/browser/render/page.js (4:11) @ Page
Expand All @@ -122,7 +154,7 @@ describe('app-dir - capture-console-error', () => {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": 1,
"count": ${isReactExperimental ? 1 : 2},
"description": "trigger an console.error in render",
"source": "app/browser/render/page.js (4:11) @ error
Expand Down Expand Up @@ -150,7 +182,7 @@ describe('app-dir - capture-console-error', () => {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": 1,
"count": ${isReactExperimental ? 1 : 2},
"description": "ssr console error:client",
"source": "app/ssr/page.js (4:11) @ Page
Expand All @@ -167,7 +199,7 @@ describe('app-dir - capture-console-error', () => {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"count": 1,
"count": ${isReactExperimental ? 1 : 2},
"description": "ssr console error:client",
"source": "app/ssr/page.js (4:11) @ error
Expand Down
2 changes: 1 addition & 1 deletion test/lib/next-test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ export async function toggleCollapseComponentStack(

export async function getRedboxCallStack(
browser: BrowserInterface
): Promise<string> {
): Promise<string | null> {
await browser.waitForElementByCss('[data-nextjs-call-stack-frame]', 30000)

const callStackFrameElements = await browser.elementsByCss(
Expand Down
Loading