From 70d6c2bdc4e6238d55b147555a1e3a06c8c8fc71 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Tue, 7 Apr 2026 12:18:15 -0400 Subject: [PATCH] git_graph: Show propagated errors from git binary command (#53320) Based on commit fba49809b39b0f9e58d68e3956f5c24fd47121d7 that I worked with Dino on in PR: #50288 Co-authored-by Dino \ Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - N/A or Added/Fixed/Improved ... --- crates/fs/src/fake_git_repo.rs | 15 ++++++- crates/fs/src/fs.rs | 7 +++ crates/git/src/repository.rs | 18 +++++++- crates/git_graph/src/git_graph.rs | 71 +++++++++++++++++++++++++++++-- 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index fbebeabf0ac15dde80016958eb358f792f46dd50..7b89a0751f17ef8c2bba837882f2a31c7d5451e5 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -61,6 +61,7 @@ pub struct FakeGitRepositoryState { pub remotes: HashMap, pub simulated_index_write_error_message: Option, pub simulated_create_worktree_error: Option, + pub simulated_graph_error: Option, pub refs: HashMap, pub graph_commits: Vec>, pub stash_entries: GitStash, @@ -78,6 +79,7 @@ impl FakeGitRepositoryState { branches: Default::default(), simulated_index_write_error_message: Default::default(), simulated_create_worktree_error: Default::default(), + simulated_graph_error: None, refs: HashMap::from_iter([("HEAD".into(), "abc".into())]), merge_base_contents: Default::default(), oids: Default::default(), @@ -1327,8 +1329,17 @@ impl GitRepository for FakeGitRepository { let fs = self.fs.clone(); let dot_git_path = self.dot_git_path.clone(); async move { - let graph_commits = - fs.with_git_state(&dot_git_path, false, |state| state.graph_commits.clone())?; + let (graph_commits, simulated_error) = + fs.with_git_state(&dot_git_path, false, |state| { + ( + state.graph_commits.clone(), + state.simulated_graph_error.clone(), + ) + })?; + + if let Some(error) = simulated_error { + anyhow::bail!("{}", error); + } for chunk in graph_commits.chunks(GRAPH_CHUNK_SIZE) { request_tx.send(chunk.to_vec()).await.ok(); diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index a26abb81255003e4059f9bcc8a68aa3c6212a73a..52cae537b6f00837b50123af0cae7c093699dedf 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -2168,6 +2168,13 @@ impl FakeFs { .unwrap(); } + pub fn set_graph_error(&self, dot_git: &Path, error: Option) { + self.with_git_state(dot_git, true, |state| { + state.simulated_graph_error = error; + }) + .unwrap(); + } + /// Put the given git repository into a state with the given status, /// by mutating the head, index, and unmerged state. pub fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&str, FileStatus)]) { diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index ba717d00c5e40374f5315d3ee8bc12e671f09552..d7049c0a50cb94c049556e395e818dbbddfb89bf 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -2784,10 +2784,11 @@ impl GitRepository for RealGitRepository { log_source.get_arg()?, ]); command.stdout(Stdio::piped()); - command.stderr(Stdio::null()); + command.stderr(Stdio::piped()); let mut child = command.spawn()?; let stdout = child.stdout.take().context("failed to get stdout")?; + let stderr = child.stderr.take().context("failed to get stderr")?; let mut reader = BufReader::new(stdout); let mut line_buffer = String::new(); @@ -2822,7 +2823,20 @@ impl GitRepository for RealGitRepository { } } - child.status().await?; + let status = child.status().await?; + if !status.success() { + let mut stderr_output = String::new(); + BufReader::new(stderr) + .read_to_string(&mut stderr_output) + .await + .log_err(); + + if stderr_output.is_empty() { + anyhow::bail!("git log command failed with {}", status); + } else { + anyhow::bail!("git log command failed with {}: {}", status, stderr_output); + } + } Ok(()) } .boxed() diff --git a/crates/git_graph/src/git_graph.rs b/crates/git_graph/src/git_graph.rs index aa5f6bc6e1293cfd057baa0c5e9f77819da71086..7594a206f14705bf47a673dee9abefad5a3446de 100644 --- a/crates/git_graph/src/git_graph.rs +++ b/crates/git_graph/src/git_graph.rs @@ -2536,11 +2536,19 @@ impl Render for GitGraph { } }; + let error = self.get_repository(cx).and_then(|repo| { + repo.read(cx) + .get_graph_data(self.log_source.clone(), self.log_order) + .and_then(|data| data.error.clone()) + }); + let content = if commit_count == 0 { - let message = if is_loading { - "Loading" + let message = if let Some(error) = &error { + format!("Error loading: {}", error) + } else if is_loading { + "Loading".to_string() } else { - "No commits found" + "No commits found".to_string() }; let label = Label::new(message) .color(Color::Muted) @@ -2552,7 +2560,7 @@ impl Render for GitGraph { .items_center() .justify_center() .child(label) - .when(is_loading, |this| { + .when(is_loading && error.is_none(), |this| { this.child(self.render_loading_spinner(cx)) }) } else { @@ -3757,6 +3765,61 @@ mod tests { ); } + #[gpui::test] + async fn test_initial_graph_data_propagates_error(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + Path::new("/project"), + json!({ + ".git": {}, + "file.txt": "content", + }), + ) + .await; + + fs.set_graph_error( + Path::new("/project/.git"), + Some("fatal: bad default revision 'HEAD'".to_string()), + ); + + let project = Project::test(fs.clone(), [Path::new("/project")], cx).await; + + let repository = project.read_with(cx, |project, cx| { + project + .active_repository(cx) + .expect("should have a repository") + }); + + repository.update(cx, |repo, cx| { + repo.graph_data( + crate::LogSource::default(), + crate::LogOrder::default(), + 0..usize::MAX, + cx, + ); + }); + + cx.run_until_parked(); + + let error = repository.read_with(cx, |repo, _| { + repo.get_graph_data(crate::LogSource::default(), crate::LogOrder::default()) + .and_then(|data| data.error.clone()) + }); + + assert!( + error.is_some(), + "graph data should contain an error after initial_graph_data fails" + ); + let error_message = error.unwrap(); + assert!( + error_message.contains("bad default revision"), + "error should contain the git error message, got: {}", + error_message + ); + } + #[gpui::test] async fn test_graph_data_repopulated_from_cache_after_repo_switch(cx: &mut TestAppContext) { init_test(cx);