Display environment loading failures in the activity indicator (#18567)

As @maan2003 noted in #18473, we should warn the user if direnv call
fails

Release Notes:

- Show a notice in the activity indicator if an error occurs while
loading the shell environment

crates/activity_indicator/src/activity_indicator.rs

@@ -10,7 +10,7 @@ use gpui::{
 use language::{
     LanguageRegistry, LanguageServerBinaryStatus, LanguageServerId, LanguageServerName,
 };
-use project::{LanguageServerProgress, Project};
+use project::{EnvironmentErrorMessage, LanguageServerProgress, Project, WorktreeId};
 use smallvec::SmallVec;
 use std::{cmp::Reverse, fmt::Write, sync::Arc, time::Duration};
 use ui::{prelude::*, ButtonLike, ContextMenu, PopoverMenu, PopoverMenuHandle};
@@ -175,7 +175,30 @@ impl ActivityIndicator {
             .flatten()
     }
 
+    fn pending_environment_errors<'a>(
+        &'a self,
+        cx: &'a AppContext,
+    ) -> impl Iterator<Item = (&'a WorktreeId, &'a EnvironmentErrorMessage)> {
+        self.project.read(cx).shell_environment_errors(cx)
+    }
+
     fn content_to_render(&mut self, cx: &mut ViewContext<Self>) -> Option<Content> {
+        // Show if any direnv calls failed
+        if let Some((&worktree_id, error)) = self.pending_environment_errors(cx).next() {
+            return Some(Content {
+                icon: Some(
+                    Icon::new(IconName::Warning)
+                        .size(IconSize::Small)
+                        .into_any_element(),
+                ),
+                message: error.0.clone(),
+                on_click: Some(Arc::new(move |this, cx| {
+                    this.project.update(cx, |project, cx| {
+                        project.remove_environment_error(cx, worktree_id);
+                    })
+                })),
+            });
+        }
         // Show any language server has pending activity.
         let mut pending_work = self.pending_language_server_work(cx);
         if let Some(PendingWork {

crates/project/src/direnv.rs

@@ -0,0 +1,72 @@
+use crate::environment::EnvironmentErrorMessage;
+use std::process::ExitStatus;
+
+#[cfg(not(any(test, feature = "test-support")))]
+use {collections::HashMap, std::path::Path, util::ResultExt};
+
+#[derive(Clone)]
+pub enum DirenvError {
+    NotFound,
+    FailedRun,
+    NonZeroExit(ExitStatus, Vec<u8>),
+    EmptyOutput,
+    InvalidJson,
+}
+
+impl From<DirenvError> for Option<EnvironmentErrorMessage> {
+    fn from(value: DirenvError) -> Self {
+        match value {
+            DirenvError::NotFound => None,
+            DirenvError::FailedRun | DirenvError::NonZeroExit(_, _) => {
+                Some(EnvironmentErrorMessage(String::from(
+                    "Failed to run direnv. See logs for more info",
+                )))
+            }
+            DirenvError::EmptyOutput => None,
+            DirenvError::InvalidJson => Some(EnvironmentErrorMessage(String::from(
+                "Direnv returned invalid json. See logs for more info",
+            ))),
+        }
+    }
+}
+
+#[cfg(not(any(test, feature = "test-support")))]
+pub async fn load_direnv_environment(dir: &Path) -> Result<HashMap<String, String>, DirenvError> {
+    let Ok(direnv_path) = which::which("direnv") else {
+        return Err(DirenvError::NotFound);
+    };
+
+    let Some(direnv_output) = smol::process::Command::new(direnv_path)
+        .args(["export", "json"])
+        .env("TERM", "dumb")
+        .current_dir(dir)
+        .output()
+        .await
+        .log_err()
+    else {
+        return Err(DirenvError::FailedRun);
+    };
+
+    if !direnv_output.status.success() {
+        log::error!(
+            "Loading direnv environment failed ({}), stderr: {}",
+            direnv_output.status,
+            String::from_utf8_lossy(&direnv_output.stderr)
+        );
+        return Err(DirenvError::NonZeroExit(
+            direnv_output.status,
+            direnv_output.stderr,
+        ));
+    }
+
+    let output = String::from_utf8_lossy(&direnv_output.stdout);
+    if output.is_empty() {
+        return Err(DirenvError::EmptyOutput);
+    }
+
+    let Some(env) = serde_json::from_str(&output).log_err() else {
+        return Err(DirenvError::InvalidJson);
+    };
+
+    Ok(env)
+}

crates/project/src/environment.rs

@@ -1,4 +1,3 @@
-use anyhow::Result;
 use futures::{future::Shared, FutureExt};
 use std::{path::Path, sync::Arc};
 use util::ResultExt;
@@ -17,6 +16,7 @@ pub struct ProjectEnvironment {
     cli_environment: Option<HashMap<String, String>>,
     get_environment_task: Option<Shared<Task<Option<HashMap<String, String>>>>>,
     cached_shell_environments: HashMap<WorktreeId, HashMap<String, String>>,
+    environment_error_messages: HashMap<WorktreeId, EnvironmentErrorMessage>,
 }
 
 impl ProjectEnvironment {
@@ -37,6 +37,7 @@ impl ProjectEnvironment {
                 cli_environment,
                 get_environment_task: None,
                 cached_shell_environments: Default::default(),
+                environment_error_messages: Default::default(),
             }
         })
     }
@@ -54,6 +55,7 @@ impl ProjectEnvironment {
 
     pub(crate) fn remove_worktree_environment(&mut self, worktree_id: WorktreeId) {
         self.cached_shell_environments.remove(&worktree_id);
+        self.environment_error_messages.remove(&worktree_id);
     }
 
     /// Returns the inherited CLI environment, if this project was opened from the Zed CLI.
@@ -66,6 +68,18 @@ impl ProjectEnvironment {
         }
     }
 
+    /// Returns an iterator over all pairs `(worktree_id, error_message)` of
+    /// environment errors associated with this project environment.
+    pub(crate) fn environment_errors(
+        &self,
+    ) -> impl Iterator<Item = (&WorktreeId, &EnvironmentErrorMessage)> {
+        self.environment_error_messages.iter()
+    }
+
+    pub(crate) fn remove_environment_error(&mut self, worktree_id: WorktreeId) {
+        self.environment_error_messages.remove(&worktree_id);
+    }
+
     /// Returns the project environment, if possible.
     /// If the project was opened from the CLI, then the inherited CLI environment is returned.
     /// If it wasn't opened from the CLI, and a worktree is given, then a shell is spawned in
@@ -120,25 +134,31 @@ impl ProjectEnvironment {
             let load_direnv = ProjectSettings::get_global(cx).load_direnv.clone();
 
             cx.spawn(|this, mut cx| async move {
-                let mut shell_env = cx
+                let (mut shell_env, error) = cx
                     .background_executor()
                     .spawn({
                         let cwd = worktree_abs_path.clone();
                         async move { load_shell_environment(&cwd, &load_direnv).await }
                     })
-                    .await
+                    .await;
-                    .ok();
 
                 if let Some(shell_env) = shell_env.as_mut() {
                     this.update(&mut cx, |this, _| {
                         this.cached_shell_environments
-                            .insert(worktree_id, shell_env.clone())
+                            .insert(worktree_id, shell_env.clone());
                     })
                     .log_err();
 
                     set_origin_marker(shell_env, EnvironmentOrigin::WorktreeShell);
                 }
 
+                if let Some(error) = error {
+                    this.update(&mut cx, |this, _| {
+                        this.environment_error_messages.insert(worktree_id, error);
+                    })
+                    .log_err();
+                }
+
                 shell_env
             })
         }
@@ -165,64 +185,62 @@ impl From<EnvironmentOrigin> for String {
     }
 }
 
+pub struct EnvironmentErrorMessage(pub String);
+
+impl EnvironmentErrorMessage {
+    #[allow(dead_code)]
+    fn from_str(s: &str) -> Self {
+        Self(String::from(s))
+    }
+}
+
 #[cfg(any(test, feature = "test-support"))]
 async fn load_shell_environment(
     _dir: &Path,
     _load_direnv: &DirenvSettings,
-) -> Result<HashMap<String, String>> {
+) -> (
-    Ok([("ZED_FAKE_TEST_ENV".into(), "true".into())]
+    Option<HashMap<String, String>>,
+    Option<EnvironmentErrorMessage>,
+) {
+    let fake_env = [("ZED_FAKE_TEST_ENV".into(), "true".into())]
         .into_iter()
-        .collect())
+        .collect();
+    (Some(fake_env), None)
 }
 
 #[cfg(not(any(test, feature = "test-support")))]
 async fn load_shell_environment(
     dir: &Path,
     load_direnv: &DirenvSettings,
-) -> Result<HashMap<String, String>> {
+) -> (
-    use anyhow::{anyhow, Context};
+    Option<HashMap<String, String>>,
+    Option<EnvironmentErrorMessage>,
+) {
+    use crate::direnv::{load_direnv_environment, DirenvError};
     use std::path::PathBuf;
     use util::parse_env_output;
 
-    async fn load_direnv_environment(dir: &Path) -> Result<Option<HashMap<String, String>>> {
+    fn message<T>(with: &str) -> (Option<T>, Option<EnvironmentErrorMessage>) {
-        let Ok(direnv_path) = which::which("direnv") else {
+        let message = EnvironmentErrorMessage::from_str(with);
-            return Ok(None);
+        (None, Some(message))
-        };
-
-        let direnv_output = smol::process::Command::new(direnv_path)
-            .args(["export", "json"])
-            .current_dir(dir)
-            .output()
-            .await
-            .context("failed to spawn direnv to get local environment variables")?;
-
-        anyhow::ensure!(
-            direnv_output.status.success(),
-            "direnv exited with error {:?}. Stderr:\n{}",
-            direnv_output.status,
-            String::from_utf8_lossy(&direnv_output.stderr)
-        );
-
-        let output = String::from_utf8_lossy(&direnv_output.stdout);
-        if output.is_empty() {
-            return Ok(None);
-        }
-
-        Ok(Some(
-            serde_json::from_str(&output).context("failed to parse direnv output")?,
-        ))
     }
 
-    let direnv_environment = match load_direnv {
+    let (direnv_environment, direnv_error) = match load_direnv {
-        DirenvSettings::ShellHook => None,
+        DirenvSettings::ShellHook => (None, None),
-        DirenvSettings::Direct => load_direnv_environment(dir).await.log_err().flatten(),
+        DirenvSettings::Direct => match load_direnv_environment(dir).await {
-    }
+            Ok(env) => (Some(env), None),
-    .unwrap_or(HashMap::default());
+            Err(err) => (
+                None,
+                <Option<EnvironmentErrorMessage> as From<DirenvError>>::from(err),
+            ),
+        },
+    };
+    let direnv_environment = direnv_environment.unwrap_or(HashMap::default());
 
     let marker = "ZED_SHELL_START";
-    let shell = std::env::var("SHELL").context(
+    let Some(shell) = std::env::var("SHELL").log_err() else {
-        "SHELL environment variable is not assigned so we can't source login environment variables",
+        return message("Failed to get login environment. SHELL environment variable is not set");
-    )?;
+    };
 
     // What we're doing here is to spawn a shell and then `cd` into
     // the project directory to get the env in there as if the user
@@ -259,26 +277,26 @@ async fn load_shell_environment(
         additional_command.unwrap_or("")
     );
 
-    let output = smol::process::Command::new(&shell)
+    let Some(output) = smol::process::Command::new(&shell)
         .args(["-l", "-i", "-c", &command])
         .envs(direnv_environment)
         .output()
         .await
-        .context("failed to spawn login shell to source login environment variables")?;
+        .log_err()
+    else {
+        return message("Failed to spawn login shell to source login environment variables. See logs for details");
+    };
 
-    anyhow::ensure!(
+    if !output.status.success() {
-        output.status.success(),
+        log::error!("login shell exited with {}", output.status);
-        "login shell exited with error {:?}",
+        return message("Login shell exited with nonzero exit code. See logs for details");
-        output.status
+    }
-    );
 
     let stdout = String::from_utf8_lossy(&output.stdout);
-    let env_output_start = stdout.find(marker).ok_or_else(|| {
+    let Some(env_output_start) = stdout.find(marker) else {
-        anyhow!(
+        log::error!("failed to parse output of `env` command in login shell: {stdout}");
-            "failed to parse output of `env` command in login shell: {}",
+        return message("Failed to parse stdout of env command. See logs for the output");
-            stdout
+    };
-        )
-    })?;
 
     let mut parsed_env = HashMap::default();
     let env_output = &stdout[env_output_start + marker.len()..];
@@ -287,5 +305,5 @@ async fn load_shell_environment(
         parsed_env.insert(key, value);
     });
 
-    Ok(parsed_env)
+    (Some(parsed_env), direnv_error)
 }

crates/project/src/project.rs

@@ -15,7 +15,9 @@ pub mod worktree_store;
 #[cfg(test)]
 mod project_tests;
 
+mod direnv;
 mod environment;
+pub use environment::EnvironmentErrorMessage;
 pub mod search_history;
 mod yarn;
 
@@ -1185,6 +1187,23 @@ impl Project {
         self.environment.read(cx).get_cli_environment()
     }
 
+    pub fn shell_environment_errors<'a>(
+        &'a self,
+        cx: &'a AppContext,
+    ) -> impl Iterator<Item = (&'a WorktreeId, &'a EnvironmentErrorMessage)> {
+        self.environment.read(cx).environment_errors()
+    }
+
+    pub fn remove_environment_error(
+        &mut self,
+        cx: &mut ModelContext<Self>,
+        worktree_id: WorktreeId,
+    ) {
+        self.environment.update(cx, |environment, _| {
+            environment.remove_environment_error(worktree_id);
+        });
+    }
+
     #[cfg(any(test, feature = "test-support"))]
     pub fn has_open_buffer(&self, path: impl Into<ProjectPath>, cx: &AppContext) -> bool {
         self.buffer_store