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

feat(error-overlay): hide Node.js internals #62532

Merged
merged 5 commits into from
Feb 26, 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
Expand Up @@ -16,7 +16,7 @@ export function RuntimeError({ error }: RuntimeErrorProps) {
!(
f.sourceStackFrame.file === '<anonymous>' &&
['stringify', '<unknown>'].includes(f.sourceStackFrame.methodName)
)
) && !f.sourceStackFrame.file?.startsWith('node:internal')
)

const firstFirstPartyFrameIndex = filteredFrames.findIndex(
Expand Down
28 changes: 25 additions & 3 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
await cleanup()
})

test('stringify <anonymous> and <unknown> <anonymous> are hidden in stack trace', async () => {
test('useless frames are hidden in stack trace', async () => {
const { session, browser, cleanup } = await sandbox(
next,
new Map([
Expand All @@ -849,6 +849,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
export default function Page() {
const e = new Error("Boom!");
e.stack += \`
// REVIEW: how to reliably test the presence of these stack frames?
at stringify (<anonymous>)
at <unknown> (<anonymous>)
at foo (bar:1:1)\`;
Expand All @@ -860,13 +861,34 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
)
expect(await session.hasRedbox()).toBe(true)
await expandCallStack(browser)
const callStackFrames = await browser.elementsByCss(
let callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
)
const texts = await Promise.all(callStackFrames.map((f) => f.innerText()))
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)')

// Test that node:internal errors should be hidden

next.patchFile(
'app/page.js',
// Node.js will throw an error about the invalid URL since this is a server component
outdent`
export default function Page() {
new URL("/", "invalid");
}`
)

expect(await session.hasRedbox()).toBe(true)
await expandCallStack(browser)
callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
)
texts = await Promise.all(callStackFrames.map((f) => f.innerText()))

expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0)

await cleanup()
})

Expand Down
39 changes: 32 additions & 7 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,34 +784,59 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => {
await cleanup()
})

test('stringify <anonymous> and <unknown> <anonymous> are hidden in stack trace for pages error', async () => {
test('useless frames are hidden in stack trace for pages error', async () => {
const { session, browser, cleanup } = await sandbox(
next,
new Map([
[
'pages/index.js',
outdent`
export default function Page() {
const e = new Error("Client error!");
e.stack += \`
export default function Page() {
const e = new Error("Client error!");
e.stack += \`
// REVIEW: how to reliably test the presence of these stack frames?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// REVIEW: how to reliably test the presence of these stack frames?

at stringify (<anonymous>)
at <unknown> (<anonymous>)
at foo (bar:1:1)\`;
throw e;
}
`,
`,
],
])
)
expect(await session.hasRedbox()).toBe(true)
await expandCallStack(browser)
const callStackFrames = await browser.elementsByCss(
let callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
)
const texts = await Promise.all(callStackFrames.map((f) => f.innerText()))
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)')

// Test that node:internal errors should be hidden

next.patchFile(
'pages/index.js',
// Node.js will throw an error about the invalid URL since it happens server-side
outdent`
export default function Page() {}

export function getServerSideProps() {
new URL("/", "invalid");
return { props: {} };
}`
)

expect(await session.hasRedbox()).toBe(true)
await expandCallStack(browser)
callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
)
texts = await Promise.all(callStackFrames.map((f) => f.innerText()))

expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0)

await cleanup()
})
})