Split Dotenv parsing methods

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<ProcessEnvVc>, path: FileSystemPathVc) -> Self {
+        DotenvProcessEnv { prior, path }.cell()
+    }
 
-    Ok(EnvMapVc),
-}
+    #[turbo_tasks::function]
+    pub async fn read_prior(self) -> Result<EnvMapVc> {
+        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<DotenvReadResult> {
-        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<EnvMapVc> {
+        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<ProcessEnvVc>, 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<EnvMapVc> {
-        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))
     }
 }
 

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};
 

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<EnvMapVc> {
-        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)
             }
         }
     }