Repository: vercel/next.js
Pull request: fix(napi): Use strong consistency for the entire project_trace_source function

This function does a few more `Vc` reads than just looking up the source map, so it's better to move the entire thing inside one strongly-consistent operation.

This was an attempt to fix a loop (tracing an error -> react fast refresh -> triggers the same error again -> ...) https://vercel.slack.com/archives/C046HAU4H7F/p1747837763588429, though it doesn't seem like this fixes it.

Closes PACK-4750

Changed files:
filename: crates/napi/src/next_api/project.rs
patch: @@ -1,4 +1,4 @@
-use std::{io::Write, path::PathBuf, sync::Arc, thread, time::Duration};
+use std::{borrow::Cow, io::Write, path::PathBuf, sync::Arc, thread, time::Duration};
 
 use anyhow::{Context, Result, anyhow, bail};
 use napi::{
@@ -24,14 +24,16 @@ use next_core::tracing_presets::{
 };
 use once_cell::sync::Lazy;
 use rand::Rng;
+use serde::{Deserialize, Serialize};
 use tokio::{io::AsyncWriteExt, time::Instant};
 use tracing::Instrument;
 use tracing_subscriber::{Registry, layer::SubscriberExt, util::SubscriberInitExt};
 use turbo_rcstr::RcStr;
 use turbo_tasks::{
-    Completion, Effects, FxIndexSet, OperationVc, ReadRef, ResolvedVc, TransientInstance,
-    TryJoinIterExt, UpdateInfo, Vc, get_effects,
+    Completion, Effects, FxIndexSet, NonLocalValue, OperationValue, OperationVc, ReadRef,
+    ResolvedVc, TaskInput, TransientInstance, TryJoinIterExt, UpdateInfo, Vc, get_effects,
     message_queue::{CompilationEvent, Severity, TimingEvent},
+    trace::TraceRawVcs,
 };
 use turbo_tasks_fs::{
     DiskFileSystem, FileContent, FileSystem, FileSystemPath, get_relative_path_to,
@@ -43,7 +45,7 @@ use turbopack_core::{
     error::PrettyPrintError,
     issue::PlainIssue,
     output::{OutputAsset, OutputAssets},
-    source_map::{OptionSourceMap, OptionStringifiedSourceMap, SourceMap, Token},
+    source_map::{OptionStringifiedSourceMap, SourceMap, Token},
     version::{PartialUpdate, TotalUpdate, Update, VersionState},
 };
 use turbopack_ecmascript_hmr_protocol::{ClientUpdateInstruction, ResourceIdentifier};
@@ -448,7 +450,7 @@ pub async fn project_new(
     ))
 }
 
-#[derive(Debug, Clone, serde::Serialize)]
+#[derive(Debug, Clone, Serialize)]
 struct SlowFilesystemEvent {
     directory: String,
     duration_ms: u128,
@@ -1326,21 +1328,36 @@ pub fn project_compilation_events_subscribe(
     Ok(())
 }
 
-#[turbo_tasks::value]
-#[derive(Debug)]
 #[napi(object)]
+#[derive(
+    Clone,
+    Debug,
+    Deserialize,
+    Eq,
+    Hash,
+    NonLocalValue,
+    OperationValue,
+    PartialEq,
+    Serialize,
+    TaskInput,
+    TraceRawVcs,
+)]
 pub struct StackFrame {
     pub is_server: bool,
     pub is_internal: Option<bool>,
-    pub original_file: Option<String>,
+    pub original_file: Option<RcStr>,
     pub file: RcStr,
-    // 1-indexed, unlike source map tokens
+    /// 1-indexed, unlike source map tokens
     pub line: Option<u32>,
-    // 1-indexed, unlike source map tokens
+    /// 1-indexed, unlike source map tokens
     pub column: Option<u32>,
     pub method_name: Option<RcStr>,
 }
 
+#[turbo_tasks::value(transparent)]
+#[derive(Clone)]
+pub struct OptionStackFrame(Option<StackFrame>);
+
 #[turbo_tasks::function]
 pub async fn get_source_map_rope(
     container: Vc<ProjectContainer>,
@@ -1407,12 +1424,97 @@ pub fn get_source_map_rope_operation(
 }
 
 #[turbo_tasks::function(operation)]
-pub fn get_source_map_operation(
+pub async fn project_trace_source_operation(
     container: ResolvedVc<ProjectContainer>,
-    file_path: RcStr,
-) -> Vc<OptionSourceMap> {
-    let map = get_source_map_rope(*container, file_path);
-    SourceMap::new_from_rope_cached(map)
+    frame: StackFrame,
+    current_directory_file_url: RcStr,
+) -> Result<Vc<OptionStackFrame>> {
+    let Some(map) =
+        &*SourceMap::new_from_rope_cached(get_source_map_rope(*container, frame.file)).await?
+    else {
+        return Ok(Vc::cell(None));
+    };
+
+    let Some(line) = frame.line else {
+        return Ok(Vc::cell(None));
+    };
+
+    let token = map
+        .lookup_token(
+            line.saturating_sub(1),
+            frame.column.unwrap_or(1).saturating_sub(1),
+        )
+        .await?;
+
+    let (original_file, line, column, method_name) = match token {
+        Token::Original(token) => (
+            match urlencoding::decode(&token.original_file)? {
+                Cow::Borrowed(_) => token.original_file,
+                Cow::Owned(original_file) => RcStr::from(original_file),
+            },
+            // JS stack frames are 1-indexed, source map tokens are 0-indexed
+            Some(token.original_line + 1),
+            Some(token.original_column + 1),
+            token.name,
+        ),
+        Token::Synthetic(token) => {
+            let Some(original_file) = token.guessed_original_file else {
+                return Ok(Vc::cell(None));
+            };
+            (original_file, None, None, None)
+        }
+    };
+
+    let project_root_uri =
+        uri_from_file(container.project().project_root_path(), None).await? + "/";
+    let (file, original_file, is_internal) =
+        if let Some(source_file) = original_file.strip_prefix(&project_root_uri) {
+            // Client code uses file://
+            (
+                RcStr::from(
+                    get_relative_path_to(&current_directory_file_url, &original_file)
+                        // TODO(sokra) remove this to include a ./ here to make it a relative path
+                        .trim_start_matches("./"),
+                ),
+                Some(RcStr::from(source_file)),
+                false,
+            )
+        } else if let Some(source_file) = original_file.strip_prefix(&*SOURCE_MAP_PREFIX_PROJECT) {
+            // Server code uses turbopack:///[project]
+            // TODO should this also be file://?
+            (
+                RcStr::from(
+                    get_relative_path_to(
+                        &current_directory_file_url,
+                        &format!("{project_root_uri}{source_file}"),
+                    )
+                    // TODO(sokra) remove this to include a ./ here to make it a relative path
+                    .trim_start_matches("./"),
+                ),
+                Some(RcStr::from(source_file)),
+                false,
+            )
+        } else if let Some(source_file) = original_file.strip_prefix(&*SOURCE_MAP_PREFIX) {
+            // All other code like turbopack:///[turbopack] is internal code
+            // TODO(veil): Should the protocol be preserved?
+            (RcStr::from(source_file), None, true)
+        } else {
+            bail!(
+                "Original file ({}) outside project ({})",
+                original_file,
+                project_root_uri
+            )
+        };
+
+    Ok(Vc::cell(Some(StackFrame {
+        file,
+        original_file,
+        method_name,
+        line,
+        column,
+        is_server: frame.is_server,
+        is_internal: Some(is_internal),
+    })))
 }
 
 #[napi]
@@ -1425,98 +1527,17 @@ pub async fn project_trace_source(
     let container = project.container;
     let traced_frame = turbo_tasks
         .run_once(async move {
-            let Some(map) = &*get_source_map_operation(container, frame.file)
-                .read_strongly_consistent()
-                .await?
-            else {
-                return Ok(None);
-            };
-
-            let Some(line) = frame.line else {
-                return Ok(None);
-            };
-
-            let token = map
-                .lookup_token(
-                    line.saturating_sub(1),
-                    frame.column.unwrap_or(1).saturating_sub(1),
-                )
-                .await?;
-
-            let (original_file, line, column, name) = match token {
-                Token::Original(token) => (
-                    urlencoding::decode(&token.original_file)?.into_owned(),
-                    // JS stack frames are 1-indexed, source map tokens are 0-indexed
-                    Some(token.original_line + 1),
-                    Some(token.original_column + 1),
-                    token.name.clone(),
-                ),
-                Token::Synthetic(token) => {
-                    let Some(file) = &token.guessed_original_file else {
-                        return Ok(None);
-                    };
-                    (file.to_owned(), None, None, None)
-                }
-            };
-
-            let project_root_uri =
-                uri_from_file(project.container.project().project_root_path(), None).await? + "/";
-            let (file, original_file, is_internal) = if let Some(source_file) =
-                original_file.strip_prefix(&project_root_uri)
-            {
-                // Client code uses file://
-                (
-                    RcStr::from(
-                        get_relative_path_to(&current_directory_file_url, &original_file)
-                            // TODO(sokra) remove this to include a ./ here to make it a relative
-                            // path
-                            .trim_start_matches("./"),
-                    ),
-                    Some(source_file.to_string()),
-                    false,
-                )
-            } else if let Some(source_file) =
-                original_file.strip_prefix(&*SOURCE_MAP_PREFIX_PROJECT)
-            {
-                // Server code uses turbopack:///[project]
-                // TODO should this also be file://?
-                (
-                    RcStr::from(
-                        get_relative_path_to(
-                            &current_directory_file_url,
-                            &format!("{project_root_uri}{source_file}"),
-                        )
-                        // TODO(sokra) remove this to include a ./ here to make it a relative path
-                        .trim_start_matches("./"),
-                    ),
-                    Some(source_file.to_string()),
-                    false,
-                )
-            } else if let Some(source_file) = original_file.strip_prefix(&*SOURCE_MAP_PREFIX) {
-                // All other code like turbopack:///[turbopack] is internal code
-                // TODO(veil): Should the protocol be preserved?
-                (RcStr::from(source_file), None, true)
-            } else {
-                bail!(
-                    "Original file ({}) outside project ({})",
-                    original_file,
-                    project_root_uri
-                )
-            };
-
-            Ok(Some(StackFrame {
-                file,
-                original_file,
-                method_name: name,
-                line,
-                column,
-                is_server: frame.is_server,
-                is_internal: Some(is_internal),
-            }))
+            project_trace_source_operation(
+                container,
+                frame,
+                RcStr::from(current_directory_file_url),
+            )
+            .read_strongly_consistent()
+            .await
         })
         .await
         .map_err(|e| napi::Error::from_reason(PrettyPrintError(&e).to_string()))?;
-    Ok(traced_frame)
+    Ok(ReadRef::into_owned(traced_frame))
 }
 
 #[napi]

filename: packages/next/src/build/swc/generated-native.d.ts
patch: @@ -319,9 +319,11 @@ export declare function projectCompilationEventsSubscribe(
 export interface StackFrame {
   isServer: boolean
   isInternal?: boolean
-  originalFile?: string
+  originalFile?: RcStr
   file: RcStr
+  /** 1-indexed, unlike source map tokens */
   line?: number
+  /** 1-indexed, unlike source map tokens */
   column?: number
   methodName?: RcStr
 }

filename: turbopack/crates/turbopack-core/src/source_map/mod.rs
patch: @@ -116,7 +116,7 @@ pub struct TokenWithSource {
 pub struct SyntheticToken {
     pub generated_line: u32,
     pub generated_column: u32,
-    pub guessed_original_file: Option<String>,
+    pub guessed_original_file: Option<RcStr>,
 }
 
 /// An OriginalToken represents a region of the generated file that exists in
@@ -126,7 +126,7 @@ pub struct SyntheticToken {
 pub struct OriginalToken {
     pub generated_line: u32,
     pub generated_column: u32,
-    pub original_file: String,
+    pub original_file: RcStr,
     pub original_line: u32,
     pub original_column: u32,
     pub name: Option<RcStr>,
@@ -154,10 +154,9 @@ impl From<sourcemap::Token<'_>> for Token {
             Token::Original(OriginalToken {
                 generated_line: t.get_dst_line(),
                 generated_column: t.get_dst_col(),
-                original_file: t
-                    .get_source()
-                    .expect("already checked token has source")
-                    .to_string(),
+                original_file: RcStr::from(
+                    t.get_source().expect("already checked token has source"),
+                ),
                 original_line: t.get_src_line(),
                 original_column: t.get_src_col(),
                 name: t.get_name().map(RcStr::from),
@@ -574,7 +573,7 @@ impl SourceMap {
                     if let DecodedMap::Regular(map) = &map.map.0 {
                         if map.get_source_count() == 1 {
                             let source = map.sources().next().unwrap();
-                            *guessed_original_file = Some(source.to_string());
+                            *guessed_original_file = Some(RcStr::from(source));
                         }
                     }
                 }

filename: turbopack/crates/turbopack-node/src/source_map/trace.rs
patch: @@ -102,7 +102,7 @@ pub async fn trace_source_map(
         .await?;
     let result = match token {
         Token::Original(t) => TraceResult::Found(StackFrame {
-            file: t.original_file.clone().into(),
+            file: Cow::Owned(t.original_file.into_owned()),
             line: Some(t.original_line.saturating_add(1)),
             column: Some(t.original_column.saturating_add(1)),
             name: t