Skip to content

Commit

Permalink
feat(error-overlay): hide Node.js internals (#62532)
Browse files Browse the repository at this point in the history
### What?

Hide frames that start with `node:internal`.

### Why?

These are usually unactionable anyway. 

### How?

Filter these lines the same way we filtered `stringify <anonymous>` in
#62325

Closes NEXT-2593
  • Loading branch information
balazsorban44 committed Feb 26, 2024
1 parent b2a2e6e commit 0fec89c
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 11 deletions.
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?
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()
})
})

0 comments on commit 0fec89c

Please sign in to comment.