Better error handling for SSH (#19533)
Before this change we sometimes showed errors inline, sometimes in alerts. Sometimes we closed the window, someimtes we didn't. Now they always show as prompts and we never close windows. Co-Authored-By: Mikayla <mikayla@zed.dev> Release Notes: - SSH Remoting: Improve error handling
This commit is contained in:
Conrad Irwin
GitHub
parent
6e485453d0
commit
4f52077d97
3 changed files with 107 additions and 136 deletions
|
|
@ -481,9 +481,7 @@ impl RemoteServerProjects {
|
|||
let connection_options = ssh_connection.into();
|
||||
workspace.update(cx, |_, cx| {
|
||||
cx.defer(move |workspace, cx| {
|
||||
workspace.toggle_modal(cx, |cx| {
|
||||
SshConnectionModal::new(&connection_options, false, cx)
|
||||
});
|
||||
workspace.toggle_modal(cx, |cx| SshConnectionModal::new(&connection_options, cx));
|
||||
let prompt = workspace
|
||||
.active_modal::<SshConnectionModal>(cx)
|
||||
.unwrap()
|
||||
|
|
@ -498,8 +496,19 @@ impl RemoteServerProjects {
|
|||
cx,
|
||||
)
|
||||
.prompt_err("Failed to connect", cx, |_, _| None);
|
||||
|
||||
cx.spawn(move |workspace, mut cx| async move {
|
||||
let Some(session) = connect.await else {
|
||||
let session = connect.await;
|
||||
|
||||
workspace
|
||||
.update(&mut cx, |workspace, cx| {
|
||||
if let Some(prompt) = workspace.active_modal::<SshConnectionModal>(cx) {
|
||||
prompt.update(cx, |prompt, cx| prompt.finished(cx))
|
||||
}
|
||||
})
|
||||
.ok();
|
||||
|
||||
let Some(session) = session else {
|
||||
workspace
|
||||
.update(&mut cx, |workspace, cx| {
|
||||
let weak = cx.view().downgrade();
|
||||
|
|
|
|||
|
|
@ -6,8 +6,8 @@ use editor::Editor;
|
|||
use futures::channel::oneshot;
|
||||
use gpui::{
|
||||
percentage, px, Animation, AnimationExt, AnyWindowHandle, AsyncAppContext, DismissEvent,
|
||||
EventEmitter, FocusableView, ParentElement as _, Render, SemanticVersion, SharedString, Task,
|
||||
TextStyleRefinement, Transformation, View,
|
||||
EventEmitter, FocusableView, ParentElement as _, PromptLevel, Render, SemanticVersion,
|
||||
SharedString, Task, TextStyleRefinement, Transformation, View,
|
||||
};
|
||||
use gpui::{AppContext, Model};
|
||||
|
||||
|
|
@ -104,14 +104,13 @@ impl Settings for SshSettings {
|
|||
pub struct SshPrompt {
|
||||
connection_string: SharedString,
|
||||
status_message: Option<SharedString>,
|
||||
error_message: Option<SharedString>,
|
||||
prompt: Option<(View<Markdown>, oneshot::Sender<Result<String>>)>,
|
||||
editor: View<Editor>,
|
||||
}
|
||||
|
||||
pub struct SshConnectionModal {
|
||||
pub(crate) prompt: View<SshPrompt>,
|
||||
is_separate_window: bool,
|
||||
finished: bool,
|
||||
}
|
||||
|
||||
impl SshPrompt {
|
||||
|
|
@ -123,7 +122,6 @@ impl SshPrompt {
|
|||
Self {
|
||||
connection_string,
|
||||
status_message: None,
|
||||
error_message: None,
|
||||
prompt: None,
|
||||
editor: cx.new_view(Editor::single_line),
|
||||
}
|
||||
|
|
@ -173,11 +171,6 @@ impl SshPrompt {
|
|||
cx.notify();
|
||||
}
|
||||
|
||||
pub fn set_error(&mut self, error_message: String, cx: &mut ViewContext<Self>) {
|
||||
self.error_message = Some(error_message.into());
|
||||
cx.notify();
|
||||
}
|
||||
|
||||
pub fn confirm(&mut self, cx: &mut ViewContext<Self>) {
|
||||
if let Some((_, tx)) = self.prompt.take() {
|
||||
self.status_message = Some("Connecting".into());
|
||||
|
|
@ -196,59 +189,27 @@ impl Render for SshPrompt {
|
|||
v_flex()
|
||||
.key_context("PasswordPrompt")
|
||||
.size_full()
|
||||
.when(
|
||||
self.error_message.is_some() || self.status_message.is_some(),
|
||||
|el| {
|
||||
el.child(
|
||||
h_flex()
|
||||
.p_2()
|
||||
.flex()
|
||||
.child(if self.error_message.is_some() {
|
||||
Icon::new(IconName::XCircle)
|
||||
.size(IconSize::Medium)
|
||||
.color(Color::Error)
|
||||
.into_any_element()
|
||||
} else {
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Medium)
|
||||
.with_animation(
|
||||
"arrow-circle",
|
||||
Animation::new(Duration::from_secs(2)).repeat(),
|
||||
|icon, delta| {
|
||||
icon.transform(Transformation::rotate(percentage(
|
||||
delta,
|
||||
)))
|
||||
},
|
||||
)
|
||||
.into_any_element()
|
||||
})
|
||||
.child(
|
||||
div()
|
||||
.ml_1()
|
||||
.text_ellipsis()
|
||||
.overflow_x_hidden()
|
||||
.when_some(self.error_message.as_ref(), |el, error| {
|
||||
el.child(
|
||||
Label::new(format!("{}", error)).size(LabelSize::Small),
|
||||
)
|
||||
})
|
||||
.when(
|
||||
self.error_message.is_none()
|
||||
&& self.status_message.is_some(),
|
||||
|el| {
|
||||
el.child(
|
||||
Label::new(format!(
|
||||
"{}…",
|
||||
self.status_message.clone().unwrap()
|
||||
))
|
||||
.size(LabelSize::Small),
|
||||
)
|
||||
},
|
||||
),
|
||||
),
|
||||
)
|
||||
},
|
||||
)
|
||||
.when_some(self.status_message.clone(), |el, status_message| {
|
||||
el.child(
|
||||
h_flex()
|
||||
.p_2()
|
||||
.flex()
|
||||
.child(
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Medium)
|
||||
.with_animation(
|
||||
"arrow-circle",
|
||||
Animation::new(Duration::from_secs(2)).repeat(),
|
||||
|icon, delta| {
|
||||
icon.transform(Transformation::rotate(percentage(delta)))
|
||||
},
|
||||
),
|
||||
)
|
||||
.child(div().ml_1().text_ellipsis().overflow_x_hidden().child(
|
||||
Label::new(format!("{}…", status_message)).size(LabelSize::Small),
|
||||
)),
|
||||
)
|
||||
})
|
||||
.when_some(self.prompt.as_ref(), |el, prompt| {
|
||||
el.child(
|
||||
div()
|
||||
|
|
@ -267,14 +228,10 @@ impl Render for SshPrompt {
|
|||
}
|
||||
|
||||
impl SshConnectionModal {
|
||||
pub fn new(
|
||||
connection_options: &SshConnectionOptions,
|
||||
is_separate_window: bool,
|
||||
cx: &mut ViewContext<Self>,
|
||||
) -> Self {
|
||||
pub fn new(connection_options: &SshConnectionOptions, cx: &mut ViewContext<Self>) -> Self {
|
||||
Self {
|
||||
prompt: cx.new_view(|cx| SshPrompt::new(connection_options, cx)),
|
||||
is_separate_window,
|
||||
finished: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -282,11 +239,13 @@ impl SshConnectionModal {
|
|||
self.prompt.update(cx, |prompt, cx| prompt.confirm(cx))
|
||||
}
|
||||
|
||||
pub fn finished(&mut self, cx: &mut ViewContext<Self>) {
|
||||
self.finished = true;
|
||||
cx.emit(DismissEvent);
|
||||
}
|
||||
|
||||
fn dismiss(&mut self, _: &menu::Cancel, cx: &mut ViewContext<Self>) {
|
||||
cx.emit(DismissEvent);
|
||||
if self.is_separate_window {
|
||||
cx.remove_window();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -378,8 +337,9 @@ impl EventEmitter<DismissEvent> for SshConnectionModal {}
|
|||
|
||||
impl ModalView for SshConnectionModal {
|
||||
fn on_before_dismiss(&mut self, _: &mut ViewContext<Self>) -> workspace::DismissDecision {
|
||||
return workspace::DismissDecision::Dismiss(false);
|
||||
return workspace::DismissDecision::Dismiss(self.finished);
|
||||
}
|
||||
|
||||
fn fade_out_background(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
|
@ -418,10 +378,6 @@ impl remote::SshClientDelegate for SshClientDelegate {
|
|||
self.update_status(status, cx)
|
||||
}
|
||||
|
||||
fn set_error(&self, error: String, cx: &mut AsyncAppContext) {
|
||||
self.update_error(error, cx)
|
||||
}
|
||||
|
||||
fn get_server_binary(
|
||||
&self,
|
||||
platform: SshPlatform,
|
||||
|
|
@ -463,16 +419,6 @@ impl SshClientDelegate {
|
|||
.ok();
|
||||
}
|
||||
|
||||
fn update_error(&self, error: String, cx: &mut AsyncAppContext) {
|
||||
self.window
|
||||
.update(cx, |_, cx| {
|
||||
self.ui.update(cx, |modal, cx| {
|
||||
modal.set_error(error, cx);
|
||||
})
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
|
||||
async fn get_server_binary_impl(
|
||||
&self,
|
||||
platform: SshPlatform,
|
||||
|
|
@ -663,45 +609,69 @@ pub async fn open_ssh_project(
|
|||
})?
|
||||
};
|
||||
|
||||
let delegate = window.update(cx, |workspace, cx| {
|
||||
cx.activate_window();
|
||||
workspace.toggle_modal(cx, |cx| {
|
||||
SshConnectionModal::new(&connection_options, true, cx)
|
||||
});
|
||||
let ui = workspace
|
||||
.active_modal::<SshConnectionModal>(cx)
|
||||
.unwrap()
|
||||
.read(cx)
|
||||
.prompt
|
||||
.clone();
|
||||
loop {
|
||||
let delegate = window.update(cx, {
|
||||
let connection_options = connection_options.clone();
|
||||
move |workspace, cx| {
|
||||
cx.activate_window();
|
||||
workspace.toggle_modal(cx, |cx| SshConnectionModal::new(&connection_options, cx));
|
||||
let ui = workspace
|
||||
.active_modal::<SshConnectionModal>(cx)
|
||||
.unwrap()
|
||||
.read(cx)
|
||||
.prompt
|
||||
.clone();
|
||||
|
||||
Arc::new(SshClientDelegate {
|
||||
window: cx.window_handle(),
|
||||
ui,
|
||||
known_password: connection_options.password.clone(),
|
||||
})
|
||||
})?;
|
||||
Arc::new(SshClientDelegate {
|
||||
window: cx.window_handle(),
|
||||
ui,
|
||||
known_password: connection_options.password.clone(),
|
||||
})
|
||||
}
|
||||
})?;
|
||||
|
||||
let did_open_ssh_project = cx
|
||||
.update(|cx| {
|
||||
workspace::open_ssh_project(
|
||||
window,
|
||||
connection_options,
|
||||
delegate.clone(),
|
||||
app_state,
|
||||
paths,
|
||||
cx,
|
||||
)
|
||||
})?
|
||||
.await;
|
||||
let did_open_ssh_project = cx
|
||||
.update(|cx| {
|
||||
workspace::open_ssh_project(
|
||||
window,
|
||||
connection_options.clone(),
|
||||
delegate.clone(),
|
||||
app_state.clone(),
|
||||
paths.clone(),
|
||||
cx,
|
||||
)
|
||||
})?
|
||||
.await;
|
||||
|
||||
let did_open_ssh_project = match did_open_ssh_project {
|
||||
Ok(ok) => Ok(ok),
|
||||
Err(e) => {
|
||||
delegate.update_error(e.to_string(), cx);
|
||||
Err(e)
|
||||
window
|
||||
.update(cx, |workspace, cx| {
|
||||
if let Some(ui) = workspace.active_modal::<SshConnectionModal>(cx) {
|
||||
ui.update(cx, |modal, cx| modal.finished(cx))
|
||||
}
|
||||
})
|
||||
.ok();
|
||||
|
||||
if let Err(e) = did_open_ssh_project {
|
||||
log::error!("Failed to open project: {:?}", e);
|
||||
let response = window
|
||||
.update(cx, |_, cx| {
|
||||
cx.prompt(
|
||||
PromptLevel::Critical,
|
||||
"Failed to connect over SSH",
|
||||
Some(&e.to_string()),
|
||||
&["Retry", "Ok"],
|
||||
)
|
||||
})?
|
||||
.await;
|
||||
|
||||
if response == Ok(1) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
did_open_ssh_project
|
||||
break;
|
||||
}
|
||||
|
||||
// Already showed the error to the user
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -233,7 +233,6 @@ pub trait SshClientDelegate: Send + Sync {
|
|||
cx: &mut AsyncAppContext,
|
||||
) -> oneshot::Receiver<Result<(PathBuf, SemanticVersion)>>;
|
||||
fn set_status(&self, status: Option<&str>, cx: &mut AsyncAppContext);
|
||||
fn set_error(&self, error_message: String, cx: &mut AsyncAppContext);
|
||||
}
|
||||
|
||||
impl SshSocket {
|
||||
|
|
@ -485,7 +484,6 @@ impl SshRemoteClient {
|
|||
|
||||
if let Err(error) = client.ping(HEARTBEAT_TIMEOUT).await {
|
||||
log::error!("failed to establish connection: {}", error);
|
||||
delegate.set_error(error.to_string(), &mut cx);
|
||||
return Err(error);
|
||||
}
|
||||
|
||||
|
|
@ -1301,9 +1299,7 @@ impl SshRemoteConnection {
|
|||
};
|
||||
|
||||
if let Err(e) = result {
|
||||
let error_message = format!("Failed to connect to host: {}.", e);
|
||||
delegate.set_error(error_message, cx);
|
||||
return Err(e);
|
||||
return Err(e.context("Failed to connect to host"));
|
||||
}
|
||||
|
||||
drop(askpass_task);
|
||||
|
|
@ -1317,7 +1313,6 @@ impl SshRemoteConnection {
|
|||
"failed to connect: {}",
|
||||
String::from_utf8_lossy(&output).trim()
|
||||
);
|
||||
delegate.set_error(error_message.clone(), cx);
|
||||
Err(anyhow!(error_message))?;
|
||||
}
|
||||
|
||||
|
|
@ -1862,8 +1857,5 @@ mod fake {
|
|||
fn set_status(&self, _: Option<&str>, _: &mut AsyncAppContext) {
|
||||
unreachable!()
|
||||
}
|
||||
fn set_error(&self, _: String, _: &mut AsyncAppContext) {
|
||||
unreachable!()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue