Skip to content

Commit

Permalink
Remove ignore for anonymous stack frame (#73610)
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi authored Dec 9, 2024
1 parent 4431754 commit 74b09dd
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ function getOriginalStackFrame(

// TODO: merge this section into ignoredList handling
if (
source.file === '<anonymous>' ||
source.file === 'file://' ||
source.file?.match(/^node:/) ||
source.file?.match(/https?:\/\//)
Expand Down Expand Up @@ -136,11 +135,6 @@ export function getFrameSource(frame: StackFrame): string {
}

if (!isWebpackInternalResource(frame.file) && frame.lineNumber != null) {
// If the method name is replayed from server, e.g. Page [Server],
// and it's formatted to empty string, recover it as <anonymous> to display it.
if (!str && frame.methodName.endsWith(' [Server]')) {
str = '<anonymous>'
}
if (str) {
if (frame.column != null) {
str += ` (${frame.lineNumber}:${frame.column})`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,5 @@ describe('webpack-module-path', () => {
'./src/hello.tsx'
)
})

it('should return an empty string for <anonymous> file paths', () => {
expect(formatFrameSourceFile('<anonymous>')).toBe('')
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,15 @@ export function isWebpackInternalResource(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
*
* <anonymous> => ''
*/
export function formatFrameSourceFile(file: string) {
if (file === '<anonymous>') return ''

for (const regex of replacementRegExes) {
file = file.replace(regex, '')
}
Expand Down
26 changes: 8 additions & 18 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createSandbox } from 'development-sandbox'
import { FileRef, nextTestSetup } from 'e2e-utils'
import {
describeVariants as describe,
getRedboxCallStack,
toggleCollapseCallStackFrames,
} from 'next-test-utils'
import path from 'path'
Expand Down Expand Up @@ -770,36 +771,25 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => {
expect(callStackFrames.length).toBeGreaterThan(9)
})

test('should hide unrelated frames in stack trace with unknown anonymous calls', async () => {
test('should show anonymous frames in stack trace', async () => {
await using sandbox = await createSandbox(
next,
new Map([
[
'pages/index.js',
// TODO: repro stringify (<anonymous>)
outdent`
export default function Page() {
const e = new Error("Client error!");
e.stack += \`
at stringify (<anonymous>)
at <unknown> (<anonymous>)
at foo (bar:1:1)\`;
throw e;
}
`,
[1, 2, 3].map(() => {
throw new Error("anonymous error!");
})
}`,
],
])
)
const { session, browser } = sandbox
await session.assertHasRedbox()
await toggleCollapseCallStackFrames(browser)
let callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
)
let texts = await Promise.all(callStackFrames.map((f) => f.innerText()))
expect(texts).not.toContain('stringify\n<anonymous>')
expect(texts).not.toContain('<unknown>\n<anonymous>')
expect(texts).toContain('foo\nbar (1:1)')
const texts = await getRedboxCallStack(browser)
expect(texts).toMatchSnapshot()
})

test('should hide unrelated frames in stack trace with node:internal calls', async () => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ describe('app-dir - capture-console-error-owner-stack', () => {
if (process.env.TURBOPACK) {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "Page
"callStacks": "button
<anonymous> (0:0)
Page
app/browser/event/page.js (5:5)",
"count": 1,
"description": "trigger an console <error>",
Expand All @@ -62,6 +64,8 @@ describe('app-dir - capture-console-error-owner-stack', () => {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "button
<anonymous> (0:0)
button
app/browser/event/page.js (5:6)",
"count": 1,
"description": "trigger an console <error>",
Expand Down Expand Up @@ -274,7 +278,10 @@ describe('app-dir - capture-console-error-owner-stack', () => {
if (process.env.TURBOPACK) {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"callStacks": "JSON.parse
<anonymous> (0:0)
Page
<anonymous> (0:0)",
"count": 1,
"description": "[ Server ] Error: boom",
"source": "app/rsc/page.js (2:17) @ Page
Expand All @@ -291,7 +298,10 @@ describe('app-dir - capture-console-error-owner-stack', () => {
} else {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"callStacks": "JSON.parse
<anonymous> (0:0)
Page
<anonymous> (0:0)",
"count": 1,
"description": "[ Server ] Error: boom",
"source": "app/rsc/page.js (2:17) @ Page
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ describe('app-dir - capture-console-error', () => {
if (process.env.TURBOPACK) {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"callStacks": "JSON.parse
<anonymous> (0:0)",
"count": 1,
"description": "[ Server ] Error: boom",
"source": "app/rsc/page.js (2:17) @ Page
Expand All @@ -297,7 +298,8 @@ describe('app-dir - capture-console-error', () => {
} else {
expect(result).toMatchInlineSnapshot(`
{
"callStacks": "",
"callStacks": "JSON.parse
<anonymous> (0:0)",
"count": 1,
"description": "[ Server ] Error: boom",
"source": "app/rsc/page.js (2:17) @ Page
Expand Down
12 changes: 9 additions & 3 deletions test/development/app-dir/dynamic-error-trace/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@ describe('app dir - dynamic error trace', () => {
.join('\n')

// TODO: Show useful stack
expect(normalizeStackTrace(stackFramesContent)).toMatchInlineSnapshot(
isReactExperimental ? `""` : `""`
)
const normalizedStack = normalizeStackTrace(stackFramesContent)
if (isReactExperimental) {
expect(normalizedStack).toMatchInlineSnapshot(`
"Array.map
<anonymous>"
`)
} else {
expect(normalizedStack).toMatchInlineSnapshot(`""`)
}

const codeframe = await getRedboxSource(browser)
expect(codeframe).toEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,39 @@ const isOwnerStackEnabled =
const source = await getRedboxSource(browser)

if (process.env.TURBOPACK) {
expect(stackFramesContent).toMatchInlineSnapshot(
`"at Page (app/rsc/page.tsx (6:13))"`
)
expect(stackFramesContent).toMatchInlineSnapshot(`
"at span ()
at Page (app/rsc/page.tsx (6:13))"
`)
expect(source).toMatchInlineSnapshot(`
"app/rsc/page.tsx (7:10) @ <anonymous>
"app/rsc/page.tsx (7:10) @ <anonymous>
5 | <div>
6 | {list.map((item, index) => (
> 7 | <span>{item}</span>
| ^
8 | ))}
9 | </div>
10 | )"
`)
5 | <div>
6 | {list.map((item, index) => (
> 7 | <span>{item}</span>
| ^
8 | ))}
9 | </div>
10 | )"
`)
} else {
// FIXME: the owner stack method names should be `Page` instead of `map`
expect(stackFramesContent).toMatchInlineSnapshot(
`"at map (app/rsc/page.tsx (6:13))"`
)
expect(stackFramesContent).toMatchInlineSnapshot(`
"at span ()
at map (app/rsc/page.tsx (6:13))"
`)
// FIXME: the methodName should be `@ <anonymous>` instead of `@ span`
expect(source).toMatchInlineSnapshot(`
"app/rsc/page.tsx (7:10) @ span
"app/rsc/page.tsx (7:10) @ span
5 | <div>
6 | {list.map((item, index) => (
> 7 | <span>{item}</span>
| ^
8 | ))}
9 | </div>
10 | )"
`)
5 | <div>
6 | {list.map((item, index) => (
> 7 | <span>{item}</span>
| ^
8 | ))}
9 | </div>
10 | )"
`)
}
})

Expand All @@ -66,37 +68,41 @@ const isOwnerStackEnabled =
const stackFramesContent = await getStackFramesContent(browser)
const source = await getRedboxSource(browser)
if (process.env.TURBOPACK) {
expect(stackFramesContent).toMatchInlineSnapshot(
`"at Page (app/ssr/page.tsx (8:13))"`
)
expect(stackFramesContent).toMatchInlineSnapshot(`
"at p ()
at Array.map ()
at Page (app/ssr/page.tsx (8:13))"
`)
expect(source).toMatchInlineSnapshot(`
"app/ssr/page.tsx (9:9) @ <unknown>
"app/ssr/page.tsx (9:9) @ <unknown>
7 | <div>
8 | {list.map((item, index) => (
> 9 | <p>{item}</p>
| ^
10 | ))}
11 | </div>
12 | )"
`)
7 | <div>
8 | {list.map((item, index) => (
> 9 | <p>{item}</p>
| ^
10 | ))}
11 | </div>
12 | )"
`)
} else {
// FIXME: the owner stack method names should be `Page` instead of `map`
expect(stackFramesContent).toMatchInlineSnapshot(
`"at map (app/ssr/page.tsx (8:13))"`
)
expect(stackFramesContent).toMatchInlineSnapshot(`
"at p ()
at Array.map ()
at map (app/ssr/page.tsx (8:13))"
`)
// FIXME: the methodName should be `@ <unknown>` instead of `@ p`
expect(source).toMatchInlineSnapshot(`
"app/ssr/page.tsx (9:10) @ p
"app/ssr/page.tsx (9:10) @ p
7 | <div>
8 | {list.map((item, index) => (
> 9 | <p>{item}</p>
| ^
10 | ))}
11 | </div>
12 | )"
`)
7 | <div>
8 | {list.map((item, index) => (
> 9 | <p>{item}</p>
| ^
10 | ))}
11 | </div>
12 | )"
`)
}
})
}
Expand Down

0 comments on commit 74b09dd

Please sign in to comment.