diff --git a/crates/turbo-tasks-env/src/dotenv.rs b/crates/turbo-tasks-env/src/dotenv.rs index 79d0c6fb774f4..11516d1b61d48 100644 --- a/crates/turbo-tasks-env/src/dotenv.rs +++ b/crates/turbo-tasks-env/src/dotenv.rs @@ -22,21 +22,22 @@ impl DotenvProcessEnvVc { pub fn new(prior: Option, path: FileSystemPathVc) -> Self { DotenvProcessEnv { prior, path }.cell() } -} -#[turbo_tasks::value_impl] -impl ProcessEnv for DotenvProcessEnv { #[turbo_tasks::function] - async fn read_all(&self) -> Result { - let prior = if let Some(p) = self.prior { - Some(p.read_all().await?) - } else { - None - }; - let empty = IndexMap::new(); - let prior = prior.as_deref().unwrap_or(&empty); + pub async fn read_prior(self) -> Result { + let this = self.await?; + match this.prior { + None => Ok(EnvMapVc::empty()), + Some(p) => Ok(p.read_all()), + } + } + + #[turbo_tasks::function] + pub async fn read_all_with_prior(self, prior: EnvMapVc) -> Result { + let this = self.await?; + let prior = prior.await?; - let file = self.path.read().await?; + let file = this.path.read().await?; if let FileContent::Content(f) = &*file { let res; let vars; @@ -48,7 +49,7 @@ impl ProcessEnv for DotenvProcessEnv { // state. let initial = env::vars().collect(); - restore_env(&initial, prior, &lock); + restore_env(&initial, &prior, &lock); // from_read will load parse and evalute the Read, and set variables // into the global env. If a later dotenv defines an already defined @@ -59,20 +60,29 @@ impl ProcessEnv for DotenvProcessEnv { restore_env(&vars, &initial, &lock); } - if res.is_err() { - res.context(anyhow!( + if let Err(e) = res { + return Err(e).context(anyhow!( "unable to read {} for env vars", - self.path.to_string().await? - ))?; + this.path.to_string().await? + )); } Ok(EnvMapVc::cell(vars)) } else { - Ok(EnvMapVc::cell(prior.clone())) + Ok(EnvMapVc::cell(prior.clone_value())) } } } +#[turbo_tasks::value_impl] +impl ProcessEnv for DotenvProcessEnv { + #[turbo_tasks::function] + async fn read_all(self_vc: DotenvProcessEnvVc) -> Result { + let prior = self_vc.read_prior(); + Ok(self_vc.read_all_with_prior(prior)) + } +} + /// Restores the global env variables to mirror `to`. fn restore_env( from: &IndexMap, diff --git a/crates/turbo-tasks/src/util.rs b/crates/turbo-tasks/src/util.rs index 14291e543b81e..a799035920e3d 100644 --- a/crates/turbo-tasks/src/util.rs +++ b/crates/turbo-tasks/src/util.rs @@ -84,6 +84,13 @@ impl<'de> Deserialize<'de> for SharedError { } } +impl Deref for SharedError { + type Target = Arc; + fn deref(&self) -> &Self::Target { + &self.inner + } +} + pub struct FormatDuration(pub Duration); impl Display for FormatDuration { diff --git a/crates/turbopack-cli-utils/src/issue.rs b/crates/turbopack-cli-utils/src/issue.rs index a5fe33d724dd3..97d2101ed7489 100644 --- a/crates/turbopack-cli-utils/src/issue.rs +++ b/crates/turbopack-cli-utils/src/issue.rs @@ -369,7 +369,7 @@ impl IssueReporter for ConsoleUi { .iter_with_shortest_path() .map(|(issue, path)| async move { let plain_issue = issue.into_plain(path); - let id = plain_issue.internal_hash().await?; + let id = plain_issue.internal_hash(false).await?; Ok((plain_issue.await?, *id)) }) .try_join() diff --git a/crates/turbopack-core/src/issue/mod.rs b/crates/turbopack-core/src/issue/mod.rs index de11a08e144c4..45c07f588cc87 100644 --- a/crates/turbopack-core/src/issue/mod.rs +++ b/crates/turbopack-core/src/issue/mod.rs @@ -503,42 +503,67 @@ pub struct PlainIssue { pub processing_path: PlainIssueProcessingPathReadRef, } +fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher, full: bool) { + hasher.write_ref(&issue.severity); + hasher.write_ref(&issue.context); + hasher.write_ref(&issue.category); + hasher.write_ref(&issue.title); + hasher.write_ref( + // Normalize syspaths from Windows. These appear in stack traces. + &issue.description.replace('\\', "/"), + ); + hasher.write_ref(&issue.detail); + hasher.write_ref(&issue.documentation_link); + + if let Some(source) = &issue.source { + hasher.write_value(1_u8); + // I'm assuming we don't need to hash the contents. Not 100% correct, but + // probably 99%. + hasher.write_ref(&source.start); + hasher.write_ref(&source.end); + } else { + hasher.write_value(0_u8); + } + + if full { + hasher.write_value(issue.sub_issues.len()); + for i in &issue.sub_issues { + hash_plain_issue(i, hasher, full); + } + + hasher.write_ref(&issue.processing_path); + } +} + impl PlainIssue { /// We need deduplicate issues that can come from unique paths, but /// represent the same underlying problem. Eg, a parse error for a file /// that is compiled in both client and server contexts. - pub fn internal_hash(&self) -> u64 { + /// + /// Passing [full] will also hash any sub-issues and processing paths. While + /// useful for generating exact matching hashes, it's possible for the + /// same issue to pass from multiple processing paths, making for overly + /// verbose logging. + pub fn internal_hash(&self, full: bool) -> u64 { let mut hasher = Xxh3Hash64Hasher::new(); - hasher.write_ref(&self.severity); - hasher.write_ref(&self.context); - hasher.write_ref(&self.category); - hasher.write_ref(&self.title); - hasher.write_ref( - // Normalize syspaths from Windows. These appear in stack traces. - &self.description.replace('\\', "/"), - ); - hasher.write_ref(&self.detail); - hasher.write_ref(&self.documentation_link); - - if let Some(source) = &self.source { - hasher.write_value(1_u8); - // I'm assuming we don't need to hash the contents. Not 100% correct, but - // probably 99%. - hasher.write_ref(&source.start); - hasher.write_ref(&source.end); - } else { - hasher.write_value(0_u8); - } - + hash_plain_issue(self, &mut hasher, full); hasher.finish() } } #[turbo_tasks::value_impl] impl PlainIssueVc { + /// We need deduplicate issues that can come from unique paths, but + /// represent the same underlying problem. Eg, a parse error for a file + /// that is compiled in both client and server contexts. + /// + /// Passing [full] will also hash any sub-issues and processing paths. While + /// useful for generating exact matching hashes, it's possible for the + /// same issue to pass from multiple processing paths, making for overly + /// verbose logging. #[turbo_tasks::function] - pub async fn internal_hash(self) -> Result { - Ok(U64Vc::cell(self.await?.internal_hash())) + pub async fn internal_hash(self, full: bool) -> Result { + Ok(U64Vc::cell(self.await?.internal_hash(full))) } } @@ -631,11 +656,11 @@ impl PlainAssetVc { } #[turbo_tasks::value(transparent, serialization = "none")] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, DeterministicHash)] pub struct PlainIssueProcessingPath(Option>); #[turbo_tasks::value(serialization = "none")] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, DeterministicHash)] pub struct PlainIssueProcessingPathItem { pub context: Option, pub description: StringReadRef, diff --git a/crates/turbopack-env/src/try_env.rs b/crates/turbopack-env/src/try_env.rs index 0641a609a0b13..f8b1db562148c 100644 --- a/crates/turbopack-env/src/try_env.rs +++ b/crates/turbopack-env/src/try_env.rs @@ -1,7 +1,5 @@ -use std::future::IntoFuture; - use anyhow::Result; -use turbo_tasks::primitives::{OptionStringVc, StringVc}; +use turbo_tasks::primitives::StringVc; use turbo_tasks_env::{DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc}; use turbo_tasks_fs::FileSystemPathVc; @@ -9,42 +7,22 @@ use crate::ProcessEnvIssue; #[turbo_tasks::value] pub struct TryDotenvProcessEnv { + dotenv: DotenvProcessEnvVc, prior: ProcessEnvVc, path: FileSystemPathVc, } -impl TryDotenvProcessEnv { - async fn with_issue>>( - &self, - op: impl Fn(ProcessEnvVc) -> V, - ) -> Result { - let r = op(DotenvProcessEnvVc::new(Some(self.prior), self.path).as_process_env()); - match r.await { - Ok(_) => Ok(r), - Err(e) => { - let r = op(self.prior); - // If the prior process env also reports an error we don't want to report our - // issue - r.await?; - - ProcessEnvIssue { - path: self.path, - description: StringVc::cell(e.to_string()), - } - .cell() - .as_issue() - .emit(); - Ok(r) - } - } - } -} - #[turbo_tasks::value_impl] impl TryDotenvProcessEnvVc { #[turbo_tasks::function] pub fn new(prior: ProcessEnvVc, path: FileSystemPathVc) -> Self { - TryDotenvProcessEnv { prior, path }.cell() + let dotenv = DotenvProcessEnvVc::new(Some(prior), path); + TryDotenvProcessEnv { + dotenv, + prior, + path, + } + .cell() } } @@ -52,11 +30,32 @@ impl TryDotenvProcessEnvVc { impl ProcessEnv for TryDotenvProcessEnv { #[turbo_tasks::function] async fn read_all(&self) -> Result { - self.with_issue(|e| e.read_all()).await - } + let dotenv = self.dotenv; + let prior = dotenv.read_prior(); - #[turbo_tasks::function] - async fn read(&self, name: &str) -> Result { - self.with_issue(|e| e.read(name)).await + // Ensure prior succeeds. If it doesn't, then we don't want to attempt to read + // the dotenv file (and potentially emit an Issue), just trust that the prior + // will have emitted its own. + prior.await?; + + let vars = dotenv.read_all_with_prior(prior); + match vars.await { + Ok(_) => Ok(vars), + Err(e) => { + // If parsing the dotenv file fails (but getting the prior value didn't), then + // we want to emit an Issue and fall back to the prior's read. + ProcessEnvIssue { + path: self.path, + // read_all_with_prior will wrap a current error with a context containing the + // failing file, which we don't really care about (we report the filepath as the + // Issue context, not the description). So extract the real error. + description: StringVc::cell(e.root_cause().to_string()), + } + .cell() + .as_issue() + .emit(); + Ok(prior) + } + } } } diff --git a/crates/turbopack-test-utils/src/snapshot.rs b/crates/turbopack-test-utils/src/snapshot.rs index 8f20ab4b936cb..0c5a7aed9cf1b 100644 --- a/crates/turbopack-test-utils/src/snapshot.rs +++ b/crates/turbopack-test-utils/src/snapshot.rs @@ -31,7 +31,7 @@ pub async fn snapshot_issues< let expected_issues = expected(issues_path).await?; let mut seen = HashSet::new(); for (plain_issue, debug_string) in captured_issues.into_iter() { - let hash = encode_hex(plain_issue.internal_hash()); + let hash = encode_hex(plain_issue.internal_hash(true)); let path = issues_path.join(&format!( "{}-{}.txt", @@ -44,7 +44,9 @@ pub async fn snapshot_issues< .replace('?', "__q__"), &hash[0..6] )); - seen.insert(path); + if !seen.insert(path) { + continue; + } // Annoyingly, the PlainIssue.source -> PlainIssueSource.asset -> // PlainAsset.path -> FileSystemPath.fs -> DiskFileSystem.root changes diff --git a/crates/turbopack-tests/tests/snapshot/export-alls/cjs-2/issues/unexpected export __star__-b4c42d.txt b/crates/turbopack-tests/tests/snapshot/export-alls/cjs-2/issues/unexpected export __star__-c9d8a6.txt similarity index 100% rename from crates/turbopack-tests/tests/snapshot/export-alls/cjs-2/issues/unexpected export __star__-b4c42d.txt rename to crates/turbopack-tests/tests/snapshot/export-alls/cjs-2/issues/unexpected export __star__-c9d8a6.txt diff --git a/crates/turbopack-tests/tests/snapshot/export-alls/cjs-script/issues/unexpected export __star__-30c177.txt b/crates/turbopack-tests/tests/snapshot/export-alls/cjs-script/issues/unexpected export __star__-38cf67.txt similarity index 100% rename from crates/turbopack-tests/tests/snapshot/export-alls/cjs-script/issues/unexpected export __star__-30c177.txt rename to crates/turbopack-tests/tests/snapshot/export-alls/cjs-script/issues/unexpected export __star__-38cf67.txt diff --git a/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-0d1499.txt b/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-5f72a6.txt similarity index 100% rename from crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-0d1499.txt rename to crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-5f72a6.txt diff --git a/crates/turbopack-tests/tests/snapshot/imports/resolve_error_cjs/issues/Error resolving commonjs request-a634e0.txt b/crates/turbopack-tests/tests/snapshot/imports/resolve_error_cjs/issues/Error resolving commonjs request-b97684.txt similarity index 100% rename from crates/turbopack-tests/tests/snapshot/imports/resolve_error_cjs/issues/Error resolving commonjs request-a634e0.txt rename to crates/turbopack-tests/tests/snapshot/imports/resolve_error_cjs/issues/Error resolving commonjs request-b97684.txt diff --git a/crates/turbopack-tests/tests/snapshot/imports/resolve_error_esm/issues/Error resolving EcmaScript Modules request-cd3585.txt b/crates/turbopack-tests/tests/snapshot/imports/resolve_error_esm/issues/Error resolving EcmaScript Modules request-aff400.txt similarity index 100% rename from crates/turbopack-tests/tests/snapshot/imports/resolve_error_esm/issues/Error resolving EcmaScript Modules request-cd3585.txt rename to crates/turbopack-tests/tests/snapshot/imports/resolve_error_esm/issues/Error resolving EcmaScript Modules request-aff400.txt