From 5199135b54c98d571af8d9cc16635fd94a0b85ac Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Fri, 27 Sep 2024 09:31:45 +0200 Subject: [PATCH] ssh remoting: Show error if opening connection timed out (#18401) This shows an error if opening a connection to a remote host didn't work in the timeout of 10s (maybe we'll need to make that configurable in the future? for now it seems fine.) ![screenshot-2024-09-26-18 01 07@2x](https://github.com/user-attachments/assets/cbfa0e9f-9c29-4b6c-bade-07fdd7393c9d) Release Notes: - N/A --------- Co-authored-by: Bennet Co-authored-by: Conrad --- crates/recent_projects/src/ssh_connections.rs | 55 +++++++--- crates/remote/src/ssh_session.rs | 101 +++++++++++++----- 2 files changed, 115 insertions(+), 41 deletions(-) diff --git a/crates/recent_projects/src/ssh_connections.rs b/crates/recent_projects/src/ssh_connections.rs index 1722c58f07..dd30f15f26 100644 --- a/crates/recent_projects/src/ssh_connections.rs +++ b/crates/recent_projects/src/ssh_connections.rs @@ -16,8 +16,9 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::{Settings, SettingsSources}; use ui::{ - h_flex, v_flex, FluentBuilder as _, Icon, IconName, IconSize, InteractiveElement, IntoElement, - Label, LabelCommon, Styled, StyledExt as _, ViewContext, VisualContext, WindowContext, + h_flex, v_flex, Color, FluentBuilder as _, Icon, IconName, IconSize, InteractiveElement, + IntoElement, Label, LabelCommon, Styled, StyledExt as _, ViewContext, VisualContext, + WindowContext, }; use workspace::{AppState, ModalView, Workspace}; @@ -79,6 +80,7 @@ impl Settings for SshSettings { pub struct SshPrompt { connection_string: SharedString, status_message: Option, + error_message: Option, prompt: Option<(SharedString, oneshot::Sender>)>, editor: View, } @@ -92,6 +94,7 @@ impl SshPrompt { Self { connection_string, status_message: None, + error_message: None, prompt: None, editor: cx.new_view(Editor::single_line), } @@ -121,6 +124,11 @@ impl SshPrompt { cx.notify(); } + pub fn set_error(&mut self, error_message: String, cx: &mut ViewContext) { + self.error_message = Some(error_message.into()); + cx.notify(); + } + pub fn confirm(&mut self, cx: &mut ViewContext) { if let Some((_, tx)) = self.prompt.take() { self.editor.update(cx, |editor, cx| { @@ -140,7 +148,12 @@ impl Render for SshPrompt { .child( h_flex() .gap_2() - .child( + .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( @@ -149,16 +162,21 @@ impl Render for SshPrompt { |icon, delta| { icon.transform(Transformation::rotate(percentage(delta))) }, - ), - ) + ) + .into_any_element() + }) .child( Label::new(format!("ssh {}…", self.connection_string)) .size(ui::LabelSize::Large), ), ) - .when_some(self.status_message.as_ref(), |el, status| { - el.child(Label::new(status.clone())) + .when_some(self.error_message.as_ref(), |el, error| { + el.child(Label::new(error.clone())) }) + .when( + self.error_message.is_none() && self.status_message.is_some(), + |el| el.child(Label::new(self.status_message.clone().unwrap())), + ) .when_some(self.prompt.as_ref(), |el, prompt| { el.child(Label::new(prompt.0.clone())) .child(self.editor.clone()) @@ -238,6 +256,10 @@ 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, @@ -270,6 +292,16 @@ 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, @@ -388,7 +420,7 @@ pub async fn open_ssh_project( })? }; - let result = window + let session = window .update(cx, |workspace, cx| { cx.activate_window(); workspace.toggle_modal(cx, |cx| SshConnectionModal::new(&connection_options, cx)); @@ -400,12 +432,7 @@ pub async fn open_ssh_project( .clone(); connect_over_ssh(connection_options.clone(), ui, cx) })? - .await; - - if result.is_err() { - window.update(cx, |_, cx| cx.remove_window()).ok(); - } - let session = result?; + .await?; cx.update(|cx| { workspace::open_ssh_project(window, connection_options, session, app_state, paths, cx) diff --git a/crates/remote/src/ssh_session.rs b/crates/remote/src/ssh_session.rs index 06a7f810e6..915595fd9d 100644 --- a/crates/remote/src/ssh_session.rs +++ b/crates/remote/src/ssh_session.rs @@ -129,6 +129,7 @@ pub trait SshClientDelegate { cx: &mut AsyncAppContext, ) -> oneshot::Receiver>; fn set_status(&self, status: Option<&str>, cx: &mut AsyncAppContext); + fn set_error(&self, error_message: String, cx: &mut AsyncAppContext); } type ResponseChannels = Mutex)>>>; @@ -208,16 +209,16 @@ impl SshSession { result = child_stdout.read(&mut stdout_buffer).fuse() => { match result { - Ok(len) => { - if len == 0 { - child_stdin.close().await?; - let status = remote_server_child.status().await?; - if !status.success() { - log::info!("channel exited with status: {status:?}"); - } - return Ok(()); + Ok(0) => { + child_stdin.close().await?; + outgoing_rx.close(); + let status = remote_server_child.status().await?; + if !status.success() { + log::error!("channel exited with status: {status:?}"); } - + return Ok(()); + } + Ok(len) => { if len < stdout_buffer.len() { child_stdout.read_exact(&mut stdout_buffer[len..]).await?; } @@ -419,8 +420,13 @@ impl SshSession { let mut response_channels_lock = self.response_channels.lock(); response_channels_lock.insert(MessageId(envelope.id), tx); drop(response_channels_lock); - self.outgoing_tx.unbounded_send(envelope).ok(); + let result = self.outgoing_tx.unbounded_send(envelope); async move { + if let Err(error) = &result { + log::error!("failed to send message: {}", error); + return Err(anyhow!("failed to send message: {}", error)); + } + let response = rx.await.context("connection lost")?.0; if let Some(proto::envelope::Payload::Error(error)) = &response.payload { return Err(RpcError::from_proto(error, type_name)); @@ -525,22 +531,25 @@ impl SshClientState { let listener = UnixListener::bind(&askpass_socket).context("failed to create askpass socket")?; - let askpass_task = cx.spawn(|mut cx| async move { - while let Ok((mut stream, _)) = listener.accept().await { - let mut buffer = Vec::new(); - let mut reader = BufReader::new(&mut stream); - if reader.read_until(b'\0', &mut buffer).await.is_err() { - buffer.clear(); - } - let password_prompt = String::from_utf8_lossy(&buffer); - if let Some(password) = delegate - .ask_password(password_prompt.to_string(), &mut cx) - .await - .context("failed to get ssh password") - .and_then(|p| p) - .log_err() - { - stream.write_all(password.as_bytes()).await.log_err(); + let askpass_task = cx.spawn({ + let delegate = delegate.clone(); + |mut cx| async move { + while let Ok((mut stream, _)) = listener.accept().await { + let mut buffer = Vec::new(); + let mut reader = BufReader::new(&mut stream); + if reader.read_until(b'\0', &mut buffer).await.is_err() { + buffer.clear(); + } + let password_prompt = String::from_utf8_lossy(&buffer); + if let Some(password) = delegate + .ask_password(password_prompt.to_string(), &mut cx) + .await + .context("failed to get ssh password") + .and_then(|p| p) + .log_err() + { + stream.write_all(password.as_bytes()).await.log_err(); + } } } }); @@ -575,7 +584,22 @@ impl SshClientState { // has completed. let stdout = master_process.stdout.as_mut().unwrap(); let mut output = Vec::new(); - stdout.read_to_end(&mut output).await?; + let connection_timeout = std::time::Duration::from_secs(10); + let result = read_with_timeout(stdout, connection_timeout, &mut output).await; + if let Err(e) = result { + let error_message = if e.kind() == std::io::ErrorKind::TimedOut { + format!( + "Failed to connect to host. Timed out after {:?}.", + connection_timeout + ) + } else { + format!("Failed to connect to host: {}.", e) + }; + + delegate.set_error(error_message, cx); + return Err(e.into()); + } + drop(askpass_task); if master_process.try_status()?.is_some() { @@ -716,6 +740,29 @@ impl SshClientState { } } +#[cfg(unix)] +async fn read_with_timeout( + stdout: &mut process::ChildStdout, + timeout: std::time::Duration, + output: &mut Vec, +) -> Result<(), std::io::Error> { + smol::future::or( + async { + stdout.read_to_end(output).await?; + Ok::<_, std::io::Error>(()) + }, + async { + smol::Timer::after(timeout).await; + + Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "Read operation timed out", + )) + }, + ) + .await +} + impl Drop for SshClientState { fn drop(&mut self) { if let Err(error) = self.master_process.kill() {