mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
devcontainer: Fix git output (#49230)
Closes #48434 In Dev Containers, failed git operations were surfaced with a generic failure message, while the useful git output (stderr/stdout) was not reliably available to users. This happened because in devcontainers the git operation errors go through an RPC layer and then got wrapped with `anyhow::Context` (e.g. “sending pull request”); the toast displayed only that outer context via `to_string()`, masking the underlying git stderr message. This change ensures the full git operation output is preserved and surfaced via Zed’s “See logs” flow in Dev Containers, matching the information you get when running the same git command in a terminal. ### What you should expect in the UI - You will see a generic toast like “git pull failed” / “git push failed”. - When clicking on the toast’s “See logs”, the log tab now contains the full git error output (e.g. non-fast-forward hints, merge conflict details, “local changes would be overwritten”, etc.), which previously could be missing/too generic. --- ## Manual testing Run inside a Dev Container and ensure git auth works (SSH keys/agent or HTTPS credentials). 1. **Dirty-tree pull failure** - Make remote ahead by 1 commit (push from another clone). - Locally modify the same file without committing. - In Zed: **Pull** - **Expect:** toast “git pull failed” + **See logs** shows “local changes would be overwritten…” (or equivalent). 2. **Non-fast-forward push failure** - Ensure remote ahead. - Locally create 1 commit. - In Zed: **Push** - **Expect:** toast “git push failed” + **See logs** shows “rejected (non-fast-forward)” + hint to pull first. 3. **Merge-conflict pull failure** - Create conflicting commits on the same lines (one local commit, one remote commit). - In Zed: **Pull** - **Expect:** toast “git pull failed” + **See logs** shows conflict output (“CONFLICT…”, “Automatic merge failed…”). Release Notes: - Fixed devcontainer git failure toasts so they show the actual git error --------- Co-authored-by: KyleBarton <kjb@initialcapacity.io>
This commit is contained in:
parent
2b774e5cd2
commit
f9895c5468
5 changed files with 70 additions and 11 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -7310,6 +7310,7 @@ dependencies = [
|
|||
"pretty_assertions",
|
||||
"project",
|
||||
"prompt_store",
|
||||
"proto",
|
||||
"rand 0.9.2",
|
||||
"remote",
|
||||
"remote_connection",
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ panel.workspace = true
|
|||
picker.workspace = true
|
||||
project.workspace = true
|
||||
prompt_store.workspace = true
|
||||
proto.workspace = true
|
||||
remote_connection.workspace = true
|
||||
remote.workspace = true
|
||||
schemars.workspace = true
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@ use project::{
|
|||
project_settings::{GitPathStyle, ProjectSettings},
|
||||
};
|
||||
use prompt_store::{BuiltInPrompt, PromptId, PromptStore, RULES_FILE_NAMES};
|
||||
use proto::RpcError;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use settings::{Settings, SettingsStore, StatusStyle};
|
||||
use smallvec::SmallVec;
|
||||
|
|
@ -6420,7 +6421,7 @@ pub(crate) fn show_error_toast(
|
|||
cx: &mut App,
|
||||
) {
|
||||
let action = action.into();
|
||||
let message = e.to_string().trim().to_string();
|
||||
let message = format_git_error_toast_message(&e);
|
||||
if message
|
||||
.matches(git::repository::REMOTE_CANCELLED_BY_USER)
|
||||
.next()
|
||||
|
|
@ -6446,6 +6447,20 @@ pub(crate) fn show_error_toast(
|
|||
}
|
||||
}
|
||||
|
||||
fn rpc_error_raw_message_from_chain(error: &anyhow::Error) -> Option<&str> {
|
||||
error
|
||||
.chain()
|
||||
.find_map(|cause| cause.downcast_ref::<RpcError>().map(RpcError::raw_message))
|
||||
}
|
||||
|
||||
fn format_git_error_toast_message(error: &anyhow::Error) -> String {
|
||||
if let Some(message) = rpc_error_raw_message_from_chain(error) {
|
||||
message.trim().to_string()
|
||||
} else {
|
||||
error.to_string().trim().to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use git::{
|
||||
|
|
@ -6477,6 +6492,47 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_git_error_toast_message_prefers_raw_rpc_message() {
|
||||
let rpc_error = RpcError::from_proto(
|
||||
&proto::Error {
|
||||
message:
|
||||
"Your local changes to the following files would be overwritten by merge\n"
|
||||
.to_string(),
|
||||
code: proto::ErrorCode::Internal as i32,
|
||||
tags: Default::default(),
|
||||
},
|
||||
"Pull",
|
||||
);
|
||||
|
||||
let message = format_git_error_toast_message(&rpc_error);
|
||||
assert_eq!(
|
||||
message,
|
||||
"Your local changes to the following files would be overwritten by merge"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_git_error_toast_message_prefers_raw_rpc_message_when_wrapped() {
|
||||
let rpc_error = RpcError::from_proto(
|
||||
&proto::Error {
|
||||
message:
|
||||
"Your local changes to the following files would be overwritten by merge\n"
|
||||
.to_string(),
|
||||
code: proto::ErrorCode::Internal as i32,
|
||||
tags: Default::default(),
|
||||
},
|
||||
"Pull",
|
||||
);
|
||||
let wrapped = rpc_error.context("sending pull request");
|
||||
|
||||
let message = format_git_error_toast_message(&wrapped);
|
||||
assert_eq!(
|
||||
message,
|
||||
"Your local changes to the following files would be overwritten by merge"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_entry_worktree_paths(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
|
|
|||
|
|
@ -4956,8 +4956,7 @@ impl Repository {
|
|||
.map(|repo_path| repo_path.to_proto())
|
||||
.collect(),
|
||||
})
|
||||
.await
|
||||
.context("sending stash request")?;
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
@ -5166,8 +5165,7 @@ impl Repository {
|
|||
}),
|
||||
askpass_id,
|
||||
})
|
||||
.await
|
||||
.context("sending commit request")?;
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -5206,8 +5204,7 @@ impl Repository {
|
|||
askpass_id,
|
||||
remote: fetch_options.to_proto(),
|
||||
})
|
||||
.await
|
||||
.context("sending fetch request")?;
|
||||
.await?;
|
||||
|
||||
Ok(RemoteCommandOutput {
|
||||
stdout: response.stdout,
|
||||
|
|
@ -5308,8 +5305,7 @@ impl Repository {
|
|||
}
|
||||
as i32),
|
||||
})
|
||||
.await
|
||||
.context("sending push request")?;
|
||||
.await?;
|
||||
|
||||
Ok(RemoteCommandOutput {
|
||||
stdout: response.stdout,
|
||||
|
|
@ -5375,8 +5371,7 @@ impl Repository {
|
|||
branch_name: branch.as_ref().map(|b| b.to_string()),
|
||||
remote_name: remote.to_string(),
|
||||
})
|
||||
.await
|
||||
.context("sending pull request")?;
|
||||
.await?;
|
||||
|
||||
Ok(RemoteCommandOutput {
|
||||
stdout: response.stdout,
|
||||
|
|
|
|||
|
|
@ -159,6 +159,12 @@ pub struct RpcError {
|
|||
/// in the app; however it is useful for chaining .message() and .with_tag() on
|
||||
/// ErrorCode.
|
||||
impl RpcError {
|
||||
/// Returns the raw server-provided error message without any RPC framing
|
||||
/// (e.g. without the "RPC request X failed: " prefix that `Display` adds).
|
||||
pub fn raw_message(&self) -> &str {
|
||||
&self.msg
|
||||
}
|
||||
|
||||
/// from_proto converts a crate::Error into an anyhow::Error containing
|
||||
/// an RpcError.
|
||||
pub fn from_proto(error: &crate::Error, request: &str) -> anyhow::Error {
|
||||
|
|
|
|||
Loading…
Reference in a new issue