Skip to content

Commit

Permalink
fix: add node internals stack frames to ignored list (#73698)
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi authored Jan 6, 2025
1 parent 4c5fa1d commit 286c14d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ function getOriginalStackFrame(
}

// TODO: merge this section into ignoredList handling
if (
source.file === 'file://' ||
source.file?.match(/^node:/) ||
source.file?.match(/https?:\/\//)
) {
if (source.file === 'file://' || source.file?.match(/https?:\/\//)) {
return Promise.resolve({
error: false,
reason: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ function shouldIgnorePath(modulePath: string): boolean {
return (
modulePath.includes('node_modules') ||
// Only relevant for when Next.js is symlinked e.g. in the Next.js monorepo
modulePath.includes('next/dist')
modulePath.includes('next/dist') ||
modulePath.startsWith('node:')
)
}

Expand All @@ -39,8 +40,25 @@ export async function batchedTraceSource(
? // TODO(veil): Why are the frames sent encoded?
decodeURIComponent(frame.file)
: undefined

if (!file) return

// For node internals they cannot traced the actual source code with project.traceSource,
// we need an early return to indicate it's ignored to avoid the unknown scheme error from `project.traceSource`.
if (file.startsWith('node:')) {
return {
frame: {
file,
lineNumber: frame.line ?? 0,
column: frame.column ?? 0,
methodName: frame.methodName ?? '<unknown>',
ignored: true,
arguments: [],
},
source: null,
}
}

const currentDirectoryFileUrl = pathToFileURL(process.cwd()).href

const sourceFrame = await project.traceSource(frame, currentDirectoryFileUrl)
Expand All @@ -51,7 +69,7 @@ export async function batchedTraceSource(
lineNumber: frame.line ?? 0,
column: frame.column ?? 0,
methodName: frame.methodName ?? '<unknown>',
ignored: shouldIgnorePath(frame.file),
ignored: shouldIgnorePath(file),
arguments: [],
},
source: null,
Expand All @@ -60,6 +78,7 @@ export async function batchedTraceSource(

let source = null
const originalFile = sourceFrame.originalFile

// Don't look up source for node_modules or internals. These can often be large bundled files.
const ignored =
shouldIgnorePath(originalFile ?? sourceFrame.file) ||
Expand Down Expand Up @@ -226,7 +245,11 @@ async function nativeTraceSource(
const sourceIndex = applicableSourceMap.sources.indexOf(
originalPosition.source!
)
ignored = applicableSourceMap.ignoreList?.includes(sourceIndex) ?? false
ignored =
applicableSourceMap.ignoreList?.includes(sourceIndex) ??
// When sourcemap is not available, fallback to checking `frame.file`.
// e.g. In pages router, nextjs server code is not bundled into the page.
shouldIgnorePath(frame.file)
}

const originalStackFrame: IgnorableStackFrame = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ function shouldIgnorePath(modulePath: string): boolean {
return (
modulePath.includes('node_modules') ||
// Only relevant for when Next.js is symlinked e.g. in the Next.js monorepo
modulePath.includes('next/dist')
modulePath.includes('next/dist') ||
modulePath.startsWith('node:')
)
}

Expand Down
50 changes: 29 additions & 21 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createSandbox } from 'development-sandbox'
import { FileRef, nextTestSetup } from 'e2e-utils'
import {
describeVariants as describe,
getRedboxCallStack,
getStackFramesContent,
toggleCollapseCallStackFrames,
} from 'next-test-utils'
import path from 'path'
Expand Down Expand Up @@ -771,7 +771,8 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => {
expect(callStackFrames.length).toBeGreaterThan(9)
})

test('should show anonymous frames in stack trace', async () => {
// TODO: hide the anonymous frames between 2 ignored frames
test('should show anonymous frames from stack trace', async () => {
await using sandbox = await createSandbox(
next,
new Map([
Expand All @@ -786,42 +787,49 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => {
],
])
)

const { session, browser } = sandbox

await session.assertHasRedbox()
const texts = await getRedboxCallStack(browser)
expect(texts).toMatchSnapshot()

const stack = await getStackFramesContent(browser)
expect(stack).toMatchInlineSnapshot(`
"at Array.map ()
at Page (pages/index.js (2:13))"
`)
})

test('should hide unrelated frames in stack trace with node:internal calls', async () => {
test('should collapse nodejs internal stack frames from stack trace', async () => {
await using sandbox = await createSandbox(
next,
new Map([
[
'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: {} };
}`,
export default function Page() {}
function createURL() {
new URL("/", "invalid")
}
export function getServerSideProps() {
createURL()
return { props: {} }
}`,
],
])
)

const { session, browser } = sandbox
await session.assertHasRedbox()

// Should still show the errored line in source code
const source = await session.getRedboxSource()
expect(source).toContain('pages/index.js')
expect(source).toContain(`new URL("/", "invalid")`)

const callStackFrames = await browser.elementsByCss(
'[data-nextjs-call-stack-frame]'
const stack = await getStackFramesContent(browser)
expect(stack).toMatchInlineSnapshot(
`"at getServerSideProps (pages/index.js (8:3))"`
)
const texts = await Promise.all(callStackFrames.map((f) => f.innerText()))

expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0)
await toggleCollapseCallStackFrames(browser)
const stackCollapsed = await getStackFramesContent(browser)
expect(stackCollapsed).toContain('at new URL ()')
})
})

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

0 comments on commit 286c14d

Please sign in to comment.