rust-sdk: Rework our error handling.

src/async_client.rs

@@ -15,6 +15,7 @@
 
 use futures::future::{BoxFuture, Future, FutureExt};
 use std::convert::{TryFrom, TryInto};
+use std::result::Result as StdResult;
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{Arc, Mutex, RwLock as SyncLock};
 use std::time::{Duration, Instant};
@@ -37,9 +38,9 @@ use ruma_identifiers::RoomId;
 use crate::api;
 use crate::base_client::Client as BaseClient;
 use crate::base_client::Room;
-use crate::error::{Error, InnerError};
 use crate::session::Session;
 use crate::VERSION;
+use crate::{Error, Result};
 
 type RoomEventCallback = Box<
     dyn FnMut(Arc<SyncLock<Room>>, Arc<EventResult<RoomEvent>>) -> BoxFuture<'static, ()> + Send,
@@ -105,7 +106,7 @@ impl AsyncClientConfig {
     ///     .proxy("http://localhost:8080")
     ///     .unwrap();
     /// ```
-    pub fn proxy(mut self, proxy: &str) -> Result<Self, Error> {
+    pub fn proxy(mut self, proxy: &str) -> Result<Self> {
         self.proxy = Some(reqwest::Proxy::all(proxy)?);
         Ok(self)
     }
@@ -117,7 +118,7 @@ impl AsyncClientConfig {
     }
 
     /// Set a custom HTTP user agent for the client.
-    pub fn user_agent(mut self, user_agent: &str) -> Result<Self, InvalidHeaderValue> {
+    pub fn user_agent(mut self, user_agent: &str) -> StdResult<Self, InvalidHeaderValue> {
         self.user_agent = Some(HeaderValue::from_str(user_agent)?);
         Ok(self)
     }
@@ -153,7 +154,10 @@ impl SyncSettings {
     /// # Arguments
     ///
     /// * `timeout` - The time the server is allowed to wait.
-    pub fn timeout<T: TryInto<UInt>>(mut self, timeout: T) -> Result<Self, js_int::TryFromIntError>
+    pub fn timeout<T: TryInto<UInt>>(
+        mut self,
+        timeout: T,
+    ) -> StdResult<Self, js_int::TryFromIntError>
     where
         js_int::TryFromIntError:
             std::convert::From<<T as std::convert::TryInto<js_int::UInt>>::Error>,
@@ -189,10 +193,7 @@ impl AsyncClient {
     /// * `homeserver_url` - The homeserver that the client should connect to.
     /// * `session` - If a previous login exists, the access token can be
     ///     reused by giving a session object here.
-    pub fn new<U: TryInto<Url>>(
+    pub fn new<U: TryInto<Url>>(homeserver_url: U, session: Option<Session>) -> Result<Self> {
-        homeserver_url: U,
-        session: Option<Session>,
-    ) -> Result<Self, Error> {
         let config = AsyncClientConfig::new();
         AsyncClient::new_with_config(homeserver_url, session, config)
     }
@@ -209,7 +210,7 @@ impl AsyncClient {
         homeserver_url: U,
         session: Option<Session>,
         config: AsyncClientConfig,
-    ) -> Result<Self, Error> {
+    ) -> Result<Self> {
         let homeserver: Url = match homeserver_url.try_into() {
             Ok(u) => u,
             Err(_e) => panic!("Error parsing homeserver url"),
@@ -242,7 +243,7 @@ impl AsyncClient {
         Ok(Self {
             homeserver,
             http_client,
-            base_client: Arc::new(RwLock::new(BaseClient::new(session))),
+            base_client: Arc::new(RwLock::new(BaseClient::new(session)?)),
             transaction_id: Arc::new(AtomicU64::new(0)),
             event_callbacks: Arc::new(Mutex::new(Vec::new())),
         })
@@ -340,7 +341,7 @@ impl AsyncClient {
         user: S,
         password: S,
         device_id: Option<S>,
-    ) -> Result<login::Response, Error> {
+    ) -> Result<login::Response> {
         let request = login::Request {
             user: login::UserInfo::MatrixId(user.into()),
             login_info: login::LoginInfo::Password {
@@ -352,7 +353,7 @@ impl AsyncClient {
 
         let response = self.send(request).await?;
         let mut client = self.base_client.write().await;
-        client.receive_login_response(&response).await;
+        client.receive_login_response(&response).await?;
 
         Ok(response)
     }
@@ -365,7 +366,7 @@ impl AsyncClient {
     pub async fn sync(
         &mut self,
         sync_settings: SyncSettings,
-    ) -> Result<sync_events::IncomingResponse, Error> {
+    ) -> Result<sync_events::IncomingResponse> {
         let request = sync_events::Request {
             filter: None,
             since: sync_settings.token,
@@ -536,7 +537,7 @@ impl AsyncClient {
     async fn send<Request: Endpoint>(
         &self,
         request: Request,
-    ) -> Result<<Request::Response as Outgoing>::Incoming, Error>
+    ) -> Result<<Request::Response as Outgoing>::Incoming>
     where
         Request::Incoming:
             TryFrom<http::Request<Vec<u8>>, Error = ruma_api::error::FromHttpRequestError>,
@@ -570,7 +571,7 @@ impl AsyncClient {
             if let Some(ref session) = client.session {
                 request_builder.bearer_auth(&session.access_token)
             } else {
-                return Err(Error(InnerError::AuthenticationRequired));
+                return Err(Error::AuthenticationRequired);
             }
         } else {
             request_builder
@@ -604,7 +605,7 @@ impl AsyncClient {
         &mut self,
         room_id: &str,
         data: MessageEventContent,
-    ) -> Result<create_message_event::Response, Error> {
+    ) -> Result<create_message_event::Response> {
         let request = create_message_event::Request {
             room_id: RoomId::try_from(room_id).unwrap(),
             event_type: EventType::RoomMessage,
@@ -626,7 +627,7 @@ impl AsyncClient {
     /// Panics if the client isn't logged in, or if no encryption keys need to
     /// be uploaded.
     #[cfg(feature = "encryption")]
-    async fn keys_upload(&self) -> Result<upload_keys::Response, Error> {
+    async fn keys_upload(&self) -> Result<upload_keys::Response> {
         let (device_keys, one_time_keys) = self
             .base_client
             .read()

src/base_client.rs

@@ -14,8 +14,10 @@
 // limitations under the License.
 
 use std::collections::HashMap;
+use std::result::Result as StdResult;
 
 use crate::api::r0 as api;
+use crate::error::Result;
 use crate::events::collections::all::{RoomEvent, StateEvent};
 use crate::events::room::member::{MemberEvent, MembershipState};
 use crate::events::EventResult;
@@ -199,20 +201,20 @@ impl Client {
     ///
     /// * `session` - An optional session if the user already has one from a
     /// previous login call.
-    pub fn new(session: Option<Session>) -> Self {
+    pub fn new(session: Option<Session>) -> Result<Self> {
         #[cfg(feature = "encryption")]
         let olm = match &session {
-            Some(s) => Some(OlmMachine::new(&s.user_id, &s.device_id)),
+            Some(s) => Some(OlmMachine::new(&s.user_id, &s.device_id)?),
             None => None,
         };
 
-        Client {
+        Ok(Client {
             session,
             sync_token: None,
             joined_rooms: HashMap::new(),
             #[cfg(feature = "encryption")]
             olm: Arc::new(Mutex::new(olm)),
-        }
+        })
     }
 
     /// Is the client logged in.
@@ -226,7 +228,10 @@ impl Client {
     ///
     /// * `response` - A successful login response that contains our access token
     /// and device id.
-    pub async fn receive_login_response(&mut self, response: &api::session::login::Response) {
+    pub async fn receive_login_response(
+        &mut self,
+        response: &api::session::login::Response,
+    ) -> Result<()> {
         let session = Session {
             access_token: response.access_token.clone(),
             device_id: response.device_id.clone(),
@@ -237,8 +242,10 @@ impl Client {
         #[cfg(feature = "encryption")]
         {
             let mut olm = self.olm.lock().await;
-            *olm = Some(OlmMachine::new(&response.user_id, &response.device_id));
+            *olm = Some(OlmMachine::new(&response.user_id, &response.device_id)?);
         }
+
+        Ok(())
     }
 
     fn get_or_create_room(&mut self, room_id: &str) -> &mut Arc<RwLock<Room>> {
@@ -331,7 +338,9 @@ impl Client {
     ///
     /// Returns an empty error if no keys need to be uploaded.
     #[cfg(feature = "encryption")]
-    pub async fn keys_for_upload(&self) -> Result<(Option<DeviceKeys>, Option<OneTimeKeys>), ()> {
+    pub async fn keys_for_upload(
+        &self,
+    ) -> StdResult<(Option<DeviceKeys>, Option<OneTimeKeys>), ()> {
         let olm = self.olm.lock().await;
 
         match &*olm {

src/crypto/error.rs

@@ -13,35 +13,35 @@
 // limitations under the License.
 
 use cjson::Error as CjsonError;
-use std::fmt::{Display, Formatter, Result as FmtResult};
+use thiserror::Error;
 
-#[derive(Debug)]
+use super::store::CryptoStoreError;
+
+pub type Result<T> = std::result::Result<T, OlmError>;
+pub type VerificationResult<T> = std::result::Result<T, SignatureError>;
+
+#[derive(Error, Debug)]
 pub enum SignatureError {
+    #[error("the provided JSON value isn't an object")]
     NotAnObject,
+    #[error("the provided JSON object doesn't contain a signatures field")]
     NoSignatureFound,
+    #[error("the provided JSON object can't be converted to a canonical representation")]
     CanonicalJsonError(CjsonError),
+    #[error("the signature didn't match the provided key")]
     VerificationError,
 }
 
-impl Display for SignatureError {
-    fn fmt(&self, f: &mut Formatter) -> FmtResult {
-        let message = match self {
-            SignatureError::NotAnObject => "The provided JSON value isn't an object.",
-            SignatureError::NoSignatureFound => {
-                "The provided JSON object doesn't contain a signatures field."
-            }
-            SignatureError::CanonicalJsonError(_) => {
-                "The provided JSON object can't be converted to a canonical representation."
-            }
-            SignatureError::VerificationError => "The signature didn't match the provided key.",
-        };
-
-        write!(f, "{}", message)
-    }
-}
-
 impl From<CjsonError> for SignatureError {
     fn from(error: CjsonError) -> Self {
         Self::CanonicalJsonError(error)
     }
 }
+
+#[derive(Error, Debug)]
+pub enum OlmError {
+    #[error("signature verification failed")]
+    Signature(#[from] SignatureError),
+    #[error("failed to read or write to the crypto store {0}")]
+    Store(#[from] CryptoStoreError),
+}

src/crypto/machine.rs

@@ -14,8 +14,9 @@
 
 use std::collections::HashMap;
 use std::convert::TryInto;
+use std::result::Result as StdResult;
 
-use super::error::SignatureError;
+use super::error::{Result, SignatureError, VerificationResult};
 use super::olm::Account;
 use crate::api;
 
@@ -26,6 +27,7 @@ use olm_rs::utility::OlmUtility;
 use serde_json::json;
 use serde_json::Value;
 
+use super::store::CryptoStoreError;
 use ruma_client_api::r0::keys::{
     AlgorithmAndDeviceId, DeviceKeys, KeyAlgorithm, OneTimeKey, SignedKey,
 };
@@ -38,6 +40,9 @@ use ruma_identifiers::{DeviceId, UserId};
 
 pub type OneTimeKeys = HashMap<AlgorithmAndDeviceId, OneTimeKey>;
 
+#[cfg(feature = "sqlite-cryptostore")]
+use super::store::sqlite::SqliteStore;
+
 #[derive(Debug)]
 pub struct OlmMachine {
     /// The unique user id that owns this account.
@@ -60,13 +65,13 @@ impl OlmMachine {
     ];
 
     /// Create a new account.
-    pub fn new(user_id: &UserId, device_id: &str) -> Self {
+    pub fn new(user_id: &UserId, device_id: &str) -> Result<Self> {
-        OlmMachine {
+        Ok(OlmMachine {
             user_id: user_id.clone(),
             device_id: device_id.to_owned(),
             account: Account::new(),
             uploaded_signed_key_count: None,
-        }
+        })
     }
 
     /// Should account or one-time keys be uploaded to the server.
@@ -111,7 +116,7 @@ impl OlmMachine {
     ///
     /// Returns the number of newly generated one-time keys. If no keys can be
     /// generated returns an empty error.
-    fn generate_one_time_keys(&self) -> Result<u64, ()> {
+    fn generate_one_time_keys(&self) -> StdResult<u64, ()> {
         match self.uploaded_signed_key_count {
             Some(count) => {
                 let max_keys = self.account.max_one_time_keys() as u64;
@@ -181,7 +186,7 @@ impl OlmMachine {
     /// Generate, sign and prepare one-time keys to be uploaded.
     ///
     /// If no one-time keys need to be uploaded returns an empty error.
-    fn signed_one_time_keys(&self) -> Result<OneTimeKeys, ()> {
+    fn signed_one_time_keys(&self) -> StdResult<OneTimeKeys, ()> {
         let _ = self.generate_one_time_keys()?;
 
         let one_time_keys = self.account.one_time_keys();
@@ -255,7 +260,7 @@ impl OlmMachine {
         device_id: &str,
         user_key: &str,
         json: &mut Value,
-    ) -> Result<(), SignatureError> {
+    ) -> VerificationResult<()> {
         let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?;
         let unsigned = json_object.remove("unsigned");
         let signatures = json_object.remove("signatures");
@@ -300,7 +305,7 @@ impl OlmMachine {
     /// Get a tuple of device and one-time keys that need to be uploaded.
     ///
     /// Returns an empty error if no keys need to be uploaded.
-    pub fn keys_for_upload(&self) -> Result<(Option<DeviceKeys>, Option<OneTimeKeys>), ()> {
+    pub fn keys_for_upload(&self) -> StdResult<(Option<DeviceKeys>, Option<OneTimeKeys>), ()> {
         if !self.should_upload_keys() {
             return Err(());
         }
@@ -324,7 +329,7 @@ impl OlmMachine {
     /// # Arguments
     ///
     /// * `event` - The to-device event that should be decrypted.
-    fn decrypt_to_device_event(&self, _: &ToDeviceEncrypted) -> Result<ToDeviceEvent, ()> {
+    fn decrypt_to_device_event(&self, _: &ToDeviceEncrypted) -> StdResult<ToDeviceEvent, ()> {
         Err(())
     }
 
@@ -410,13 +415,13 @@ mod test {
 
     #[test]
     fn create_olm_machine() {
-        let machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
         assert!(machine.should_upload_keys());
     }
 
     #[tokio::test]
     async fn receive_keys_upload_response() {
-        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
         let mut response = keys_upload_response();
 
         response
@@ -445,7 +450,7 @@ mod test {
 
     #[tokio::test]
     async fn generate_one_time_keys() {
-        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
 
         let mut response = keys_upload_response();
 
@@ -466,7 +471,7 @@ mod test {
 
     #[test]
     fn test_device_key_signing() {
-        let machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
 
         let mut device_keys = machine.device_keys();
         let identity_keys = machine.account.identity_keys();
@@ -483,7 +488,7 @@ mod test {
 
     #[test]
     fn test_invalid_signature() {
-        let machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
 
         let mut device_keys = machine.device_keys();
 
@@ -498,7 +503,7 @@ mod test {
 
     #[test]
     fn test_one_time_key_signing() {
-        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
         machine.uploaded_signed_key_count = Some(49);
 
         let mut one_time_keys = machine.signed_one_time_keys().unwrap();
@@ -518,7 +523,7 @@ mod test {
 
     #[tokio::test]
     async fn test_keys_for_upload() {
-        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID);
+        let mut machine = OlmMachine::new(&user_id(), DEVICE_ID).unwrap();
         machine.uploaded_signed_key_count = Some(0);
 
         let identity_keys = machine.account.identity_keys();

src/crypto/mod.rs

@@ -20,4 +20,5 @@ mod machine;
 mod olm;
 mod store;
 
+pub use error::OlmError;
 pub use machine::{OlmMachine, OneTimeKeys};

src/crypto/store/mod.rs

@@ -19,7 +19,7 @@ use sqlx::Error as SqlxError;
 pub enum CryptoStoreError {
     #[error("can't read or write from the store")]
     Io(#[from] IoError),
-    #[error("Olm operation failed")]
+    #[error("can't finish Olm account operation {0}")]
     OlmAccountError(#[from] OlmAccountError),
     #[error("URL can't be parsed")]
     UrlParse(#[from] ParseError),

src/error.rs

@@ -15,69 +15,50 @@
 
 //! Error conditions.
 
-use std::error::Error as StdError;
-use std::fmt::{Display, Formatter, Result as FmtResult};
-
 use reqwest::Error as ReqwestError;
 use ruma_api::error::FromHttpResponseError as RumaResponseError;
 use ruma_api::error::IntoHttpError as RumaIntoHttpError;
+use thiserror::Error;
 use url::ParseError;
 
-/// An error that can occur during client operations.
+#[cfg(feature = "encryption")]
-#[derive(Debug)]
+use crate::crypto::OlmError;
-pub struct Error(pub(crate) InnerError);
 
-impl Display for Error {
+/// Result type of the rust-sdk.
-    fn fmt(&self, f: &mut Formatter) -> FmtResult {
+pub type Result<T> = std::result::Result<T, Error>;
-        let message = match self.0 {
-            InnerError::AuthenticationRequired => "The queried endpoint requires authentication but was called with an anonymous client.",
-            InnerError::Reqwest(_) => "An HTTP error occurred.",
-            InnerError::Uri(_) => "Provided string could not be converted into a URI.",
-            InnerError::RumaResponseError(_) => "An error occurred converting between ruma_client_api and hyper types.",
-            InnerError::IntoHttpError(_) => "An error occurred converting between ruma_client_api and hyper types.",
-        };
-
-        write!(f, "{}", message)
-    }
-}
-
-impl StdError for Error {}
 
 /// Internal representation of errors.
-#[derive(Debug)]
+#[derive(Error, Debug)]
-pub(crate) enum InnerError {
+pub enum Error {
     /// Queried endpoint requires authentication but was called on an anonymous client.
+    #[error("the queried endpoint requires authentication but was called before logging in")]
     AuthenticationRequired,
     /// An error at the HTTP layer.
-    Reqwest(ReqwestError),
+    #[error(transparent)]
+    Reqwest(#[from] ReqwestError),
     /// An error when parsing a string as a URI.
-    Uri(ParseError),
+    #[error("can't parse the provided string as an URL")]
+    Uri(#[from] ParseError),
     /// An error converting between ruma_client_api types and Hyper types.
-    RumaResponseError(RumaResponseError),
+    #[error("can't parse the JSON response as a Matrix response")]
+    RumaResponse(RumaResponseError),
     /// An error converting between ruma_client_api types and Hyper types.
-    IntoHttpError(RumaIntoHttpError),
+    #[error("can't convert between ruma_client_api and hyper types.")]
-}
+    IntoHttp(RumaIntoHttpError),
-
+    #[cfg(feature = "encryption")]
-impl From<ParseError> for Error {
+    /// An error occured durring a E2EE operation.
-    fn from(error: ParseError) -> Self {
+    #[error(transparent)]
-        Self(InnerError::Uri(error))
+    OlmError(#[from] OlmError),
-    }
 }
 
 impl From<RumaResponseError> for Error {
     fn from(error: RumaResponseError) -> Self {
-        Self(InnerError::RumaResponseError(error))
+        Self::RumaResponse(error)
     }
 }
 
 impl From<RumaIntoHttpError> for Error {
     fn from(error: RumaIntoHttpError) -> Self {
-        Self(InnerError::IntoHttpError(error))
+        Self::IntoHttp(error)
-    }
-}
-
-impl From<ReqwestError> for Error {
-    fn from(error: ReqwestError) -> Self {
-        Self(InnerError::Reqwest(error))
     }
 }

src/lib.rs

@@ -16,7 +16,7 @@
 //! This crate implements a [Matrix](https://matrix.org/) client library.
 #![deny(missing_docs)]
 
-pub use crate::{error::Error, session::Session};
+pub use crate::{error::Error, error::Result, session::Session};
 pub use reqwest::header::InvalidHeaderValue;
 pub use ruma_client_api as api;
 pub use ruma_events as events;