From ed6ed99d8f99e18dd371d7ed53a3362d2f222de5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 16 May 2022 19:56:10 +0200 Subject: [PATCH] Show the reason why a join request was declined Co-Authored-By: Nathan Sobo --- Cargo.lock | 1 + crates/collab/src/rpc.rs | 150 ++++++++++++++++++++++++++++-- crates/project/Cargo.toml | 1 + crates/project/src/project.rs | 30 +++++- crates/rpc/proto/zed.proto | 10 +- crates/workspace/src/workspace.rs | 28 ++++-- 6 files changed, 203 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60b35ef3c6..5a640659c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3394,6 +3394,7 @@ dependencies = [ "sum_tree", "tempdir", "text", + "thiserror", "toml", "unindent", "util", diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 14c3b71e13..7fdaaf5d9f 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -354,7 +354,9 @@ impl Server { receipt, proto::JoinProjectResponse { variant: Some(proto::join_project_response::Variant::Decline( - proto::join_project_response::Decline {}, + proto::join_project_response::Decline { + reason: proto::join_project_response::decline::Reason::WentOffline as i32 + }, )), }, )?; @@ -434,7 +436,10 @@ impl Server { receipt, proto::JoinProjectResponse { variant: Some(proto::join_project_response::Variant::Decline( - proto::join_project_response::Decline {}, + proto::join_project_response::Decline { + reason: proto::join_project_response::decline::Reason::Closed + as i32, + }, )), }, )?; @@ -542,7 +547,10 @@ impl Server { receipt, proto::JoinProjectResponse { variant: Some(proto::join_project_response::Variant::Decline( - proto::join_project_response::Decline {}, + proto::join_project_response::Decline { + reason: proto::join_project_response::decline::Reason::Declined + as i32, + }, )), }, )?; @@ -1837,17 +1845,26 @@ mod tests { } #[gpui::test(iterations = 10)] - async fn test_host_disconnect(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { + async fn test_host_disconnect( + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + cx_c: &mut TestAppContext, + ) { let lang_registry = Arc::new(LanguageRegistry::test()); let fs = FakeFs::new(cx_a.background()); cx_a.foreground().forbid_parking(); - // Connect to a server as 2 clients. + // Connect to a server as 3 clients. let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await; let client_a = server.create_client(cx_a, "user_a").await; let mut client_b = server.create_client(cx_b, "user_b").await; + let client_c = server.create_client(cx_c, "user_c").await; server - .make_contacts(vec![(&client_a, cx_a), (&client_b, cx_b)]) + .make_contacts(vec![ + (&client_a, cx_a), + (&client_b, cx_b), + (&client_c, cx_c), + ]) .await; // Share a project as client A @@ -1868,6 +1885,9 @@ mod tests { cx, ) }); + let project_id = project_a + .read_with(cx_a, |project, _| project.next_remote_id()) + .await; let (worktree_a, _) = project_a .update(cx_a, |p, cx| { p.find_or_create_local_worktree("/a", true, cx) @@ -1887,6 +1907,24 @@ mod tests { .await .unwrap(); + // Request to join that project as client C + let project_c = cx_c.spawn(|mut cx| { + let client = client_c.client.clone(); + let user_store = client_c.user_store.clone(); + let lang_registry = lang_registry.clone(); + async move { + Project::remote( + project_id, + client, + user_store, + lang_registry.clone(), + FakeFs::new(cx.background()), + &mut cx, + ) + .await + } + }); + // Drop client A's connection. Collaborators should disappear and the project should not be shown as shared. server.disconnect_client(client_a.current_user_id(cx_a)); cx_a.foreground().advance_clock(rpc::RECEIVE_TIMEOUT); @@ -1901,6 +1939,10 @@ mod tests { cx_b.update(|_| { drop(project_b); }); + assert!(matches!( + project_c.await.unwrap_err(), + project::JoinProjectError::HostWentOffline + )); // Ensure guests can still join. let project_b2 = client_b.build_remote_project(&project_a, cx_a, cx_b).await; @@ -1911,6 +1953,102 @@ mod tests { .unwrap(); } + #[gpui::test(iterations = 10)] + async fn test_decline_join_request( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + ) { + let lang_registry = Arc::new(LanguageRegistry::test()); + let fs = FakeFs::new(cx_a.background()); + cx_a.foreground().forbid_parking(); + + // Connect to a server as 2 clients. + let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .make_contacts(vec![(&client_a, cx_a), (&client_b, cx_b)]) + .await; + + // Share a project as client A + fs.insert_tree("/a", json!({})).await; + let project_a = cx_a.update(|cx| { + Project::local( + client_a.clone(), + client_a.user_store.clone(), + lang_registry.clone(), + fs.clone(), + cx, + ) + }); + let project_id = project_a + .read_with(cx_a, |project, _| project.next_remote_id()) + .await; + let (worktree_a, _) = project_a + .update(cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", true, cx) + }) + .await + .unwrap(); + worktree_a + .read_with(cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) + .await; + + // Request to join that project as client B + let project_b = cx_b.spawn(|mut cx| { + let client = client_b.client.clone(); + let user_store = client_b.user_store.clone(); + let lang_registry = lang_registry.clone(); + async move { + Project::remote( + project_id, + client, + user_store, + lang_registry.clone(), + FakeFs::new(cx.background()), + &mut cx, + ) + .await + } + }); + deterministic.run_until_parked(); + project_a.update(cx_a, |project, cx| { + project.respond_to_join_request(client_b.user_id().unwrap(), false, cx) + }); + assert!(matches!( + project_b.await.unwrap_err(), + project::JoinProjectError::HostDeclined + )); + + // Request to join the project again as client B + let project_b = cx_b.spawn(|mut cx| { + let client = client_b.client.clone(); + let user_store = client_b.user_store.clone(); + let lang_registry = lang_registry.clone(); + async move { + Project::remote( + project_id, + client, + user_store, + lang_registry.clone(), + FakeFs::new(cx.background()), + &mut cx, + ) + .await + } + }); + + // Close the project on the host + deterministic.run_until_parked(); + cx_a.update(|_| drop(project_a)); + deterministic.run_until_parked(); + assert!(matches!( + project_b.await.unwrap_err(), + project::JoinProjectError::HostClosedProject + )); + } + #[gpui::test(iterations = 10)] async fn test_propagate_saves_and_fs_changes( cx_a: &mut TestAppContext, diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index eaae45bcc6..7a3c7c5367 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -45,6 +45,7 @@ serde_json = { version = "1.0.64", features = ["preserve_order"] } sha2 = "0.10" similar = "1.3" smol = "1.2.5" +thiserror = "1.0.29" toml = "0.5" [dev-dependencies] diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 28e3a16fd5..d2856f876d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -49,6 +49,7 @@ use std::{ }, time::Instant, }; +use thiserror::Error; use util::{post_inc, ResultExt, TryFutureExt as _}; pub use fs::*; @@ -90,6 +91,18 @@ pub struct Project { nonce: u128, } +#[derive(Error, Debug)] +pub enum JoinProjectError { + #[error("host declined join request")] + HostDeclined, + #[error("host closed the project")] + HostClosedProject, + #[error("host went offline")] + HostWentOffline, + #[error("{0}")] + Other(#[from] anyhow::Error), +} + enum OpenBuffer { Strong(ModelHandle), Weak(WeakModelHandle), @@ -356,7 +369,7 @@ impl Project { languages: Arc, fs: Arc, cx: &mut AsyncAppContext, - ) -> Result> { + ) -> Result, JoinProjectError> { client.authenticate_and_connect(true, &cx).await?; let response = client @@ -367,7 +380,20 @@ impl Project { let response = match response.variant.ok_or_else(|| anyhow!("missing variant"))? { proto::join_project_response::Variant::Accept(response) => response, - proto::join_project_response::Variant::Decline(_) => Err(anyhow!("rejected"))?, + proto::join_project_response::Variant::Decline(decline) => { + match proto::join_project_response::decline::Reason::from_i32(decline.reason) { + Some(proto::join_project_response::decline::Reason::Declined) => { + Err(JoinProjectError::HostDeclined)? + } + Some(proto::join_project_response::decline::Reason::Closed) => { + Err(JoinProjectError::HostClosedProject)? + } + Some(proto::join_project_response::decline::Reason::WentOffline) => { + Err(JoinProjectError::HostWentOffline)? + } + None => Err(anyhow!("missing decline reason"))?, + } + } }; let replica_id = response.replica_id as ReplicaId; diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 83ee637657..511348c9d0 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -153,7 +153,15 @@ message JoinProjectResponse { repeated LanguageServer language_servers = 4; } - message Decline {} + message Decline { + Reason reason = 1; + + enum Reason { + Declined = 0; + Closed = 1; + WentOffline = 2; + } + } } message LeaveProject { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 8f659aa1d6..85e03461a9 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2310,10 +2310,11 @@ pub fn join_project( let app_state = app_state.clone(); cx.spawn(|mut cx| async move { - let (window, joining_notice) = - cx.update(|cx| cx.add_window((app_state.build_window_options)(), |_| JoiningNotice { + let (window, joining_notice) = cx.update(|cx| { + cx.add_window((app_state.build_window_options)(), |_| JoiningNotice { message: "Loading remote project...", - })); + }) + }); let project = Project::remote( project_id, app_state.client.clone(), @@ -2336,13 +2337,24 @@ pub fn join_project( ); workspace })), - Err(error) => { - joining_notice.update(cx, |joining_notice, cx| { - joining_notice.message = "An error occurred trying to join the project. Please, close this window and retry."; + Err(error @ _) => { + let message = match error { + project::JoinProjectError::HostDeclined => { + "The host declined your request to join." + } + project::JoinProjectError::HostClosedProject => "The host closed the project.", + project::JoinProjectError::HostWentOffline => "The host went offline.", + project::JoinProjectError::Other(_) => { + "An error occurred when attempting to join the project." + } + }; + joining_notice.update(cx, |notice, cx| { + notice.message = message; cx.notify(); }); - Err(error) - }, + + Err(error)? + } }) }) }