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 all 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,5 +1,6 @@
import isError from '../../../lib/is-error'
import { isNextRouterError } from '../is-next-router-error'
import { captureStackTrace } from '../react-dev-overlay/internal/helpers/capture-stack-trace'
import { handleClientError } from '../react-dev-overlay/internal/helpers/use-error-handler'

export const originConsoleError = window.console.error
Expand All @@ -10,13 +11,14 @@ export function patchConsoleError() {
if (typeof window === 'undefined') {
return
}

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

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
Expand All @@ -28,6 +30,11 @@ export function patchConsoleError() {

if (!isNextRouterError(maybeError)) {
if (process.env.NODE_ENV !== 'production') {
// Create an origin stack that pointing to the origin location of the error
if (!isReplayedError && isError(maybeError)) {
captureStackTrace(maybeError)
}

handleClientError(
// replayed errors have their own complex format string that should be used,
// but if we pass the error directly, `handleClientError` will ignore it
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Polyfill for `Error.captureStackTrace` in browsers
export function captureStackTrace(obj: any) {
const container = new Error()
Object.defineProperty(obj, 'stack', {
configurable: true,
get() {
const { stack } = container
Object.defineProperty(this, 'stack', { value: stack })
return stack
},
})
}
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
Expand Up @@ -10,7 +10,6 @@ export function getReactStitchedError<T = unknown>(err: T): Error | T {
if (typeof (React as any).captureOwnerStack !== 'function') {
return err
}

const isErrorInstance = isError(err)
const originStack = isErrorInstance ? err.stack || '' : ''
const originMessage = isErrorInstance ? err.message : ''
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