From b8793adb9af462a53679269eb1c460c37f226b68 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 4 Apr 2023 14:56:57 -0400 Subject: [PATCH 1/6] Use root cause of DotenvProcessEnv error when emitting Issue --- crates/turbopack-env/src/try_env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turbopack-env/src/try_env.rs b/crates/turbopack-env/src/try_env.rs index 0641a609a0b13..381b18180a249 100644 --- a/crates/turbopack-env/src/try_env.rs +++ b/crates/turbopack-env/src/try_env.rs @@ -29,7 +29,7 @@ impl TryDotenvProcessEnv { ProcessEnvIssue { path: self.path, - description: StringVc::cell(e.to_string()), + description: StringVc::cell(e.root_cause().to_string()), } .cell() .as_issue() From f7794c6d4cbb0565b2c73cc59dfa1203680b8091 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 4 Apr 2023 18:57:39 -0400 Subject: [PATCH 2/6] Expose Dotenv error states in method --- crates/turbo-tasks-env/src/dotenv.rs | 65 ++++++++++++------ crates/turbo-tasks-env/src/lib.rs | 8 +-- crates/turbo-tasks/src/util.rs | 7 ++ crates/turbopack-core/src/issue/mod.rs | 56 ++++++++------- crates/turbopack-env/src/try_env.rs | 75 ++++++++++----------- crates/turbopack-test-utils/src/snapshot.rs | 4 +- 6 files changed, 129 insertions(+), 86 deletions(-) diff --git a/crates/turbo-tasks-env/src/dotenv.rs b/crates/turbo-tasks-env/src/dotenv.rs index 79d0c6fb774f4..4722b7d507cc0 100644 --- a/crates/turbo-tasks-env/src/dotenv.rs +++ b/crates/turbo-tasks-env/src/dotenv.rs @@ -1,6 +1,6 @@ use std::{env, sync::MutexGuard}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, Error, Result}; use indexmap::IndexMap; use turbo_tasks::ValueToString; use turbo_tasks_fs::{FileContent, FileSystemPathVc}; @@ -16,22 +16,26 @@ pub struct DotenvProcessEnv { path: FileSystemPathVc, } -#[turbo_tasks::value_impl] -impl DotenvProcessEnvVc { - #[turbo_tasks::function] - pub fn new(prior: Option, path: FileSystemPathVc) -> Self { - DotenvProcessEnv { prior, path }.cell() - } +pub enum DotenvReadResult { + /// A PriorError is an error that happens during the read_all of the prior + /// [ProcessEnvVc]. + PriorError(Error), + + /// A CurrentError is an error that happens during the read/parse of the + /// `.env` file. + CurrentError(Error), + + Ok(EnvMapVc), } -#[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 +impl DotenvProcessEnv { + pub async fn try_read_all(&self) -> Result { + let prior = match self.prior { + None => None, + Some(p) => match p.read_all().await { + Ok(p) => Some(p), + Err(e) => return Ok(DotenvReadResult::PriorError(e)), + }, }; let empty = IndexMap::new(); let prior = prior.as_deref().unwrap_or(&empty); @@ -59,16 +63,37 @@ impl ProcessEnv for DotenvProcessEnv { restore_env(&vars, &initial, &lock); } - if res.is_err() { - res.context(anyhow!( + if let Err(e) = res { + return Ok(DotenvReadResult::CurrentError(anyhow!(e).context(anyhow!( "unable to read {} for env vars", self.path.to_string().await? - ))?; + )))); } - Ok(EnvMapVc::cell(vars)) + Ok(DotenvReadResult::Ok(EnvMapVc::cell(vars))) } else { - Ok(EnvMapVc::cell(prior.clone())) + Ok(DotenvReadResult::Ok(EnvMapVc::cell(prior.clone()))) + } + } +} + +#[turbo_tasks::value_impl] +impl DotenvProcessEnvVc { + #[turbo_tasks::function] + 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_vc: DotenvProcessEnvVc) -> Result { + let this = self_vc.await?; + match this.try_read_all().await? { + DotenvReadResult::Ok(v) => Ok(v), + DotenvReadResult::PriorError(e) => Err(e), + DotenvReadResult::CurrentError(e) => Err(e), } } } diff --git a/crates/turbo-tasks-env/src/lib.rs b/crates/turbo-tasks-env/src/lib.rs index fbe06368e75f6..d1a9f18b845d9 100644 --- a/crates/turbo-tasks-env/src/lib.rs +++ b/crates/turbo-tasks-env/src/lib.rs @@ -1,9 +1,9 @@ #![feature(min_specialization)] -mod command_line; -mod custom; -mod dotenv; -mod filter; +pub mod command_line; +pub mod custom; +pub mod dotenv; +pub mod filter; use std::{env, sync::Mutex}; 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-core/src/issue/mod.rs b/crates/turbopack-core/src/issue/mod.rs index de11a08e144c4..dea850ad03d8a 100644 --- a/crates/turbopack-core/src/issue/mod.rs +++ b/crates/turbopack-core/src/issue/mod.rs @@ -503,33 +503,43 @@ pub struct PlainIssue { pub processing_path: PlainIssueProcessingPathReadRef, } +fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher) { + 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); + } + + hasher.write_value(issue.sub_issues.len()); + for i in &issue.sub_issues { + hash_plain_issue(i, hasher); + } + + 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 { 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); hasher.finish() } } @@ -631,11 +641,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 381b18180a249..df1b534cce72f 100644 --- a/crates/turbopack-env/src/try_env.rs +++ b/crates/turbopack-env/src/try_env.rs @@ -1,50 +1,30 @@ -use std::future::IntoFuture; - use anyhow::Result; -use turbo_tasks::primitives::{OptionStringVc, StringVc}; -use turbo_tasks_env::{DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc}; +use turbo_tasks::primitives::StringVc; +use turbo_tasks_env::{ + dotenv::DotenvReadResult, DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc, +}; use turbo_tasks_fs::FileSystemPathVc; 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.root_cause().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 +32,30 @@ impl TryDotenvProcessEnvVc { impl ProcessEnv for TryDotenvProcessEnv { #[turbo_tasks::function] async fn read_all(&self) -> Result { - self.with_issue(|e| e.read_all()).await - } - - #[turbo_tasks::function] - async fn read(&self, name: &str) -> Result { - self.with_issue(|e| e.read(name)).await + let dotenv = self.dotenv.await?; + match dotenv.try_read_all().await? { + DotenvReadResult::Ok(v) => Ok(v), + DotenvReadResult::PriorError(e) => { + // If reading the prior value failed, then we cannot determine what the read + // could have been (the dotenv depends on the state of the prior to build) + // build). Trust the prior will emit an issue, and error out. + Err(e) + } + DotenvReadResult::CurrentError(e) => { + // If parsing the dotenv file fails (but getting the prior value didn't), then + // we want to emit an error and fall back to the prior's read. + ProcessEnvIssue { + path: self.path, + // try_read_all 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(self.prior.read_all()) + } + } } } diff --git a/crates/turbopack-test-utils/src/snapshot.rs b/crates/turbopack-test-utils/src/snapshot.rs index 8f20ab4b936cb..47bd5a7ebb99c 100644 --- a/crates/turbopack-test-utils/src/snapshot.rs +++ b/crates/turbopack-test-utils/src/snapshot.rs @@ -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 From a5bbe3b6068cceb61bc99da41151c8a5c3a01174 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 4 Apr 2023 19:17:02 -0400 Subject: [PATCH 3/6] Differentiate logging from snapshotting --- crates/turbopack-cli-utils/src/issue.rs | 2 +- crates/turbopack-core/src/issue/mod.rs | 35 +++++++++++++++------ crates/turbopack-test-utils/src/snapshot.rs | 2 +- 3 files changed, 27 insertions(+), 12 deletions(-) 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 dea850ad03d8a..45c07f588cc87 100644 --- a/crates/turbopack-core/src/issue/mod.rs +++ b/crates/turbopack-core/src/issue/mod.rs @@ -503,7 +503,7 @@ pub struct PlainIssue { pub processing_path: PlainIssueProcessingPathReadRef, } -fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher) { +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); @@ -525,30 +525,45 @@ fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher) { hasher.write_value(0_u8); } - hasher.write_value(issue.sub_issues.len()); - for i in &issue.sub_issues { - hash_plain_issue(i, hasher); - } + 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); + 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(); - hash_plain_issue(self, &mut hasher); + 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))) } } diff --git a/crates/turbopack-test-utils/src/snapshot.rs b/crates/turbopack-test-utils/src/snapshot.rs index 47bd5a7ebb99c..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", From 599648f97840c2b5384056ae5e31b89ec99f2013 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 4 Apr 2023 19:25:52 -0400 Subject: [PATCH 4/6] Comments --- crates/turbo-tasks-env/src/dotenv.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/turbo-tasks-env/src/dotenv.rs b/crates/turbo-tasks-env/src/dotenv.rs index 4722b7d507cc0..b04174907493b 100644 --- a/crates/turbo-tasks-env/src/dotenv.rs +++ b/crates/turbo-tasks-env/src/dotenv.rs @@ -16,6 +16,9 @@ pub struct DotenvProcessEnv { path: FileSystemPathVc, } +/// Dotenv loading depends on prior state to resolve the current state. This +/// exposes the origin of a failed parse, so that callers can determine if the +/// prior state failed, or parsing of the current dotenv failed. pub enum DotenvReadResult { /// A PriorError is an error that happens during the read_all of the prior /// [ProcessEnvVc]. @@ -29,6 +32,11 @@ pub enum DotenvReadResult { } impl DotenvProcessEnv { + /// Attempts to assemble the EnvMapVc for our dotenv file. If either the + /// prior fails to read, or the current dotenv can't be parsed, an + /// appropriate Ok(DotenvReadResult) will be returned. If an unexpected + /// error (like disk reading or remote cache access) fails, then a regular + /// Err() will be returned. pub async fn try_read_all(&self) -> Result { let prior = match self.prior { None => None, From 1a01e23f6461a211b2148d8e47f6e98cb58f8087 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Wed, 5 Apr 2023 00:18:26 -0400 Subject: [PATCH 5/6] Update snapshots --- ... __star__-b4c42d.txt => unexpected export __star__-c9d8a6.txt} | 0 ... __star__-30c177.txt => unexpected export __star__-38cf67.txt} | 0 ...1499.txt => Code generation for chunk item errored-5f72a6.txt} | 0 ...est-a634e0.txt => Error resolving commonjs request-b97684.txt} | 0 ....txt => Error resolving EcmaScript Modules request-aff400.txt} | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename crates/turbopack-tests/tests/snapshot/export-alls/cjs-2/issues/{unexpected export __star__-b4c42d.txt => unexpected export __star__-c9d8a6.txt} (100%) rename crates/turbopack-tests/tests/snapshot/export-alls/cjs-script/issues/{unexpected export __star__-30c177.txt => unexpected export __star__-38cf67.txt} (100%) rename crates/turbopack-tests/tests/snapshot/imports/json/issues/{Code generation for chunk item errored-0d1499.txt => Code generation for chunk item errored-5f72a6.txt} (100%) rename crates/turbopack-tests/tests/snapshot/imports/resolve_error_cjs/issues/{Error resolving commonjs request-a634e0.txt => Error resolving commonjs request-b97684.txt} (100%) rename crates/turbopack-tests/tests/snapshot/imports/resolve_error_esm/issues/{Error resolving EcmaScript Modules request-cd3585.txt => Error resolving EcmaScript Modules request-aff400.txt} (100%) 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 From dac29fcd11eba81c3a921b4e0d573cdd47f41e71 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Wed, 5 Apr 2023 19:20:08 -0400 Subject: [PATCH 6/6] Split Dotenv parsing methods --- crates/turbo-tasks-env/src/dotenv.rs | 79 ++++++++++------------------ crates/turbo-tasks-env/src/lib.rs | 8 +-- crates/turbopack-env/src/try_env.rs | 36 ++++++------- 3 files changed, 50 insertions(+), 73 deletions(-) diff --git a/crates/turbo-tasks-env/src/dotenv.rs b/crates/turbo-tasks-env/src/dotenv.rs index b04174907493b..11516d1b61d48 100644 --- a/crates/turbo-tasks-env/src/dotenv.rs +++ b/crates/turbo-tasks-env/src/dotenv.rs @@ -1,6 +1,6 @@ use std::{env, sync::MutexGuard}; -use anyhow::{anyhow, Error, Result}; +use anyhow::{anyhow, Context, Result}; use indexmap::IndexMap; use turbo_tasks::ValueToString; use turbo_tasks_fs::{FileContent, FileSystemPathVc}; @@ -16,39 +16,28 @@ pub struct DotenvProcessEnv { path: FileSystemPathVc, } -/// Dotenv loading depends on prior state to resolve the current state. This -/// exposes the origin of a failed parse, so that callers can determine if the -/// prior state failed, or parsing of the current dotenv failed. -pub enum DotenvReadResult { - /// A PriorError is an error that happens during the read_all of the prior - /// [ProcessEnvVc]. - PriorError(Error), - - /// A CurrentError is an error that happens during the read/parse of the - /// `.env` file. - CurrentError(Error), +#[turbo_tasks::value_impl] +impl DotenvProcessEnvVc { + #[turbo_tasks::function] + pub fn new(prior: Option, path: FileSystemPathVc) -> Self { + DotenvProcessEnv { prior, path }.cell() + } - Ok(EnvMapVc), -} + #[turbo_tasks::function] + pub async fn read_prior(self) -> Result { + let this = self.await?; + match this.prior { + None => Ok(EnvMapVc::empty()), + Some(p) => Ok(p.read_all()), + } + } -impl DotenvProcessEnv { - /// Attempts to assemble the EnvMapVc for our dotenv file. If either the - /// prior fails to read, or the current dotenv can't be parsed, an - /// appropriate Ok(DotenvReadResult) will be returned. If an unexpected - /// error (like disk reading or remote cache access) fails, then a regular - /// Err() will be returned. - pub async fn try_read_all(&self) -> Result { - let prior = match self.prior { - None => None, - Some(p) => match p.read_all().await { - Ok(p) => Some(p), - Err(e) => return Ok(DotenvReadResult::PriorError(e)), - }, - }; - let empty = IndexMap::new(); - let prior = prior.as_deref().unwrap_or(&empty); + #[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; @@ -60,7 +49,7 @@ impl 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 @@ -72,37 +61,25 @@ impl DotenvProcessEnv { } if let Err(e) = res { - return Ok(DotenvReadResult::CurrentError(anyhow!(e).context(anyhow!( + return Err(e).context(anyhow!( "unable to read {} for env vars", - self.path.to_string().await? - )))); + this.path.to_string().await? + )); } - Ok(DotenvReadResult::Ok(EnvMapVc::cell(vars))) + Ok(EnvMapVc::cell(vars)) } else { - Ok(DotenvReadResult::Ok(EnvMapVc::cell(prior.clone()))) + Ok(EnvMapVc::cell(prior.clone_value())) } } } -#[turbo_tasks::value_impl] -impl DotenvProcessEnvVc { - #[turbo_tasks::function] - 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_vc: DotenvProcessEnvVc) -> Result { - let this = self_vc.await?; - match this.try_read_all().await? { - DotenvReadResult::Ok(v) => Ok(v), - DotenvReadResult::PriorError(e) => Err(e), - DotenvReadResult::CurrentError(e) => Err(e), - } + let prior = self_vc.read_prior(); + Ok(self_vc.read_all_with_prior(prior)) } } diff --git a/crates/turbo-tasks-env/src/lib.rs b/crates/turbo-tasks-env/src/lib.rs index d1a9f18b845d9..fbe06368e75f6 100644 --- a/crates/turbo-tasks-env/src/lib.rs +++ b/crates/turbo-tasks-env/src/lib.rs @@ -1,9 +1,9 @@ #![feature(min_specialization)] -pub mod command_line; -pub mod custom; -pub mod dotenv; -pub mod filter; +mod command_line; +mod custom; +mod dotenv; +mod filter; use std::{env, sync::Mutex}; diff --git a/crates/turbopack-env/src/try_env.rs b/crates/turbopack-env/src/try_env.rs index df1b534cce72f..f8b1db562148c 100644 --- a/crates/turbopack-env/src/try_env.rs +++ b/crates/turbopack-env/src/try_env.rs @@ -1,8 +1,6 @@ use anyhow::Result; use turbo_tasks::primitives::StringVc; -use turbo_tasks_env::{ - dotenv::DotenvReadResult, DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc, -}; +use turbo_tasks_env::{DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc}; use turbo_tasks_fs::FileSystemPathVc; use crate::ProcessEnvIssue; @@ -32,29 +30,31 @@ impl TryDotenvProcessEnvVc { impl ProcessEnv for TryDotenvProcessEnv { #[turbo_tasks::function] async fn read_all(&self) -> Result { - let dotenv = self.dotenv.await?; - match dotenv.try_read_all().await? { - DotenvReadResult::Ok(v) => Ok(v), - DotenvReadResult::PriorError(e) => { - // If reading the prior value failed, then we cannot determine what the read - // could have been (the dotenv depends on the state of the prior to build) - // build). Trust the prior will emit an issue, and error out. - Err(e) - } - DotenvReadResult::CurrentError(e) => { + let dotenv = self.dotenv; + let prior = dotenv.read_prior(); + + // 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 error and fall back to the prior's read. + // we want to emit an Issue and fall back to the prior's read. ProcessEnvIssue { path: self.path, - // try_read_all 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. + // 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(self.prior.read_all()) + Ok(prior) } } }