Turbopack: support more server code in tracing error stack frames (#57156)

This:
- Uses `isServer` to use the appropriate Turbopack `FileSystem` when
creating `FileSystemPath`s
- Properly uri decodes path segments originating from `file://` uris
- Correctly reads chunks starting at the project path instead of the
root path


Closes WEB-1815

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This commit is contained in:
Will Binns-Smith 2023-10-20 17:09:10 -07:00 committed by GitHub
parent 0d2edbb23f
commit 1ffef0f1a3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 89 additions and 70 deletions

1
Cargo.lock generated
View file

@ -3493,6 +3493,7 @@ dependencies = [
"turbo-tasks", "turbo-tasks",
"turbopack-binding", "turbopack-binding",
"url", "url",
"urlencoding",
] ]
[[package]] [[package]]

View file

@ -76,6 +76,7 @@ turbopack-binding = { workspace = true, features = [
"__turbopack_ecmascript_hmr_protocol", "__turbopack_ecmascript_hmr_protocol",
] } ] }
url = {workspace = true} url = {workspace = true}
urlencoding = {workspace = true}
[target.'cfg(not(all(target_os = "linux", target_env = "musl", target_arch = "aarch64")))'.dependencies] [target.'cfg(not(all(target_os = "linux", target_env = "musl", target_arch = "aarch64")))'.dependencies]
turbopack-binding = { workspace = true, features = ["__turbo_tasks_malloc"] } turbopack-binding = { workspace = true, features = ["__turbo_tasks_malloc"] }

View file

@ -617,10 +617,11 @@ pub fn project_update_info_subscribe(
#[derive(Debug)] #[derive(Debug)]
#[napi(object)] #[napi(object)]
pub struct StackFrame { pub struct StackFrame {
pub file: String,
pub method_name: Option<String>,
pub line: u32,
pub column: Option<u32>, pub column: Option<u32>,
pub file: String,
pub is_server: bool,
pub line: u32,
pub method_name: Option<String>,
} }
#[napi] #[napi]
@ -633,7 +634,7 @@ pub async fn project_trace_source(
.run_once(async move { .run_once(async move {
let file = match Url::parse(&frame.file) { let file = match Url::parse(&frame.file) {
Ok(url) => match url.scheme() { Ok(url) => match url.scheme() {
"file" => url.path().to_string(), "file" => urlencoding::decode(url.path())?.to_string(),
_ => bail!("Unknown url scheme"), _ => bail!("Unknown url scheme"),
}, },
Err(_) => frame.file.to_string(), Err(_) => frame.file.to_string(),
@ -641,8 +642,8 @@ pub async fn project_trace_source(
let Some(chunk_base) = file.strip_prefix( let Some(chunk_base) = file.strip_prefix(
&(format!( &(format!(
"{}/{}", "{}/{}/",
project.container.project().await?.root_path, project.container.project().await?.project_path,
project.container.project().dist_dir().await? project.container.project().dist_dir().await?
)), )),
) else { ) else {
@ -650,32 +651,29 @@ pub async fn project_trace_source(
return Ok(None); return Ok(None);
}; };
let chunk_path = format!( let path = if frame.is_server {
"{}{}", project
.container
.project()
.node_root()
.join(chunk_base.to_owned())
} else {
project project
.container .container
.project() .project()
.client_relative_path() .client_relative_path()
.await? .join(chunk_base.to_owned())
.path,
chunk_base
);
let path = project
.container
.project()
.client_root()
.fs()
.root()
.join(chunk_path);
let Some(generatable): Option<Vc<Box<dyn GenerateSourceMap>>> =
Vc::try_resolve_sidecast(project.container.get_versioned_content(path)).await?
else {
return Ok(None);
}; };
let map = generatable let Some(versioned) = Vc::try_resolve_sidecast::<Box<dyn GenerateSourceMap>>(
project.container.get_versioned_content(path),
)
.await?
else {
bail!("Could not GenerateSourceMap")
};
let map = versioned
.generate_source_map() .generate_source_map()
.await? .await?
.context("Chunk is missing a sourcemap")?; .context("Chunk is missing a sourcemap")?;
@ -700,6 +698,7 @@ pub async fn project_trace_source(
method_name: token.name, method_name: token.name,
line: token.original_line as u32, line: token.original_line as u32,
column: Some(token.original_column as u32), column: Some(token.original_column as u32),
is_server: frame.is_server,
})) }))
}) })
.await .await
@ -719,12 +718,14 @@ pub async fn project_get_source_for_asset(
.container .container
.project() .project()
.project_path() .project_path()
.fs()
.root()
.join(file_path.to_string()) .join(file_path.to_string())
.read() .read()
.await?; .await?;
let FileContent::Content(source_content) = source_content else { let FileContent::Content(source_content) = source_content else {
return Ok(None); bail!("Cannot find source for asset {}", file_path);
}; };
Ok(Some(source_content.content().to_str()?.to_string())) Ok(Some(source_content.content().to_str()?.to_string()))

View file

@ -269,13 +269,13 @@ impl ProjectContainer {
pub struct Project { pub struct Project {
/// A root path from which all files must be nested under. Trying to access /// A root path from which all files must be nested under. Trying to access
/// a file outside this root will fail. Think of this as a chroot. /// a file outside this root will fail. Think of this as a chroot.
pub root_path: String, root_path: String,
/// A path where to emit the build outputs. next.config.js's distDir. /// A path where to emit the build outputs. next.config.js's distDir.
dist_dir: String, dist_dir: String,
/// A path inside the root_path which contains the app/pages directories. /// A path inside the root_path which contains the app/pages directories.
project_path: String, pub project_path: String,
/// Whether to watch the filesystem for file changes. /// Whether to watch the filesystem for file changes.
watch: bool, watch: bool,
@ -369,7 +369,7 @@ impl Project {
} }
#[turbo_tasks::function] #[turbo_tasks::function]
async fn node_fs(self: Vc<Self>) -> Result<Vc<Box<dyn FileSystem>>> { pub async fn node_fs(self: Vc<Self>) -> Result<Vc<Box<dyn FileSystem>>> {
let this = self.await?; let this = self.await?;
let disk_fs = DiskFileSystem::new("node".to_string(), this.project_path.clone()); let disk_fs = DiskFileSystem::new("node".to_string(), this.project_path.clone());
disk_fs.await?.start_watching_with_invalidation_reason()?; disk_fs.await?.start_watching_with_invalidation_reason()?;

View file

@ -519,11 +519,12 @@ export interface HmrIdentifiers {
identifiers: string[] identifiers: string[]
} }
export interface StackFrame { interface TurbopackStackFrame {
file: string
methodName: string | null
line: number
column: number | null column: number | null
file: string
isServer: boolean
line: number
methodName: string | null
} }
export interface UpdateInfo { export interface UpdateInfo {
@ -548,7 +549,9 @@ export interface Project {
TurbopackResult<HmrIdentifiers> TurbopackResult<HmrIdentifiers>
> >
getSourceForAsset(filePath: string): Promise<string | null> getSourceForAsset(filePath: string): Promise<string | null>
traceSource(stackFrame: StackFrame): Promise<StackFrame | null> traceSource(
stackFrame: TurbopackStackFrame
): Promise<TurbopackStackFrame | null>
updateInfoSubscribe(): AsyncIterableIterator<TurbopackResult<UpdateInfo>> updateInfoSubscribe(): AsyncIterableIterator<TurbopackResult<UpdateInfo>>
} }
@ -925,7 +928,9 @@ function bindingToApi(binding: any, _wasm: boolean) {
return subscription return subscription
} }
traceSource(stackFrame: StackFrame): Promise<StackFrame | null> { traceSource(
stackFrame: TurbopackStackFrame
): Promise<TurbopackStackFrame | null> {
return binding.projectTraceSource(this._nativeProject, stackFrame) return binding.projectTraceSource(this._nativeProject, stackFrame)
} }

View file

@ -1,7 +1,7 @@
import { parse } from 'next/dist/compiled/stacktrace-parser' import { parse } from 'next/dist/compiled/stacktrace-parser'
import type { StackFrame } from 'next/dist/compiled/stacktrace-parser' import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
const regexNextStatic = /\/_next(\/static\/.+)/g const regexNextStatic = /\/_next(\/static\/.+)/
export function parseStack(stack: string): StackFrame[] { export function parseStack(stack: string): StackFrame[] {
const frames = parse(stack) const frames = parse(stack)

View file

@ -2188,20 +2188,24 @@ async function startWatcher(opts: SetupOpts) {
) )
let originalFrame, isEdgeCompiler let originalFrame, isEdgeCompiler
if (frame?.lineNumber && frame?.file) { const frameFile = frame?.file
if (frame?.lineNumber && frameFile) {
if (opts.turbo) { if (opts.turbo) {
try { try {
originalFrame = await createOriginalTurboStackFrame( originalFrame = await createOriginalTurboStackFrame(project!, {
project!, file: frameFile,
frame methodName: frame.methodName,
) line: frame.lineNumber ?? 0,
column: frame.column,
isServer: true,
})
} catch {} } catch {}
} else { } else {
const moduleId = frame.file!.replace( const moduleId = frameFile.replace(
/^(webpack-internal:\/\/\/|file:\/\/)/, /^(webpack-internal:\/\/\/|file:\/\/)/,
'' ''
) )
const modulePath = frame.file.replace( const modulePath = frameFile.replace(
/^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/, /^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/,
'' ''
) )

View file

@ -1,7 +1,7 @@
import type { StackFrame } from 'stacktrace-parser' import type { StackFrame } from 'stacktrace-parser'
import { parse } from 'stacktrace-parser' import { parse } from 'stacktrace-parser'
const regexNextStatic = /\/_next(\/static\/.+)/g const regexNextStatic = /\/_next(\/static\/.+)/
export function parseStack(stack: string): StackFrame[] { export function parseStack(stack: string): StackFrame[] {
const frames = parse(stack) const frames = parse(stack)

View file

@ -1,5 +1,4 @@
import type { IncomingMessage, ServerResponse } from 'http' import type { IncomingMessage, ServerResponse } from 'http'
import type { StackFrame } from 'stacktrace-parser'
import type { ParsedUrlQuery } from 'querystring' import type { ParsedUrlQuery } from 'querystring'
import type { OriginalStackFrameResponse } from './middleware' import type { OriginalStackFrameResponse } from './middleware'
@ -10,29 +9,30 @@ import { launchEditor } from './internal/helpers/launchEditor'
interface Project { interface Project {
getSourceForAsset(filePath: string): Promise<string | null> getSourceForAsset(filePath: string): Promise<string | null>
traceSource(stackFrame: RustStackFrame): Promise<RustStackFrame | null> traceSource(
stackFrame: TurbopackStackFrame
): Promise<TurbopackStackFrame | null>
} }
interface RustStackFrame { interface TurbopackStackFrame {
file: string
methodName: string | null
line: number
column: number | null column: number | null
file: string
isServer: boolean
line: number
methodName: string | null
} }
const currentSourcesByFile: Map<string, Promise<string | null>> = new Map() const currentSourcesByFile: Map<string, Promise<string | null>> = new Map()
async function batchedTraceSource(project: Project, frame: StackFrame) { async function batchedTraceSource(
const file = frame.file project: Project,
frame: TurbopackStackFrame
) {
const file = frame.file ? decodeURIComponent(frame.file) : undefined
if (!file) { if (!file) {
return return
} }
const sourceFrame = await project.traceSource({ const sourceFrame = await project.traceSource(frame)
file,
methodName: frame.methodName,
line: frame.lineNumber ?? 0,
column: frame.column,
})
if (!sourceFrame) { if (!sourceFrame) {
return return
@ -60,7 +60,7 @@ async function batchedTraceSource(project: Project, frame: StackFrame) {
file: sourceFrame.file, file: sourceFrame.file,
lineNumber: sourceFrame.line, lineNumber: sourceFrame.line,
column: sourceFrame.column, column: sourceFrame.column,
methodName: sourceFrame.methodName ?? frame.methodName, methodName: sourceFrame.methodName ?? frame.methodName ?? '<unknown>',
arguments: [], arguments: [],
}, },
source: source ?? null, source: source ?? null,
@ -69,7 +69,7 @@ async function batchedTraceSource(project: Project, frame: StackFrame) {
export async function createOriginalStackFrame( export async function createOriginalStackFrame(
project: Project, project: Project,
frame: StackFrame frame: TurbopackStackFrame
): Promise<OriginalStackFrameResponse | null> { ): Promise<OriginalStackFrameResponse | null> {
const traced = await batchedTraceSource(project, frame) const traced = await batchedTraceSource(project, frame)
if (!traced) { if (!traced) {
@ -94,17 +94,15 @@ export async function createOriginalStackFrame(
} }
} }
function stackFrameFromQuery(query: ParsedUrlQuery): StackFrame { function stackFrameFromQuery(query: ParsedUrlQuery): TurbopackStackFrame {
return { return {
file: query.file as string, file: query.file as string,
methodName: query.methodName as string, methodName: query.methodName as string,
arguments: query.arguments as string[], line:
lineNumber: typeof query.lineNumber === 'string' ? parseInt(query.lineNumber, 10) : 0,
typeof query.lineNumber === 'string'
? parseInt(query.lineNumber, 10)
: null,
column: column:
typeof query.column === 'string' ? parseInt(query.column, 10) : null, typeof query.column === 'string' ? parseInt(query.column, 10) : null,
isServer: query.isServer === 'true',
} }
} }
@ -158,7 +156,7 @@ export function getOverlayMiddleware(project: Project) {
} }
try { try {
launchEditor(filePath, frame.lineNumber ?? 1, frame.column ?? 1) launchEditor(filePath, frame.line, frame.column ?? 1)
} catch (err) { } catch (err) {
console.log('Failed to launch editor:', err) console.log('Failed to launch editor:', err)
res.statusCode = 500 res.statusCode = 500

View file

@ -65,6 +65,11 @@ describe('Read-only source HMR', () => {
const originalContent = await fs.readFile(pagePath, 'utf8') const originalContent = await fs.readFile(pagePath, 'utf8')
const editedContent = originalContent.replace('Hello World', 'COOL page') const editedContent = originalContent.replace('Hello World', 'COOL page')
if (process.env.TURBOPACK) {
// TODO Turbopack needs a bit to start watching
await new Promise((resolve) => setTimeout(resolve, 500))
}
await writeReadOnlyFile(pagePath, editedContent) await writeReadOnlyFile(pagePath, editedContent)
await check(() => getBrowserBodyText(browser), /COOL page/) await check(() => getBrowserBodyText(browser), /COOL page/)
@ -86,8 +91,12 @@ describe('Read-only source HMR', () => {
const originalContent = await fs.readFile(pagePath, 'utf8') const originalContent = await fs.readFile(pagePath, 'utf8')
await fs.remove(pagePath) if (process.env.TURBOPACK) {
// TODO Turbopack needs a bit to start watching
await new Promise((resolve) => setTimeout(resolve, 500))
}
await fs.remove(pagePath)
await writeReadOnlyFile(pagePath, originalContent) await writeReadOnlyFile(pagePath, originalContent)
await check(() => getBrowserBodyText(browser), /Hello World/) await check(() => getBrowserBodyText(browser), /Hello World/)
} finally { } finally {
@ -106,7 +115,7 @@ describe('Read-only source HMR', () => {
newPagePath, newPagePath,
` `
const New = () => <p>New page</p> const New = () => <p>New page</p>
export default New export default New
` `
) )