From a15b307a174c6b21a3448758ecd0ce7448f11e44 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 24 Jan 2024 23:23:58 -0700 Subject: [PATCH] collab errors (#4152) One of the complaints of users on our first Hack call was that the error messages you got when channel joining failed were not great. This aims to fix that specific case, and lay the groundwork for future improvements. It adds two new methods to anyhow::Error * `.error_code()` which returns a value from zed.proto (or ErrorCode::Internal if the error has no specific tag) * `.error_tag("key")` which returns the value of the tag (or None). To construct errors with these fields set, you can use a builder API based on the ErrorCode type: * `Err(ErrorCode::Forbidden.anyhow())` * `Err(ErrorCode::Forbidden.message("cannot join channel").into())` - to add any context you want in the logs * `Err(ErrorCode::WrongReleaseChannel.tag("required", "stable").into())` - to add structured metadata to help the client handle the error better. Release Notes: - Improved error messaging when channel joining fails. --- crates/assistant/src/assistant_panel.rs | 1 + crates/auto_update/src/auto_update.rs | 3 +- crates/client/src/client.rs | 7 +- crates/collab/src/db/queries/channels.rs | 10 +- crates/collab/src/rpc.rs | 15 +- crates/collab_ui/src/collab_panel.rs | 64 +++-- .../src/collab_panel/channel_modal.rs | 8 +- crates/feedback/src/feedback.rs | 3 +- crates/feedback/src/feedback_modal.rs | 4 +- crates/gpui/src/platform.rs | 8 +- crates/gpui/src/platform/mac/window.rs | 11 +- crates/gpui/src/platform/test/window.rs | 1 + crates/gpui/src/window.rs | 5 +- crates/project_panel/src/project_panel.rs | 1 + crates/rpc/proto/zed.proto | 13 + crates/rpc/src/error.rs | 223 ++++++++++++++++++ crates/rpc/src/peer.rs | 41 ++-- crates/rpc/src/rpc.rs | 2 + crates/search/src/project_search.rs | 1 + crates/workspace/src/notifications.rs | 42 +++- crates/workspace/src/pane.rs | 17 +- crates/workspace/src/workspace.rs | 56 +++-- crates/zed/src/zed.rs | 11 +- 23 files changed, 446 insertions(+), 101 deletions(-) create mode 100644 crates/rpc/src/error.rs diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index b2c539fcc21dc1ccb05532e5a5aa7e9ca7012b0a..3fcbb9a3c995be7b4c2362f5d76a20059f250767 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -2959,6 +2959,7 @@ impl InlineAssistant { cx.prompt( PromptLevel::Info, prompt_text.as_str(), + None, &["Continue", "Cancel"], ) })?; diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 67ca15e295b2f05b2f0481bc6a4ecd3b03014c52..e322b1cf0de5cb35ce378d6b628aef7d4fbee986 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -130,7 +130,8 @@ pub fn check(_: &Check, cx: &mut WindowContext) { } else { drop(cx.prompt( gpui::PromptLevel::Info, - "Auto-updates disabled for non-bundled app.", + "Could not check for updates", + Some("Auto-updates disabled for non-bundled app."), &["Ok"], )); } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 370a2ba6ffdb8ab13d8279c19e9207990b160b20..3c2a831ce2786b4503794234a41d18db8f773be7 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -689,12 +689,7 @@ impl Client { Ok(()) } Err(error) => { - client.respond_with_error( - receipt, - proto::Error { - message: format!("{:?}", error), - }, - )?; + client.respond_with_error(receipt, error.to_proto())?; Err(error) } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index c2428150fc4e53d5450330a58db01afad1f76c4c..ac18907894d995b53da977a4337ea532e4dd2920 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -1,5 +1,5 @@ use super::*; -use rpc::proto::channel_member::Kind; +use rpc::{proto::channel_member::Kind, ErrorCode, ErrorCodeExt}; use sea_orm::TryGetableMany; impl Database { @@ -166,7 +166,7 @@ impl Database { } if role.is_none() || role == Some(ChannelRole::Banned) { - Err(anyhow!("not allowed"))? + Err(ErrorCode::Forbidden.anyhow())? } let role = role.unwrap(); @@ -1201,7 +1201,7 @@ impl Database { Ok(channel::Entity::find_by_id(channel_id) .one(&*tx) .await? - .ok_or_else(|| anyhow!("no such channel"))?) + .ok_or_else(|| proto::ErrorCode::NoSuchChannel.anyhow())?) } pub(crate) async fn get_or_create_channel_room( @@ -1219,7 +1219,9 @@ impl Database { let room_id = if let Some(room) = room { if let Some(env) = room.environment { if &env != environment { - Err(anyhow!("must join using the {} release", env))?; + Err(ErrorCode::WrongReleaseChannel + .with_tag("required", &env) + .anyhow())?; } } room.id diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 2a715fa4a456ccd0564b88a2afe662136e151d6a..7170f7f1c5b65b389ed246b93c2c1329ce48d735 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -10,7 +10,7 @@ use crate::{ User, UserId, }, executor::Executor, - AppState, Result, + AppState, Error, Result, }; use anyhow::anyhow; use async_tungstenite::tungstenite::{ @@ -44,7 +44,7 @@ use rpc::{ self, Ack, AnyTypedEnvelope, EntityMessage, EnvelopedMessage, LiveKitConnectionInfo, RequestMessage, ShareProject, UpdateChannelBufferCollaborators, }, - Connection, ConnectionId, Peer, Receipt, TypedEnvelope, + Connection, ConnectionId, ErrorCode, ErrorCodeExt, ErrorExt, Peer, Receipt, TypedEnvelope, }; use serde::{Serialize, Serializer}; use std::{ @@ -543,12 +543,11 @@ impl Server { } } Err(error) => { - peer.respond_with_error( - receipt, - proto::Error { - message: error.to_string(), - }, - )?; + let proto_err = match &error { + Error::Internal(err) => err.to_proto(), + _ => ErrorCode::Internal.message(format!("{}", error)).to_proto(), + }; + peer.respond_with_error(receipt, proto_err)?; Err(error) } } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 28c76b155eabbefdd087f1732e794ee706e5bf2f..c42a4e35cd37bffd62f617481a64361520a5d775 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -22,7 +22,10 @@ use gpui::{ }; use menu::{Cancel, Confirm, SecondaryConfirm, SelectNext, SelectPrev}; use project::{Fs, Project}; -use rpc::proto::{self, PeerId}; +use rpc::{ + proto::{self, PeerId}, + ErrorCode, ErrorExt, +}; use serde_derive::{Deserialize, Serialize}; use settings::Settings; use smallvec::SmallVec; @@ -35,7 +38,7 @@ use ui::{ use util::{maybe, ResultExt, TryFutureExt}; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, - notifications::{NotifyResultExt, NotifyTaskExt}, + notifications::{DetachAndPromptErr, NotifyResultExt, NotifyTaskExt}, Workspace, }; @@ -879,7 +882,7 @@ impl CollabPanel { .update(cx, |workspace, cx| { let app_state = workspace.app_state().clone(); workspace::join_remote_project(project_id, host_user_id, app_state, cx) - .detach_and_log_err(cx); + .detach_and_prompt_err("Failed to join project", cx, |_, _| None); }) .ok(); })) @@ -1017,7 +1020,12 @@ impl CollabPanel { ) }) }) - .detach_and_notify_err(cx) + .detach_and_prompt_err("Failed to grant write access", cx, |e, _| { + match e.error_code() { + ErrorCode::NeedsCla => Some("This user has not yet signed the CLA at https://zed.dev/cla.".into()), + _ => None, + } + }) }), ) } else if role == proto::ChannelRole::Member { @@ -1038,7 +1046,7 @@ impl CollabPanel { ) }) }) - .detach_and_notify_err(cx) + .detach_and_prompt_err("Failed to revoke write access", cx, |_, _| None) }), ) } else { @@ -1258,7 +1266,11 @@ impl CollabPanel { app_state, cx, ) - .detach_and_log_err(cx); + .detach_and_prompt_err( + "Failed to join project", + cx, + |_, _| None, + ); } } ListEntry::ParticipantScreen { peer_id, .. } => { @@ -1432,7 +1444,7 @@ impl CollabPanel { fn leave_call(cx: &mut WindowContext) { ActiveCall::global(cx) .update(cx, |call, cx| call.hang_up(cx)) - .detach_and_log_err(cx); + .detach_and_prompt_err("Failed to hang up", cx, |_, _| None); } fn toggle_contact_finder(&mut self, cx: &mut ViewContext) { @@ -1534,11 +1546,11 @@ impl CollabPanel { cx: &mut ViewContext, ) { if let Some(clipboard) = self.channel_clipboard.take() { - self.channel_store.update(cx, |channel_store, cx| { - channel_store - .move_channel(clipboard.channel_id, Some(to_channel_id), cx) - .detach_and_log_err(cx) - }) + self.channel_store + .update(cx, |channel_store, cx| { + channel_store.move_channel(clipboard.channel_id, Some(to_channel_id), cx) + }) + .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) } } @@ -1610,7 +1622,12 @@ impl CollabPanel { "Are you sure you want to remove the channel \"{}\"?", channel.name ); - let answer = cx.prompt(PromptLevel::Warning, &prompt_message, &["Remove", "Cancel"]); + let answer = cx.prompt( + PromptLevel::Warning, + &prompt_message, + None, + &["Remove", "Cancel"], + ); cx.spawn(|this, mut cx| async move { if answer.await? == 0 { channel_store @@ -1631,7 +1648,12 @@ impl CollabPanel { "Are you sure you want to remove \"{}\" from your contacts?", github_login ); - let answer = cx.prompt(PromptLevel::Warning, &prompt_message, &["Remove", "Cancel"]); + let answer = cx.prompt( + PromptLevel::Warning, + &prompt_message, + None, + &["Remove", "Cancel"], + ); cx.spawn(|_, mut cx| async move { if answer.await? == 0 { user_store @@ -1641,7 +1663,7 @@ impl CollabPanel { } anyhow::Ok(()) }) - .detach_and_log_err(cx); + .detach_and_prompt_err("Failed to remove contact", cx, |_, _| None); } fn respond_to_contact_request( @@ -1654,7 +1676,7 @@ impl CollabPanel { .update(cx, |store, cx| { store.respond_to_contact_request(user_id, accept, cx) }) - .detach_and_log_err(cx); + .detach_and_prompt_err("Failed to respond to contact request", cx, |_, _| None); } fn respond_to_channel_invite( @@ -1675,7 +1697,7 @@ impl CollabPanel { .update(cx, |call, cx| { call.invite(recipient_user_id, Some(self.project.clone()), cx) }) - .detach_and_log_err(cx); + .detach_and_prompt_err("Call failed", cx, |_, _| None); } fn join_channel(&self, channel_id: u64, cx: &mut ViewContext) { @@ -1691,7 +1713,7 @@ impl CollabPanel { Some(handle), cx, ) - .detach_and_log_err(cx) + .detach_and_prompt_err("Failed to join channel", cx, |_, _| None) } fn join_channel_chat(&mut self, channel_id: ChannelId, cx: &mut ViewContext) { @@ -1704,7 +1726,7 @@ impl CollabPanel { panel.update(cx, |panel, cx| { panel .select_channel(channel_id, None, cx) - .detach_and_log_err(cx); + .detach_and_notify_err(cx); }); } }); @@ -1981,7 +2003,7 @@ impl CollabPanel { .update(cx, |channel_store, cx| { channel_store.move_channel(dragged_channel.id, None, cx) }) - .detach_and_log_err(cx) + .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) })) }) } @@ -2256,7 +2278,7 @@ impl CollabPanel { .update(cx, |channel_store, cx| { channel_store.move_channel(dragged_channel.id, Some(channel_id), cx) }) - .detach_and_log_err(cx) + .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) })) .child( ListItem::new(channel_id as usize) diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 3d7facf2e8d917d7ea93a9f2e0e57fa306a9bf5d..891c609316162f0d722379f4d6a8298c4a37d032 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -14,7 +14,7 @@ use rpc::proto::channel_member; use std::sync::Arc; use ui::{prelude::*, Avatar, Checkbox, ContextMenu, ListItem, ListItemSpacing}; use util::TryFutureExt; -use workspace::{notifications::NotifyTaskExt, ModalView}; +use workspace::{notifications::DetachAndPromptErr, ModalView}; actions!( channel_modal, @@ -498,7 +498,7 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_notify_err(cx); + .detach_and_prompt_err("Failed to update role", cx, |_, _| None); Some(()) } @@ -530,7 +530,7 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_notify_err(cx); + .detach_and_prompt_err("Failed to remove member", cx, |_, _| None); Some(()) } @@ -556,7 +556,7 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_notify_err(cx); + .detach_and_prompt_err("Failed to invite member", cx, |_, _| None); } fn show_context_menu(&mut self, ix: usize, cx: &mut ViewContext>) { diff --git a/crates/feedback/src/feedback.rs b/crates/feedback/src/feedback.rs index 58e68e2197532151a0cd1dae93540a1d53c0c4fc..0690c412cd8d91b0203d64164b03f09b687ef377 100644 --- a/crates/feedback/src/feedback.rs +++ b/crates/feedback/src/feedback.rs @@ -31,7 +31,8 @@ pub fn init(cx: &mut AppContext) { let prompt = cx.prompt( PromptLevel::Info, - &format!("Copied into clipboard:\n\n{specs}"), + "Copied into clipboard", + Some(&specs), &["OK"], ); cx.spawn(|_, _cx| async move { diff --git a/crates/feedback/src/feedback_modal.rs b/crates/feedback/src/feedback_modal.rs index 99c96fe8808c51af845b5fb261a75d88405d4ef7..446ebe602151e6788e63076a924e7fa66a6c040b 100644 --- a/crates/feedback/src/feedback_modal.rs +++ b/crates/feedback/src/feedback_modal.rs @@ -97,7 +97,7 @@ impl ModalView for FeedbackModal { return true; } - let answer = cx.prompt(PromptLevel::Info, "Discard feedback?", &["Yes", "No"]); + let answer = cx.prompt(PromptLevel::Info, "Discard feedback?", None, &["Yes", "No"]); cx.spawn(move |this, mut cx| async move { if answer.await.ok() == Some(0) { @@ -222,6 +222,7 @@ impl FeedbackModal { let answer = cx.prompt( PromptLevel::Info, "Ready to submit your feedback?", + None, &["Yes, Submit!", "No"], ); let client = cx.global::>().clone(); @@ -255,6 +256,7 @@ impl FeedbackModal { let prompt = cx.prompt( PromptLevel::Critical, FEEDBACK_SUBMISSION_ERROR_TEXT, + None, &["OK"], ); cx.spawn(|_, _cx| async move { diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 3d2679dd7ecabe5d92e31038fa9c5d4a02f9d55c..a7b71c78858366eda21abddf1e8be57ffec78626 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -150,7 +150,13 @@ pub(crate) trait PlatformWindow { fn as_any_mut(&mut self) -> &mut dyn Any; fn set_input_handler(&mut self, input_handler: PlatformInputHandler); fn take_input_handler(&mut self) -> Option; - fn prompt(&self, level: PromptLevel, msg: &str, answers: &[&str]) -> oneshot::Receiver; + fn prompt( + &self, + level: PromptLevel, + msg: &str, + detail: Option<&str>, + answers: &[&str], + ) -> oneshot::Receiver; fn activate(&self); fn set_title(&mut self, title: &str); fn set_edited(&mut self, edited: bool); diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index 5b454d9b4e06c958c59feb75b2304c977902a047..2e5a14cd8d6a5b1b4713ffb1075437c0c2e8d430 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -772,7 +772,13 @@ impl PlatformWindow for MacWindow { self.0.as_ref().lock().input_handler.take() } - fn prompt(&self, level: PromptLevel, msg: &str, answers: &[&str]) -> oneshot::Receiver { + fn prompt( + &self, + level: PromptLevel, + msg: &str, + detail: Option<&str>, + answers: &[&str], + ) -> oneshot::Receiver { // macOs applies overrides to modal window buttons after they are added. // Two most important for this logic are: // * Buttons with "Cancel" title will be displayed as the last buttons in the modal @@ -808,6 +814,9 @@ impl PlatformWindow for MacWindow { }; let _: () = msg_send![alert, setAlertStyle: alert_style]; let _: () = msg_send![alert, setMessageText: ns_string(msg)]; + if let Some(detail) = detail { + let _: () = msg_send![alert, setInformativeText: ns_string(detail)]; + } for (ix, answer) in answers .iter() diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index c03384aadfcdcbf7bcebf0eddd298a55e0dab2a4..af58707c6d444d3b321ccc6385fbdca1f89ff42f 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -185,6 +185,7 @@ impl PlatformWindow for TestWindow { &self, _level: crate::PromptLevel, _msg: &str, + _detail: Option<&str>, _answers: &[&str], ) -> futures::channel::oneshot::Receiver { self.0 diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index d8522837b7ef566f51eae4d901d6694318b31b88..618c7eb4e450bb48a171faa78335eb389acafd2e 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -1478,9 +1478,12 @@ impl<'a> WindowContext<'a> { &self, level: PromptLevel, message: &str, + detail: Option<&str>, answers: &[&str], ) -> oneshot::Receiver { - self.window.platform_window.prompt(level, message, answers) + self.window + .platform_window + .prompt(level, message, detail, answers) } /// Returns all available actions for the focused element. diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 1ad483837f7ef4f583ad0182b67a440164cbad6f..63524d682ee1295a5b31a3a32531fb6580fa40ef 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -781,6 +781,7 @@ impl ProjectPanel { let answer = cx.prompt( PromptLevel::Info, &format!("Delete {file_name:?}?"), + None, &["Delete", "Cancel"], ); diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 941053be538b6f50b110e2a069bca6f4e5d0518f..8bea2f24e0d116a0d954708fec15d353c77a616f 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -197,6 +197,19 @@ message Ack {} message Error { string message = 1; + ErrorCode code = 2; + repeated string tags = 3; +} + +enum ErrorCode { + Internal = 0; + NoSuchChannel = 1; + Disconnected = 2; + SignedOut = 3; + UpgradeRequired = 4; + Forbidden = 5; + WrongReleaseChannel = 6; + NeedsCla = 7; } message Test { diff --git a/crates/rpc/src/error.rs b/crates/rpc/src/error.rs new file mode 100644 index 0000000000000000000000000000000000000000..97f93e74657864e6827fbff116bbe7d8c1b302a9 --- /dev/null +++ b/crates/rpc/src/error.rs @@ -0,0 +1,223 @@ +/// Some helpers for structured error handling. +/// +/// The helpers defined here allow you to pass type-safe error codes from +/// the collab server to the client; and provide a mechanism for additional +/// structured data alongside the message. +/// +/// When returning an error, it can be as simple as: +/// +/// `return Err(Error::Forbidden.into())` +/// +/// If you'd like to log more context, you can set a message. These messages +/// show up in our logs, but are not shown visibly to users. +/// +/// `return Err(Error::Forbidden.message("not an admin").into())` +/// +/// If you'd like to provide enough context that the UI can render a good error +/// message (or would be helpful to see in a structured format in the logs), you +/// can use .with_tag(): +/// +/// `return Err(Error::WrongReleaseChannel.with_tag("required", "stable").into())` +/// +/// When handling an error you can use .error_code() to match which error it was +/// and .error_tag() to read any tags. +/// +/// ``` +/// match err.error_code() { +/// ErrorCode::Forbidden => alert("I'm sorry I can't do that.") +/// ErrorCode::WrongReleaseChannel => +/// alert(format!("You need to be on the {} release channel.", err.error_tag("required").unwrap())) +/// ErrorCode::Internal => alert("Sorry, something went wrong") +/// } +/// ``` +/// +use crate::proto; +pub use proto::ErrorCode; + +/// ErrorCodeExt provides some helpers for structured error handling. +/// +/// The primary implementation is on the proto::ErrorCode to easily convert +/// that into an anyhow::Error, which we use pervasively. +/// +/// The RpcError struct provides support for further metadata if needed. +pub trait ErrorCodeExt { + /// Return an anyhow::Error containing this. + /// (useful in places where .into() doesn't have enough type information) + fn anyhow(self) -> anyhow::Error; + + /// Add a message to the error (by default the error code is used) + fn message(self, msg: String) -> RpcError; + + /// Add a tag to the error. Tags are key value pairs that can be used + /// to send semi-structured data along with the error. + fn with_tag(self, k: &str, v: &str) -> RpcError; +} + +impl ErrorCodeExt for proto::ErrorCode { + fn anyhow(self) -> anyhow::Error { + self.into() + } + + fn message(self, msg: String) -> RpcError { + let err: RpcError = self.into(); + err.message(msg) + } + + fn with_tag(self, k: &str, v: &str) -> RpcError { + let err: RpcError = self.into(); + err.with_tag(k, v) + } +} + +/// ErrorExt provides helpers for structured error handling. +/// +/// The primary implementation is on the anyhow::Error, which is +/// what we use throughout our codebase. Though under the hood this +pub trait ErrorExt { + /// error_code() returns the ErrorCode (or ErrorCode::Internal if there is none) + fn error_code(&self) -> proto::ErrorCode; + /// error_tag() returns the value of the tag with the given key, if any. + fn error_tag(&self, k: &str) -> Option<&str>; + /// to_proto() converts the error into a proto::Error + fn to_proto(&self) -> proto::Error; +} + +impl ErrorExt for anyhow::Error { + fn error_code(&self) -> proto::ErrorCode { + if let Some(rpc_error) = self.downcast_ref::() { + rpc_error.code + } else { + proto::ErrorCode::Internal + } + } + + fn error_tag(&self, k: &str) -> Option<&str> { + if let Some(rpc_error) = self.downcast_ref::() { + rpc_error.error_tag(k) + } else { + None + } + } + + fn to_proto(&self) -> proto::Error { + if let Some(rpc_error) = self.downcast_ref::() { + rpc_error.to_proto() + } else { + ErrorCode::Internal.message(format!("{}", self)).to_proto() + } + } +} + +impl From for anyhow::Error { + fn from(value: proto::ErrorCode) -> Self { + RpcError { + request: None, + code: value, + msg: format!("{:?}", value).to_string(), + tags: Default::default(), + } + .into() + } +} + +#[derive(Clone, Debug)] +pub struct RpcError { + request: Option, + msg: String, + code: proto::ErrorCode, + tags: Vec, +} + +/// RpcError is a structured error type that is returned by the collab server. +/// In addition to a message, it lets you set a specific ErrorCode, and attach +/// small amounts of metadata to help the client handle the error appropriately. +/// +/// This struct is not typically used directly, as we pass anyhow::Error around +/// in the app; however it is useful for chaining .message() and .with_tag() on +/// ErrorCode. +impl RpcError { + /// from_proto converts a proto::Error into an anyhow::Error containing + /// an RpcError. + pub fn from_proto(error: &proto::Error, request: &str) -> anyhow::Error { + RpcError { + request: Some(request.to_string()), + code: error.code(), + msg: error.message.clone(), + tags: error.tags.clone(), + } + .into() + } +} + +impl ErrorCodeExt for RpcError { + fn message(mut self, msg: String) -> RpcError { + self.msg = msg; + self + } + + fn with_tag(mut self, k: &str, v: &str) -> RpcError { + self.tags.push(format!("{}={}", k, v)); + self + } + + fn anyhow(self) -> anyhow::Error { + self.into() + } +} + +impl ErrorExt for RpcError { + fn error_tag(&self, k: &str) -> Option<&str> { + for tag in &self.tags { + let mut parts = tag.split('='); + if let Some(key) = parts.next() { + if key == k { + return parts.next(); + } + } + } + None + } + + fn error_code(&self) -> proto::ErrorCode { + self.code + } + + fn to_proto(&self) -> proto::Error { + proto::Error { + code: self.code as i32, + message: self.msg.clone(), + tags: self.tags.clone(), + } + } +} + +impl std::error::Error for RpcError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + None + } +} + +impl std::fmt::Display for RpcError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + if let Some(request) = &self.request { + write!(f, "RPC request {} failed: {}", request, self.msg)? + } else { + write!(f, "{}", self.msg)? + } + for tag in &self.tags { + write!(f, " {}", tag)? + } + Ok(()) + } +} + +impl From for RpcError { + fn from(code: proto::ErrorCode) -> Self { + RpcError { + request: None, + code, + msg: format!("{:?}", code).to_string(), + tags: Default::default(), + } + } +} diff --git a/crates/rpc/src/peer.rs b/crates/rpc/src/peer.rs index 20a36efdfe1c1ae59fd9130668c7afda0b1f792e..9d789bd3d01aef7a08dd9cabe7b29e9d86fa1be8 100644 --- a/crates/rpc/src/peer.rs +++ b/crates/rpc/src/peer.rs @@ -1,3 +1,5 @@ +use crate::{ErrorCode, ErrorCodeExt, ErrorExt, RpcError}; + use super::{ proto::{self, AnyTypedEnvelope, EnvelopedMessage, MessageStream, PeerId, RequestMessage}, Connection, @@ -423,11 +425,7 @@ impl Peer { let (response, _barrier) = rx.await.map_err(|_| anyhow!("connection was closed"))?; if let Some(proto::envelope::Payload::Error(error)) = &response.payload { - Err(anyhow!( - "RPC request {} failed - {}", - T::NAME, - error.message - )) + Err(RpcError::from_proto(&error, T::NAME)) } else { Ok(TypedEnvelope { message_id: response.id, @@ -516,9 +514,12 @@ impl Peer { envelope: Box, ) -> Result<()> { let connection = self.connection_state(envelope.sender_id())?; - let response = proto::Error { - message: format!("message {} was not handled", envelope.payload_type_name()), - }; + let response = ErrorCode::Internal + .message(format!( + "message {} was not handled", + envelope.payload_type_name() + )) + .to_proto(); let message_id = connection .next_message_id .fetch_add(1, atomic::Ordering::SeqCst); @@ -692,17 +693,17 @@ mod tests { server .send( server_to_client_conn_id, - proto::Error { - message: "message 1".to_string(), - }, + ErrorCode::Internal + .message("message 1".to_string()) + .to_proto(), ) .unwrap(); server .send( server_to_client_conn_id, - proto::Error { - message: "message 2".to_string(), - }, + ErrorCode::Internal + .message("message 2".to_string()) + .to_proto(), ) .unwrap(); server.respond(request.receipt(), proto::Ack {}).unwrap(); @@ -797,17 +798,17 @@ mod tests { server .send( server_to_client_conn_id, - proto::Error { - message: "message 1".to_string(), - }, + ErrorCode::Internal + .message("message 1".to_string()) + .to_proto(), ) .unwrap(); server .send( server_to_client_conn_id, - proto::Error { - message: "message 2".to_string(), - }, + ErrorCode::Internal + .message("message 2".to_string()) + .to_proto(), ) .unwrap(); server.respond(request1.receipt(), proto::Ack {}).unwrap(); diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index da0880377fb7ef4e381587a08c4ac3e0a9a2e4dc..c22ecb740d4a52c84a7622e151b8e71e6ee2c07a 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -1,10 +1,12 @@ pub mod auth; mod conn; +mod error; mod notification; mod peer; pub mod proto; pub use conn::Connection; +pub use error::*; pub use notification::*; pub use peer::*; mod macros; diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index c1ebcfe0a6a4a8214d6cbc76b3ebca5e0e1f5125..0b169949b989efa8073e0bf11f08a8197733ff84 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -746,6 +746,7 @@ impl ProjectSearchView { cx.prompt( PromptLevel::Info, prompt_text.as_str(), + None, &["Continue", "Cancel"], ) })?; diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index 1b41b7040ceb00eacbcc38890018e852f446d95a..30d8ec9e82adaef24f724b2f74312ad95518ea68 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -1,8 +1,8 @@ use crate::{Toast, Workspace}; use collections::HashMap; use gpui::{ - AnyView, AppContext, AsyncWindowContext, DismissEvent, Entity, EntityId, EventEmitter, Render, - Task, View, ViewContext, VisualContext, WindowContext, + AnyView, AppContext, AsyncWindowContext, DismissEvent, Entity, EntityId, EventEmitter, + PromptLevel, Render, Task, View, ViewContext, VisualContext, WindowContext, }; use std::{any::TypeId, ops::DerefMut}; @@ -299,7 +299,7 @@ pub trait NotifyTaskExt { impl NotifyTaskExt for Task> where - E: std::fmt::Debug + 'static, + E: std::fmt::Debug + Sized + 'static, R: 'static, { fn detach_and_notify_err(self, cx: &mut WindowContext) { @@ -307,3 +307,39 @@ where .detach(); } } + +pub trait DetachAndPromptErr { + fn detach_and_prompt_err( + self, + msg: &str, + cx: &mut WindowContext, + f: impl FnOnce(&anyhow::Error, &mut WindowContext) -> Option + 'static, + ); +} + +impl DetachAndPromptErr for Task> +where + R: 'static, +{ + fn detach_and_prompt_err( + self, + msg: &str, + cx: &mut WindowContext, + f: impl FnOnce(&anyhow::Error, &mut WindowContext) -> Option + 'static, + ) { + let msg = msg.to_owned(); + cx.spawn(|mut cx| async move { + if let Err(err) = self.await { + log::error!("{err:?}"); + if let Ok(prompt) = cx.update(|cx| { + let detail = f(&err, cx) + .unwrap_or_else(|| format!("{err:?}. Please try again.", err = err)); + cx.prompt(PromptLevel::Critical, &msg, Some(&detail), &["Ok"]) + }) { + prompt.await.ok(); + } + } + }) + .detach(); + } +} diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 7f164c6e69b22ce20d95308584d5819b48bd91f7..2336230c86feec631b535204afda2aff6102e11e 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -870,7 +870,7 @@ impl Pane { items: &mut dyn Iterator>, all_dirty_items: usize, cx: &AppContext, - ) -> String { + ) -> (String, String) { /// Quantity of item paths displayed in prompt prior to cutoff.. const FILE_NAMES_CUTOFF_POINT: usize = 10; let mut file_names: Vec<_> = items @@ -894,10 +894,12 @@ impl Pane { file_names.push(format!(".. {} files not shown", not_shown_files).into()); } } - let file_names = file_names.join("\n"); - format!( - "Do you want to save changes to the following {} files?\n{file_names}", - all_dirty_items + ( + format!( + "Do you want to save changes to the following {} files?", + all_dirty_items + ), + file_names.join("\n"), ) } @@ -929,11 +931,12 @@ impl Pane { cx.spawn(|pane, mut cx| async move { if save_intent == SaveIntent::Close && dirty_items.len() > 1 { let answer = pane.update(&mut cx, |_, cx| { - let prompt = + let (prompt, detail) = Self::file_names_for_prompt(&mut dirty_items.iter(), dirty_items.len(), cx); cx.prompt( PromptLevel::Warning, &prompt, + Some(&detail), &["Save all", "Discard all", "Cancel"], ) })?; @@ -1131,6 +1134,7 @@ impl Pane { cx.prompt( PromptLevel::Warning, CONFLICT_MESSAGE, + None, &["Overwrite", "Discard", "Cancel"], ) })?; @@ -1154,6 +1158,7 @@ impl Pane { cx.prompt( PromptLevel::Warning, &prompt, + None, &["Save", "Don't Save", "Cancel"], ) })?; diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a552d9c5af645d5a76ba8010b52dae79139b022a..94de08938b5fa976e6b53b038f8b85f35b66ef20 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -14,8 +14,8 @@ mod workspace_settings; use anyhow::{anyhow, Context as _, Result}; use call::ActiveCall; use client::{ - proto::{self, PeerId}, - Client, Status, TypedEnvelope, UserStore, + proto::{self, ErrorCode, PeerId}, + Client, ErrorExt, Status, TypedEnvelope, UserStore, }; use collections::{hash_map, HashMap, HashSet}; use dock::{Dock, DockPosition, Panel, PanelButtons, PanelHandle}; @@ -30,8 +30,8 @@ use gpui::{ DragMoveEvent, Element, ElementContext, Entity, EntityId, EventEmitter, FocusHandle, FocusableView, GlobalPixels, InteractiveElement, IntoElement, KeyContext, LayoutId, ManagedView, Model, ModelContext, ParentElement, PathPromptOptions, Pixels, Point, PromptLevel, - Render, Size, Styled, Subscription, Task, View, ViewContext, VisualContext, WeakView, - WindowBounds, WindowContext, WindowHandle, WindowOptions, + Render, SharedString, Size, Styled, Subscription, Task, View, ViewContext, VisualContext, + WeakView, WindowBounds, WindowContext, WindowHandle, WindowOptions, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ItemSettings, ProjectItem}; use itertools::Itertools; @@ -1159,6 +1159,7 @@ impl Workspace { cx.prompt( PromptLevel::Warning, "Do you want to leave the current call?", + None, &["Close window and hang up", "Cancel"], ) })?; @@ -1214,7 +1215,7 @@ impl Workspace { // Override save mode and display "Save all files" prompt if save_intent == SaveIntent::Close && dirty_items.len() > 1 { let answer = workspace.update(&mut cx, |_, cx| { - let prompt = Pane::file_names_for_prompt( + let (prompt, detail) = Pane::file_names_for_prompt( &mut dirty_items.iter().map(|(_, handle)| handle), dirty_items.len(), cx, @@ -1222,6 +1223,7 @@ impl Workspace { cx.prompt( PromptLevel::Warning, &prompt, + Some(&detail), &["Save all", "Discard all", "Cancel"], ) })?; @@ -3887,13 +3889,16 @@ async fn join_channel_internal( if should_prompt { if let Some(workspace) = requesting_window { - let answer = workspace.update(cx, |_, cx| { - cx.prompt( - PromptLevel::Warning, - "Leaving this call will unshare your current project.\nDo you want to switch channels?", - &["Yes, Join Channel", "Cancel"], - ) - })?.await; + let answer = workspace + .update(cx, |_, cx| { + cx.prompt( + PromptLevel::Warning, + "Do you want to switch channels?", + Some("Leaving this call will unshare your current project."), + &["Yes, Join Channel", "Cancel"], + ) + })? + .await; if answer == Ok(1) { return Ok(false); @@ -3919,10 +3924,10 @@ async fn join_channel_internal( | Status::Reconnecting | Status::Reauthenticating => continue, Status::Connected { .. } => break 'outer, - Status::SignedOut => return Err(anyhow!("not signed in")), - Status::UpgradeRequired => return Err(anyhow!("zed is out of date")), + Status::SignedOut => return Err(ErrorCode::SignedOut.into()), + Status::UpgradeRequired => return Err(ErrorCode::UpgradeRequired.into()), Status::ConnectionError | Status::ConnectionLost | Status::ReconnectionError { .. } => { - return Err(anyhow!("zed is offline")) + return Err(ErrorCode::Disconnected.into()) } } } @@ -3995,9 +4000,27 @@ pub fn join_channel( if let Some(active_window) = active_window { active_window .update(&mut cx, |_, cx| { + let detail: SharedString = match err.error_code() { + ErrorCode::SignedOut => { + "Please sign in to continue.".into() + }, + ErrorCode::UpgradeRequired => { + "Your are running an unsupported version of Zed. Please update to continue.".into() + }, + ErrorCode::NoSuchChannel => { + "No matching channel was found. Please check the link and try again.".into() + }, + ErrorCode::Forbidden => { + "This channel is private, and you do not have access. Please ask someone to add you and try again.".into() + }, + ErrorCode::Disconnected => "Please check your internet connection and try again.".into(), + ErrorCode::WrongReleaseChannel => format!("Others in the channel are using the {} release of Zed. Please switch to join this call.", err.error_tag("required").unwrap_or("other")).into(), + _ => format!("{}\n\nPlease try again.", err).into(), + }; cx.prompt( PromptLevel::Critical, - &format!("Failed to join channel: {}", err), + "Failed to join channel", + Some(&detail), &["Ok"], ) })? @@ -4224,6 +4247,7 @@ pub fn restart(_: &Restart, cx: &mut AppContext) { cx.prompt( PromptLevel::Info, "Are you sure you want to restart?", + None, &["Restart", "Cancel"], ) }) diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 3c714ad5da131da46054703e992d071ab96dc1bb..7a4e9707bf57451b33eff0b2aa76b374b02b6b4e 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -385,16 +385,12 @@ fn initialize_pane(workspace: &mut Workspace, pane: &View, cx: &mut ViewCo } fn about(_: &mut Workspace, _: &About, cx: &mut gpui::ViewContext) { - use std::fmt::Write as _; - let app_name = cx.global::().display_name(); let version = env!("CARGO_PKG_VERSION"); - let mut message = format!("{app_name} {version}"); - if let Some(sha) = cx.try_global::() { - write!(&mut message, "\n\n{}", sha.0).unwrap(); - } + let message = format!("{app_name} {version}"); + let detail = cx.try_global::().map(|sha| sha.0.as_ref()); - let prompt = cx.prompt(PromptLevel::Info, &message, &["OK"]); + let prompt = cx.prompt(PromptLevel::Info, &message, detail, &["OK"]); cx.foreground_executor() .spawn(async { prompt.await.ok(); @@ -425,6 +421,7 @@ fn quit(_: &Quit, cx: &mut AppContext) { cx.prompt( PromptLevel::Info, "Are you sure you want to quit?", + None, &["Quit", "Cancel"], ) })