Skip to content

Commit

Permalink
Move node: to ignore list
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi committed Dec 27, 2024
1 parent 2564c81 commit 42bc72a
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ function getOriginalStackFrame(
// TODO: merge this section into ignoredList handling
if (
source.file === 'file://' ||
source.file?.match(/^node:/) ||
source.file?.match(/https?:\/\//)
) {
return Promise.resolve({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,30 @@ import type { Project, TurbopackStackFrame } from '../../../../build/swc/types'
import { getSourceMapFromFile } from '../internal/helpers/get-source-map-from-file'
import { findSourceMap, type SourceMapPayload } from 'node:module'
import { pathToFileURL } from 'node:url'
import isError from '../../../../lib/is-error'

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:')
)
}

function createIgnoredStackFrame(
frame: TurbopackStackFrame
): IgnorableStackFrame {
return {
file: frame.file,
lineNumber: frame.line ?? 0,
column: frame.column ?? 0,
methodName: frame.methodName ?? '<unknown>',
ignored: shouldIgnorePath(frame.file),
arguments: [],
}
}

type IgnorableStackFrame = StackFrame & { ignored: boolean }

const currentSourcesByFile: Map<string, Promise<string | null>> = new Map()
Expand All @@ -46,14 +61,7 @@ export async function batchedTraceSource(
const sourceFrame = await project.traceSource(frame, currentDirectoryFileUrl)
if (!sourceFrame) {
return {
frame: {
file,
lineNumber: frame.line ?? 0,
column: frame.column ?? 0,
methodName: frame.methodName ?? '<unknown>',
ignored: shouldIgnorePath(frame.file),
arguments: [],
},
frame: createIgnoredStackFrame(frame),
source: null,
}
}
Expand All @@ -80,19 +88,7 @@ export async function batchedTraceSource(
}

// TODO: get ignoredList from turbopack source map
const ignorableFrame = {
file: sourceFrame.file,
lineNumber: sourceFrame.line ?? 0,
column: sourceFrame.column ?? 0,
methodName:
// We ignore the sourcemapped name since it won't be the correct name.
// The callsite will point to the column of the variable name instead of the
// name of the enclosing function.
// TODO(NDX-531): Spy on prepareStackTrace to get the enclosing line number for method name mapping.
frame.methodName ?? '<unknown>',
ignored,
arguments: [],
}
const ignorableFrame = createIgnoredStackFrame(frame)

return {
frame: ignorableFrame,
Expand Down Expand Up @@ -268,18 +264,33 @@ async function createOriginalStackFrame(
project: Project,
frame: TurbopackStackFrame
): Promise<OriginalStackFrameResponse | null> {
const traced =
(await nativeTraceSource(frame)) ??
// TODO(veil): When would the bundler know more than native?
// If it's faster, try the bundler first and fall back to native later.
(await batchedTraceSource(project, frame))
if (!traced) {
return null
}
try {
const traced =
(await nativeTraceSource(frame)) ??
// TODO(veil): When would the bundler know more than native?
// If it's faster, try the bundler first and fall back to native later.
(await batchedTraceSource(project, frame))
if (!traced) {
return null
}

return {
originalStackFrame: traced.frame,
originalCodeFrame: getOriginalCodeFrame(traced.frame, traced.source),
return {
originalStackFrame: traced.frame,
originalCodeFrame: getOriginalCodeFrame(traced.frame, traced.source),
}
} catch (e) {
// FIXME: avoid the error [Error: Unknown url scheme] { code: 'GenericFailure' }
if (
isError(e) &&
e.message === 'Unknown url scheme' &&
(e as any).code === 'GenericFailure'
) {
return {
originalStackFrame: createIgnoredStackFrame(frame),
originalCodeFrame: null,
}
}
throw e
}
}

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
34 changes: 0 additions & 34 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ 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 @@ -791,37 +790,4 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => {
const texts = await getRedboxCallStack(browser)
expect(texts).toMatchSnapshot()
})

test('should hide unrelated frames in stack trace with node:internal calls', 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: {} };
}`,
],
])
)
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 texts = await Promise.all(callStackFrames.map((f) => f.innerText()))

expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0)
})
})
60 changes: 60 additions & 0 deletions test/development/errors/node-internal-stack-frame.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { nextTestSetup } from 'e2e-utils'
import {
assertHasRedbox,
getStackFramesContent,
toggleCollapseCallStackFrames,
} from 'next-test-utils'
import { outdent } from 'outdent'

describe('errors - node-internal-stack-frame', () => {
const { next } = nextTestSetup({
files: {
'pages/index.js': outdent`
export default function Page() {}
export function getServerSideProps() {
new URL("/", "invalid");
return { props: {} };
}`,
},
})

test('should hide unrelated frames in stack trace with node:internal calls', async () => {
const browser = await next.browser('/')

await assertHasRedbox(browser)

const stack = await getStackFramesContent(browser)
if (process.env.TURBOPACK) {
// FIXME: ignore the next internal frames from node_modules
expect(stack).toMatchInlineSnapshot(`
"at getServerSideProps ()
at spanContext ()
at async doRender ()
at async responseGenerator ()
at async DevServer.renderToResponseWithComponentsImpl ()
at async DevServer.renderPageComponent ()
at async DevServer.renderToResponseImpl ()
at async DevServer.pipeImpl ()
at async NextNodeServer.handleCatchallRenderRequest ()
at async DevServer.handleRequestImpl ()
at async Span.traceAsyncFn ()
at async DevServer.handleRequest ()
at async invokeRender ()
at async handleRequest ()
at async requestHandlerImpl ()
at async Server.requestListener ()"
`)
} else {
expect(stack).toMatchInlineSnapshot(`
"at eval ()
at renderToHTMLImpl ()"
`)
}
await toggleCollapseCallStackFrames(browser)

// TODO: Since there're still the locations
const expandedStack = await getStackFramesContent(browser)
expect(expandedStack).toContain(`at new URL ()`)
})
})

0 comments on commit 42bc72a

Please sign in to comment.