From 471e02d48f597e9cdcc50ad45a488325eb7bd628 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 10 May 2025 15:12:58 +0300 Subject: [PATCH] Separate timeout and connection dropped errors out (#30457) --- Cargo.lock | 1 + .../src/context_editor.rs | 21 ++++- crates/client/src/client.rs | 77 ++++++++++----- crates/client/src/test.rs | 1 + crates/collab/src/tests/editor_tests.rs | 2 + crates/collab/src/tests/integration_tests.rs | 6 ++ crates/collab/src/tests/test_server.rs | 1 + crates/collab_ui/Cargo.toml | 1 + crates/collab_ui/src/collab_panel.rs | 1 + crates/collab_ui/src/notification_panel.rs | 16 +++- crates/copilot/src/copilot.rs | 36 +++++-- crates/editor/src/editor_tests.rs | 2 + crates/editor/src/inlay_hint_cache.rs | 5 + crates/language_models/src/provider/cloud.rs | 9 +- crates/lsp/src/lsp.rs | 68 +++++++++++--- crates/prettier/src/prettier.rs | 8 +- crates/project/src/lsp_store.rs | 94 +++++++++++-------- crates/project/src/project.rs | 5 +- crates/project/src/project_tests.rs | 2 + crates/title_bar/src/title_bar.rs | 1 + crates/util/src/util.rs | 23 +++++ crates/workspace/src/notifications.rs | 2 +- crates/worktree/src/worktree.rs | 6 +- crates/zed/src/main.rs | 35 +++++-- crates/zeta/src/onboarding_modal.rs | 5 +- 25 files changed, 313 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10e949c359..008342a11c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3068,6 +3068,7 @@ dependencies = [ "gpui", "http_client", "language", + "log", "menu", "notifications", "picker", diff --git a/crates/assistant_context_editor/src/context_editor.rs b/crates/assistant_context_editor/src/context_editor.rs index d725943655..37cb766986 100644 --- a/crates/assistant_context_editor/src/context_editor.rs +++ b/crates/assistant_context_editor/src/context_editor.rs @@ -1894,11 +1894,24 @@ impl ContextEditor { .log_err(); if let Some(client) = client { - cx.spawn(async move |this, cx| { - client.authenticate_and_connect(true, cx).await?; - this.update(cx, |_, cx| cx.notify()) + cx.spawn(async move |context_editor, cx| { + match client.authenticate_and_connect(true, cx).await { + util::ConnectionResult::Timeout => { + log::error!("Authentication timeout") + } + util::ConnectionResult::ConnectionReset => { + log::error!("Connection reset") + } + util::ConnectionResult::Result(r) => { + if r.log_err().is_some() { + context_editor + .update(cx, |_, cx| cx.notify()) + .ok(); + } + } + } }) - .detach_and_log_err(cx) + .detach() } })), ) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index f1f8355f89..55e81bbccf 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -49,7 +49,7 @@ use telemetry::Telemetry; use thiserror::Error; use tokio::net::TcpStream; use url::Url; -use util::{ResultExt, TryFutureExt}; +use util::{ConnectionResult, ResultExt}; pub use rpc::*; pub use telemetry_events::Event; @@ -151,9 +151,19 @@ pub fn init(client: &Arc, cx: &mut App) { let client = client.clone(); move |_: &SignIn, cx| { if let Some(client) = client.upgrade() { - cx.spawn(async move |cx| { - client.authenticate_and_connect(true, &cx).log_err().await - }) + cx.spawn( + async move |cx| match client.authenticate_and_connect(true, &cx).await { + ConnectionResult::Timeout => { + log::error!("Initial authentication timed out"); + } + ConnectionResult::ConnectionReset => { + log::error!("Initial authentication connection reset"); + } + ConnectionResult::Result(r) => { + r.log_err(); + } + }, + ) .detach(); } } @@ -658,7 +668,7 @@ impl Client { state._reconnect_task = None; } Status::ConnectionLost => { - let this = self.clone(); + let client = self.clone(); state._reconnect_task = Some(cx.spawn(async move |cx| { #[cfg(any(test, feature = "test-support"))] let mut rng = StdRng::seed_from_u64(0); @@ -666,10 +676,25 @@ impl Client { let mut rng = StdRng::from_entropy(); let mut delay = INITIAL_RECONNECTION_DELAY; - while let Err(error) = this.authenticate_and_connect(true, &cx).await { - log::error!("failed to connect {}", error); - if matches!(*this.status().borrow(), Status::ConnectionError) { - this.set_status( + loop { + match client.authenticate_and_connect(true, &cx).await { + ConnectionResult::Timeout => { + log::error!("client connect attempt timed out") + } + ConnectionResult::ConnectionReset => { + log::error!("client connect attempt reset") + } + ConnectionResult::Result(r) => { + if let Err(error) = r { + log::error!("failed to connect: {error}"); + } else { + break; + } + } + } + + if matches!(*client.status().borrow(), Status::ConnectionError) { + client.set_status( Status::ReconnectionError { next_reconnection: Instant::now() + delay, }, @@ -827,7 +852,7 @@ impl Client { self: &Arc, try_provider: bool, cx: &AsyncApp, - ) -> anyhow::Result<()> { + ) -> ConnectionResult<()> { let was_disconnected = match *self.status().borrow() { Status::SignedOut => true, Status::ConnectionError @@ -836,9 +861,14 @@ impl Client { | Status::Reauthenticating { .. } | Status::ReconnectionError { .. } => false, Status::Connected { .. } | Status::Connecting { .. } | Status::Reconnecting { .. } => { - return Ok(()); + return ConnectionResult::Result(Ok(())); + } + Status::UpgradeRequired => { + return ConnectionResult::Result( + Err(EstablishConnectionError::UpgradeRequired) + .context("client auth and connect"), + ); } - Status::UpgradeRequired => return Err(EstablishConnectionError::UpgradeRequired)?, }; if was_disconnected { self.set_status(Status::Authenticating, cx); @@ -862,12 +892,12 @@ impl Client { Ok(creds) => credentials = Some(creds), Err(err) => { self.set_status(Status::ConnectionError, cx); - return Err(err); + return ConnectionResult::Result(Err(err)); } } } _ = status_rx.next().fuse() => { - return Err(anyhow!("authentication canceled")); + return ConnectionResult::Result(Err(anyhow!("authentication canceled"))); } } } @@ -892,10 +922,10 @@ impl Client { } futures::select_biased! { - result = self.set_connection(conn, cx).fuse() => result, + result = self.set_connection(conn, cx).fuse() => ConnectionResult::Result(result.context("client auth and connect")), _ = timeout => { self.set_status(Status::ConnectionError, cx); - Err(anyhow!("timed out waiting on hello message from server")) + ConnectionResult::Timeout } } } @@ -907,22 +937,22 @@ impl Client { self.authenticate_and_connect(false, cx).await } else { self.set_status(Status::ConnectionError, cx); - Err(EstablishConnectionError::Unauthorized)? + ConnectionResult::Result(Err(EstablishConnectionError::Unauthorized).context("client auth and connect")) } } Err(EstablishConnectionError::UpgradeRequired) => { self.set_status(Status::UpgradeRequired, cx); - Err(EstablishConnectionError::UpgradeRequired)? + ConnectionResult::Result(Err(EstablishConnectionError::UpgradeRequired).context("client auth and connect")) } Err(error) => { self.set_status(Status::ConnectionError, cx); - Err(error)? + ConnectionResult::Result(Err(error).context("client auth and connect")) } } } _ = &mut timeout => { self.set_status(Status::ConnectionError, cx); - Err(anyhow!("timed out trying to establish connection")) + ConnectionResult::Timeout } } } @@ -938,10 +968,7 @@ impl Client { let peer_id = async { log::debug!("waiting for server hello"); - let message = incoming - .next() - .await - .ok_or_else(|| anyhow!("no hello message received"))?; + let message = incoming.next().await.context("no hello message received")?; log::debug!("got server hello"); let hello_message_type_name = message.payload_type_name().to_string(); let hello = message @@ -1743,7 +1770,7 @@ mod tests { status.next().await, Some(Status::ConnectionError { .. }) )); - auth_and_connect.await.unwrap_err(); + auth_and_connect.await.into_response().unwrap_err(); // Allow the connection to be established. let server = FakeServer::for_client(user_id, &client, cx).await; diff --git a/crates/client/src/test.rs b/crates/client/src/test.rs index 825c0f43b4..428a6d3ac7 100644 --- a/crates/client/src/test.rs +++ b/crates/client/src/test.rs @@ -107,6 +107,7 @@ impl FakeServer { client .authenticate_and_connect(false, &cx.to_async()) .await + .into_response() .unwrap(); server diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 316937733e..ef40720fc9 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -1740,6 +1740,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( fake_language_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); executor.run_until_parked(); @@ -1930,6 +1931,7 @@ async fn test_inlay_hint_refresh_is_forwarded( fake_language_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); executor.run_until_parked(); editor_a.update(cx_a, |editor, _| { diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index e87c487080..ae0dbc37a5 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -1253,6 +1253,7 @@ async fn test_calls_on_multiple_connections( client_b1 .authenticate_and_connect(false, &cx_b1.to_async()) .await + .into_response() .unwrap(); // User B hangs up, and user A calls them again. @@ -1633,6 +1634,7 @@ async fn test_project_reconnect( client_a .authenticate_and_connect(false, &cx_a.to_async()) .await + .into_response() .unwrap(); executor.run_until_parked(); @@ -1761,6 +1763,7 @@ async fn test_project_reconnect( client_b .authenticate_and_connect(false, &cx_b.to_async()) .await + .into_response() .unwrap(); executor.run_until_parked(); @@ -4317,6 +4320,7 @@ async fn test_collaborating_with_lsp_progress_updates_and_diagnostics_ordering( token: lsp::NumberOrString::String("the-disk-based-token".to_string()), }) .await + .into_response() .unwrap(); fake_language_server.notify::(&lsp::ProgressParams { token: lsp::NumberOrString::String("the-disk-based-token".to_string()), @@ -5699,6 +5703,7 @@ async fn test_contacts( client_c .authenticate_and_connect(false, &cx_c.to_async()) .await + .into_response() .unwrap(); executor.run_until_parked(); @@ -6229,6 +6234,7 @@ async fn test_contact_requests( client .authenticate_and_connect(false, &cx.to_async()) .await + .into_response() .unwrap(); } } diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index da02e6c32a..683bd97e0f 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -313,6 +313,7 @@ impl TestServer { client .authenticate_and_connect(false, &cx.to_async()) .await + .into_response() .unwrap(); let client = TestClient { diff --git a/crates/collab_ui/Cargo.toml b/crates/collab_ui/Cargo.toml index 3b966edb18..46ba3ae496 100644 --- a/crates/collab_ui/Cargo.toml +++ b/crates/collab_ui/Cargo.toml @@ -42,6 +42,7 @@ futures.workspace = true fuzzy.workspace = true gpui.workspace = true language.workspace = true +log.workspace = true menu.workspace = true notifications.workspace = true picker.workspace = true diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 6faac7aff8..6df8baaacc 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -2227,6 +2227,7 @@ impl CollabPanel { client .authenticate_and_connect(true, &cx) .await + .into_response() .notify_async_err(cx); }) .detach() diff --git a/crates/collab_ui/src/notification_panel.rs b/crates/collab_ui/src/notification_panel.rs index 166a383c6b..5e5e8164f9 100644 --- a/crates/collab_ui/src/notification_panel.rs +++ b/crates/collab_ui/src/notification_panel.rs @@ -646,10 +646,20 @@ impl Render for NotificationPanel { let client = client.clone(); window .spawn(cx, async move |cx| { - client + match client .authenticate_and_connect(true, &cx) - .log_err() - .await; + .await + { + util::ConnectionResult::Timeout => { + log::error!("Connection timeout"); + } + util::ConnectionResult::ConnectionReset => { + log::error!("Connection reset"); + } + util::ConnectionResult::Result(r) => { + r.log_err(); + } + } }) .detach() } diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index ab2895990a..78f34d2c03 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -5,7 +5,7 @@ mod sign_in; use crate::sign_in::initiate_sign_in_within_workspace; use ::fs::Fs; -use anyhow::{Result, anyhow}; +use anyhow::{Context as _, Result, anyhow}; use collections::{HashMap, HashSet}; use command_palette_hooks::CommandPaletteFilter; use futures::{Future, FutureExt, TryFutureExt, channel::oneshot, future::Shared}; @@ -531,11 +531,15 @@ impl Copilot { .request::(request::CheckStatusParams { local_checks_only: false, }) - .await?; + .await + .into_response() + .context("copilot: check status")?; server .request::(editor_info) - .await?; + .await + .into_response() + .context("copilot: set editor info")?; anyhow::Ok((server, status)) }; @@ -581,7 +585,9 @@ impl Copilot { .request::( request::SignInInitiateParams {}, ) - .await?; + .await + .into_response() + .context("copilot sign-in")?; match sign_in { request::SignInInitiateResult::AlreadySignedIn { user } => { Ok(request::SignInStatus::Ok { user: Some(user) }) @@ -609,7 +615,9 @@ impl Copilot { user_code: flow.user_code, }, ) - .await?; + .await + .into_response() + .context("copilot: sign in confirm")?; Ok(response) } } @@ -656,7 +664,9 @@ impl Copilot { cx.background_spawn(async move { server .request::(request::SignOutParams {}) - .await?; + .await + .into_response() + .context("copilot: sign in confirm")?; anyhow::Ok(()) }) } @@ -873,7 +883,10 @@ impl Copilot { uuid: completion.uuid.clone(), }); cx.background_spawn(async move { - request.await?; + request + .await + .into_response() + .context("copilot: notify accepted")?; Ok(()) }) } @@ -897,7 +910,10 @@ impl Copilot { .collect(), }); cx.background_spawn(async move { - request.await?; + request + .await + .into_response() + .context("copilot: notify rejected")?; Ok(()) }) } @@ -957,7 +973,9 @@ impl Copilot { version: version.try_into().unwrap(), }, }) - .await?; + .await + .into_response() + .context("copilot: get completions")?; let completions = result .completions .into_iter() diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index e4a6705d73..f4ef0b899c 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8900,6 +8900,7 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) { }, }) .await + .into_response() .unwrap(); Ok(Some(json!(null))) } @@ -19153,6 +19154,7 @@ async fn test_apply_code_lens_actions_with_commands(cx: &mut gpui::TestAppContex }, ) .await + .into_response() .unwrap(); Ok(Some(json!(null))) } diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index b24bd7944a..2ffe97c245 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -1409,6 +1409,7 @@ pub mod tests { fake_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); cx.executor().run_until_parked(); editor @@ -1492,6 +1493,7 @@ pub mod tests { token: lsp::ProgressToken::String(progress_token.to_string()), }) .await + .into_response() .expect("work done progress create request failed"); cx.executor().run_until_parked(); fake_server.notify::(&lsp::ProgressParams { @@ -1863,6 +1865,7 @@ pub mod tests { fake_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); cx.executor().run_until_parked(); editor @@ -2008,6 +2011,7 @@ pub mod tests { fake_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); cx.executor().run_until_parked(); editor @@ -2070,6 +2074,7 @@ pub mod tests { fake_server .request::(()) .await + .into_response() .expect("inlay refresh request failed"); cx.executor().run_until_parked(); editor diff --git a/crates/language_models/src/provider/cloud.rs b/crates/language_models/src/provider/cloud.rs index d4a200f8c5..c35f5f10c1 100644 --- a/crates/language_models/src/provider/cloud.rs +++ b/crates/language_models/src/provider/cloud.rs @@ -180,9 +180,12 @@ impl State { fn authenticate(&self, cx: &mut Context) -> Task> { let client = self.client.clone(); - cx.spawn(async move |this, cx| { - client.authenticate_and_connect(true, &cx).await?; - this.update(cx, |_, cx| cx.notify()) + cx.spawn(async move |state, cx| { + client + .authenticate_and_connect(true, &cx) + .await + .into_response()?; + state.update(cx, |_, cx| cx.notify()) }) } diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 77d2e88dae..2c54066a51 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -5,7 +5,12 @@ pub use lsp_types::*; use anyhow::{Context as _, Result, anyhow}; use collections::HashMap; -use futures::{AsyncRead, AsyncWrite, Future, FutureExt, channel::oneshot, io::BufWriter, select}; +use futures::{ + AsyncRead, AsyncWrite, Future, FutureExt, + channel::oneshot::{self, Canceled}, + io::BufWriter, + select, +}; use gpui::{App, AppContext as _, AsyncApp, BackgroundExecutor, SharedString, Task}; use notification::DidChangeWorkspaceFolders; use parking_lot::{Mutex, RwLock}; @@ -39,7 +44,7 @@ use std::{ time::{Duration, Instant}, }; use std::{path::Path, process::Stdio}; -use util::{ResultExt, TryFutureExt}; +use util::{ConnectionResult, ResultExt, TryFutureExt}; const JSON_RPC_VERSION: &str = "2.0"; const CONTENT_LEN_HEADER: &str = "Content-Length: "; @@ -259,7 +264,7 @@ struct Error { message: String, } -pub trait LspRequestFuture: Future { +pub trait LspRequestFuture: Future> { fn id(&self) -> i32; } @@ -284,7 +289,10 @@ impl Future for LspRequest { } } -impl LspRequestFuture for LspRequest { +impl LspRequestFuture for LspRequest +where + F: Future>, +{ fn id(&self) -> i32 { self.id } @@ -824,7 +832,17 @@ impl LanguageServer { cx: &App, ) -> Task>> { cx.spawn(async move |_| { - let response = self.request::(params).await?; + let response = self + .request::(params) + .await + .into_response() + .with_context(|| { + format!( + "initializing server {}, id {}", + self.name(), + self.server_id() + ) + })?; if let Some(info) = response.server_info { self.process_name = info.name.into(); } @@ -863,7 +881,13 @@ impl LanguageServer { select! { request_result = shutdown_request.fuse() => { - request_result?; + match request_result { + ConnectionResult::Timeout => { + log::warn!("timeout waiting for language server {name} to shutdown"); + }, + ConnectionResult::ConnectionReset => {}, + ConnectionResult::Result(r) => r?, + } } _ = timer => { @@ -1084,7 +1108,7 @@ impl LanguageServer { pub fn request( &self, params: T::Params, - ) -> impl LspRequestFuture> + use + ) -> impl LspRequestFuture + use where T::Result: 'static + Send, { @@ -1097,15 +1121,18 @@ impl LanguageServer { ) } - fn request_internal( + fn request_internal( next_id: &AtomicI32, response_handlers: &Mutex>>, outbound_tx: &channel::Sender, executor: &BackgroundExecutor, params: T::Params, - ) -> impl LspRequestFuture> + use + ) -> impl LspRequestFuture + use where T::Result: 'static + Send, + T: request::Request, + // TODO kb + // ::Result: ConnectionResult, { let id = next_id.fetch_add(1, SeqCst); let message = serde_json::to_string(&Request { @@ -1120,7 +1147,7 @@ impl LanguageServer { let handle_response = response_handlers .lock() .as_mut() - .ok_or_else(|| anyhow!("server shut down")) + .context("server shut down") .map(|handlers| { let executor = executor.clone(); handlers.insert( @@ -1153,8 +1180,12 @@ impl LanguageServer { let mut timeout = executor.timer(LSP_REQUEST_TIMEOUT).fuse(); let started = Instant::now(); LspRequest::new(id, async move { - handle_response?; - send?; + if let Err(e) = handle_response { + return ConnectionResult::Result(Err(e)); + } + if let Err(e) = send { + return ConnectionResult::Result(Err(e)); + } let cancel_on_drop = util::defer(move || { if let Some(outbound_tx) = outbound_tx.upgrade() { @@ -1174,12 +1205,18 @@ impl LanguageServer { let elapsed = started.elapsed(); log::trace!("Took {elapsed:?} to receive response to {method:?} id {id}"); cancel_on_drop.abort(); - response? + match response { + Ok(response_result) => ConnectionResult::Result(response_result), + Err(Canceled) => { + log::error!("Server reset connection for a request {method:?} id {id}"); + ConnectionResult::ConnectionReset + }, + } } _ = timeout => { log::error!("Cancelled LSP request task for {method:?} id {id} which took over {LSP_REQUEST_TIMEOUT:?}"); - anyhow::bail!("LSP request timeout"); + ConnectionResult::Timeout } } }) @@ -1506,7 +1543,7 @@ impl FakeLanguageServer { } /// See [`LanguageServer::request`]. - pub async fn request(&self, params: T::Params) -> Result + pub async fn request(&self, params: T::Params) -> ConnectionResult where T: request::Request, T::Result: 'static + Send, @@ -1608,6 +1645,7 @@ impl FakeLanguageServer { token: NumberOrString::String(token.clone()), }) .await + .into_response() .unwrap(); self.notify::(&ProgressParams { token: NumberOrString::String(token), diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index 0e6f5c0d3e..ecc74c232a 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -452,7 +452,12 @@ impl Prettier { })? .context("prettier params calculation")?; - let response = local.server.request::(params).await?; + let response = local + .server + .request::(params) + .await + .into_response() + .context("prettier format")?; let diff_task = buffer.update(cx, |buffer, cx| buffer.diff(response.text, cx))?; Ok(diff_task.await) } @@ -482,6 +487,7 @@ impl Prettier { .server .request::(()) .await + .into_response() .context("prettier clear cache"), #[cfg(any(test, feature = "test-support"))] Self::Test(_) => Ok(()), diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index be76898601..960d9516f9 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -86,7 +86,7 @@ use std::{ use text::{Anchor, BufferId, LineEnding, OffsetRangeExt}; use url::Url; use util::{ - ResultExt, TryFutureExt as _, debug_panic, defer, maybe, merge_json_value_into, + ResultExt as _, debug_panic, defer, maybe, merge_json_value_into, paths::{PathExt, SanitizedPath}, post_inc, }; @@ -1769,7 +1769,8 @@ impl LocalLspStore { ..Default::default() }, ) - .await; + .await + .into_response(); if execute_command_result.is_err() { zlog::error!( @@ -1894,7 +1895,8 @@ impl LocalLspStore { options: lsp_command::lsp_formatting_options(settings), work_done_progress_params: Default::default(), }) - .await? + .await + .into_response()? { edits.get_or_insert_with(Vec::new).append(&mut edit); } @@ -1945,7 +1947,8 @@ impl LocalLspStore { options: lsp_command::lsp_formatting_options(settings), work_done_progress_params: Default::default(), }) - .await? + .await + .into_response()? } else if matches!(range_formatting_provider, Some(p) if *p != OneOf::Left(false)) { let _timer = zlog::time!(logger => "format-range"); let buffer_start = lsp::Position::new(0, 0); @@ -1957,7 +1960,8 @@ impl LocalLspStore { options: lsp_command::lsp_formatting_options(settings), work_done_progress_params: Default::default(), }) - .await? + .await + .into_response()? } else { None }; @@ -2065,7 +2069,8 @@ impl LocalLspStore { *lsp_action = Box::new( lang_server .request::(*lsp_action.clone()) - .await?, + .await + .into_response()?, ); } } @@ -2073,7 +2078,8 @@ impl LocalLspStore { if !action.resolved && GetCodeLens::can_resolve_lens(&lang_server.capabilities()) { *lens = lang_server .request::(lens.clone()) - .await?; + .await + .into_response()?; } } LspAction::Command(_) => {} @@ -2578,7 +2584,9 @@ impl LocalLspStore { arguments: command.arguments.clone().unwrap_or_default(), ..Default::default() }) - .await?; + .await + .into_response() + .context("execute command")?; lsp_store.update(cx, |this, _| { if let LspStoreMode::Local(mode) = &mut this.mode { @@ -4223,7 +4231,7 @@ impl LspStore { language_server.name(), err ); - log::warn!("{}", message); + log::warn!("{message}"); return Task::ready(Err(anyhow!(message))); } }; @@ -4268,7 +4276,7 @@ impl LspStore { None }; - let result = lsp_request.await; + let result = lsp_request.await.into_response(); let response = result.map_err(|err| { let message = format!( @@ -4277,7 +4285,7 @@ impl LspStore { language_server.name(), err ); - log::warn!("{}", message); + log::warn!("{message}"); anyhow!(message) })?; @@ -4521,15 +4529,14 @@ impl LspStore { .remove(&lang_server.server_id()); })?; - let result = lang_server + let _result = lang_server .request::(lsp::ExecuteCommandParams { command: command.command.clone(), arguments: command.arguments.clone().unwrap_or_default(), - ..Default::default() + ..lsp::ExecuteCommandParams::default() }) - .await; - - result?; + .await.into_response() + .context("execute command")?; return this.update(cx, |this, _| { this.as_local_mut() @@ -4649,6 +4656,7 @@ impl LspStore { ); let resolved_hint = resolve_task .await + .into_response() .context("inlay hint resolve LSP request")?; let resolved_hint = InlayHints::lsp_to_project_hint( resolved_hint, @@ -5232,7 +5240,10 @@ impl LspStore { } } }; - let resolved_completion = request.await?; + let resolved_completion = request + .await + .into_response() + .context("resolve completion")?; let mut updated_insert_range = None; if let Some(text_edit) = resolved_completion.text_edit.as_ref() { @@ -5847,27 +5858,30 @@ impl LspStore { ..Default::default() }, ) - .log_err() .map(move |response| { - let lsp_symbols = response.flatten().map(|symbol_response| match symbol_response { - lsp::WorkspaceSymbolResponse::Flat(flat_responses) => { - flat_responses.into_iter().map(|lsp_symbol| { - (lsp_symbol.name, lsp_symbol.kind, lsp_symbol.location) - }).collect::>() - } - lsp::WorkspaceSymbolResponse::Nested(nested_responses) => { - nested_responses.into_iter().filter_map(|lsp_symbol| { - let location = match lsp_symbol.location { - OneOf::Left(location) => location, - OneOf::Right(_) => { - log::error!("Unexpected: client capabilities forbid symbol resolutions in workspace.symbol.resolveSupport"); - return None - } - }; - Some((lsp_symbol.name, lsp_symbol.kind, location)) - }).collect::>() - } - }).unwrap_or_default(); + let lsp_symbols = response.into_response() + .context("workspace symbols request") + .log_err() + .flatten() + .map(|symbol_response| match symbol_response { + lsp::WorkspaceSymbolResponse::Flat(flat_responses) => { + flat_responses.into_iter().map(|lsp_symbol| { + (lsp_symbol.name, lsp_symbol.kind, lsp_symbol.location) + }).collect::>() + } + lsp::WorkspaceSymbolResponse::Nested(nested_responses) => { + nested_responses.into_iter().filter_map(|lsp_symbol| { + let location = match lsp_symbol.location { + OneOf::Left(location) => location, + OneOf::Right(_) => { + log::error!("Unexpected: client capabilities forbid symbol resolutions in workspace.symbol.resolveSupport"); + return None + } + }; + Some((lsp_symbol.name, lsp_symbol.kind, location)) + }).collect::>() + } + }).unwrap_or_default(); WorkspaceSymbolsResult { server_id, @@ -7517,8 +7531,10 @@ impl LspStore { .request::(RenameFilesParams { files: vec![FileRename { old_uri, new_uri }], }) - .log_err() .await + .into_response() + .context("will rename files") + .log_err() .flatten()?; LocalLspStore::deserialize_workspace_edit( @@ -7788,6 +7804,8 @@ impl LspStore { server .request::(lsp_completion) .await + .into_response() + .context("resolve completion item") } else { anyhow::Ok(lsp_completion) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 35477765f4..0d68ac7335 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1224,7 +1224,10 @@ impl Project { fs: Arc, cx: AsyncApp, ) -> Result> { - client.authenticate_and_connect(true, &cx).await?; + client + .authenticate_and_connect(true, &cx) + .await + .into_response()?; let subscriptions = [ EntitySubscription::Project(client.subscribe_to_entity::(remote_id)?), diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index e418848516..37d0868770 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1089,6 +1089,7 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon }], }) .await + .into_response() .unwrap(); fake_server.handle_notification::({ let file_changes = file_changes.clone(); @@ -3431,6 +3432,7 @@ async fn test_apply_code_actions_with_commands(cx: &mut gpui::TestAppContext) { }, ) .await + .into_response() .unwrap(); Ok(Some(json!(null))) } diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index 6947795b6e..47c903d977 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -664,6 +664,7 @@ impl TitleBar { client .authenticate_and_connect(true, &cx) .await + .into_response() .notify_async_err(cx); }) .detach(); diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 770c86b1db..ce5c7fae36 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -1025,6 +1025,29 @@ pub fn get_system_shell() -> String { } } +#[derive(Debug)] +pub enum ConnectionResult { + Timeout, + ConnectionReset, + Result(anyhow::Result), +} + +impl ConnectionResult { + pub fn into_response(self) -> anyhow::Result { + match self { + ConnectionResult::Timeout => anyhow::bail!("Request timed out"), + ConnectionResult::ConnectionReset => anyhow::bail!("Server reset the connection"), + ConnectionResult::Result(r) => r, + } + } +} + +impl From> for ConnectionResult { + fn from(result: anyhow::Result) -> Self { + ConnectionResult::Result(result) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index 3205a5dc86..f1d98b3077 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -693,7 +693,7 @@ pub mod simple_message_notification { ) } else { Tooltip::for_action( - "Close.", + "Close", &menu::Cancel, window, cx, diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 495525fba4..bf3b9e6a26 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -4358,11 +4358,7 @@ impl BackgroundScanner { let canonical_path = match self.fs.canonicalize(&child_abs_path).await { Ok(path) => path, Err(err) => { - log::error!( - "error reading target of symlink {:?}: {:?}", - child_abs_path, - err - ); + log::error!("error reading target of symlink {child_abs_path:?}: {err:#}",); continue; } }; diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 098c9715f2..52e7efcc92 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -44,7 +44,7 @@ use theme::{ ActiveTheme, IconThemeNotFoundError, SystemAppearance, ThemeNotFoundError, ThemeRegistry, ThemeSettings, }; -use util::{ResultExt, TryFutureExt, maybe}; +use util::{ConnectionResult, ResultExt, TryFutureExt, maybe}; use uuid::Uuid; use welcome::{BaseKeymap, FIRST_OPEN, show_welcome_view}; use workspace::{AppState, SerializedWorkspaceLocation, WorkspaceSettings, WorkspaceStore}; @@ -612,9 +612,17 @@ fn main() { cx.spawn({ let client = app_state.client.clone(); - async move |cx| authenticate(client, &cx).await + async move |cx| match authenticate(client, &cx).await { + ConnectionResult::Timeout => log::error!("Timeout during initial auth"), + ConnectionResult::ConnectionReset => { + log::error!("Connection reset during initial auth") + } + ConnectionResult::Result(r) => { + r.log_err(); + } + } }) - .detach_and_log_err(cx); + .detach(); let urls: Vec<_> = args .paths_or_urls @@ -727,7 +735,15 @@ fn handle_open_request(request: OpenRequest, app_state: Arc, cx: &mut let client = app_state.client.clone(); // we continue even if authentication fails as join_channel/ open channel notes will // show a visible error message. - authenticate(client, &cx).await.log_err(); + match authenticate(client, &cx).await { + ConnectionResult::Timeout => { + log::error!("Timeout during open request handling") + } + ConnectionResult::ConnectionReset => { + log::error!("Connection reset during open request handling") + } + ConnectionResult::Result(r) => r?, + }; if let Some(channel_id) = request.join_channel { cx.update(|cx| { @@ -777,17 +793,18 @@ fn handle_open_request(request: OpenRequest, app_state: Arc, cx: &mut } } -async fn authenticate(client: Arc, cx: &AsyncApp) -> Result<()> { +async fn authenticate(client: Arc, cx: &AsyncApp) -> ConnectionResult<()> { if stdout_is_a_pty() { if client::IMPERSONATE_LOGIN.is_some() { - client.authenticate_and_connect(false, cx).await?; + return client.authenticate_and_connect(false, cx).await; } else if client.has_credentials(cx).await { - client.authenticate_and_connect(true, cx).await?; + return client.authenticate_and_connect(true, cx).await; } } else if client.has_credentials(cx).await { - client.authenticate_and_connect(true, cx).await?; + return client.authenticate_and_connect(true, cx).await; } - Ok::<_, anyhow::Error>(()) + + ConnectionResult::Result(Ok(())) } async fn system_id() -> Result { diff --git a/crates/zeta/src/onboarding_modal.rs b/crates/zeta/src/onboarding_modal.rs index 80159cd075..bfd9e611b2 100644 --- a/crates/zeta/src/onboarding_modal.rs +++ b/crates/zeta/src/onboarding_modal.rs @@ -139,7 +139,10 @@ impl ZedPredictModal { self.sign_in_status = SignInStatus::Waiting; cx.spawn(async move |this, cx| { - let result = client.authenticate_and_connect(true, &cx).await; + let result = client + .authenticate_and_connect(true, &cx) + .await + .into_response(); let status = match result { Ok(_) => SignInStatus::SignedIn,