diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 370a2ba6ff..3c2a831ce2 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 c2428150fc..ac18907894 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 2a715fa4a4..7170f7f1c5 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/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 941053be53..ecfab714f4 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -197,6 +197,18 @@ 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; } message Test { diff --git a/crates/rpc/src/error.rs b/crates/rpc/src/error.rs new file mode 100644 index 0000000000..64c75a8d2f --- /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() convers 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 20a36efdfe..cfb5223930 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 da0880377f..c22ecb740d 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/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f645b1e7fa..e7f5e2e61e 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}; @@ -3919,10 +3919,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 +3995,23 @@ pub fn join_channel( if let Some(active_window) = active_window { active_window .update(&mut cx, |_, cx| { + let message:SharedString = match err.error_code() { + ErrorCode::SignedOut => { + "Failed to join channel\n\nPlease sign in to continue.".into() + }, + ErrorCode::UpgradeRequired => { + "Failed to join channel\n\nPlease update to the latest version of Zed to continue.".into() + }, + ErrorCode::NoSuchChannel => { + "Failed to find channel\n\nPlease check the link and try again.".into() + }, + ErrorCode::Disconnected => "Failed to join channel\n\nPlease check your internet connection and try again.".into(), + ErrorCode::WrongReleaseChannel => format!("Failed to join channel\n\nOther people in the channel are using the {} release of Zed, please switch to that release instead.", err.error_tag("required").unwrap_or("other")).into(), + _ => format!("Failed to join channel\n\n{}\n\nPlease try again.", err).into(), + }; cx.prompt( PromptLevel::Critical, - &format!("Failed to join channel: {}", err), + &message, &["Ok"], ) })?