From 6e4a57046eb0a454cd25aae6f63fd615cc8bcffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 9 Sep 2021 10:30:46 +0200 Subject: [PATCH] fix(sdk): Use a pure HTTP error for methods that don't touch local state --- matrix_sdk/src/client.rs | 41 ++++++++++++++---------------- matrix_sdk/src/error.rs | 27 ++++++++++++++++++++ matrix_sdk/src/lib.rs | 2 +- matrix_sdk/src/room/common.rs | 3 ++- matrix_sdk/src/room/joined.rs | 14 +++++----- matrix_sdk_appservice/src/error.rs | 6 +++++ matrix_sdk_appservice/src/lib.rs | 6 ++--- 7 files changed, 65 insertions(+), 34 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index ad5abc1e..d250e8fe 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -145,7 +145,7 @@ use crate::{ verification::{QrVerification, SasVerification, Verification, VerificationRequest}, }; use crate::{ - error::HttpError, + error::{HttpError, HttpResult}, event_handler::{EventHandler, EventHandlerData, EventHandlerResult, EventKind, SyncEvent}, http_client::{client_with_config, HttpClient, HttpSend}, room, Error, Result, @@ -645,7 +645,7 @@ impl Client { Ok(result) } - async fn discover_homeserver(&self) -> Result { + async fn discover_homeserver(&self) -> HttpResult { self.send(discover_homeserver::Request::new(), Some(RequestConfig::new().disable_retry())) .await } @@ -660,7 +660,7 @@ impl Client { *homeserver = homeserver_url; } - async fn get_supported_versions(&self) -> Result { + async fn get_supported_versions(&self) -> HttpResult { self.send( get_supported_versions::Request::new(), Some(RequestConfig::new().disable_retry()), @@ -1123,7 +1123,7 @@ impl Client { /// /// This should be the first step when trying to login so you can call the /// appropriate method for the next step. - pub async fn get_login_types(&self) -> Result { + pub async fn get_login_types(&self) -> HttpResult { let request = get_login_types::Request::new(); self.send(request, None).await } @@ -1540,7 +1540,7 @@ impl Client { pub async fn register( &self, registration: impl Into>, - ) -> Result { + ) -> HttpResult { info!("Registering to {}", self.homeserver().await); let config = if self.appservice_mode { @@ -1630,7 +1630,7 @@ impl Client { /// # Arguments /// /// * `room_id` - The `RoomId` of the room to be joined. - pub async fn join_room_by_id(&self, room_id: &RoomId) -> Result { + pub async fn join_room_by_id(&self, room_id: &RoomId) -> HttpResult { let request = join_room_by_id::Request::new(room_id); self.send(request, None).await } @@ -1648,7 +1648,7 @@ impl Client { &self, alias: &RoomIdOrAliasId, server_names: &[Box], - ) -> Result { + ) -> HttpResult { let request = assign!(join_room_by_id_or_alias::Request::new(alias), { server_name: server_names, }); @@ -1691,7 +1691,7 @@ impl Client { limit: Option, since: Option<&str>, server: Option<&ServerName>, - ) -> Result { + ) -> HttpResult { let limit = limit.map(UInt::from); let request = assign!(get_public_rooms::Request::new(), { @@ -1732,7 +1732,7 @@ impl Client { pub async fn create_room( &self, room: impl Into>, - ) -> Result { + ) -> HttpResult { let request = room.into(); self.send(request, None).await } @@ -1772,7 +1772,7 @@ impl Client { pub async fn public_rooms_filtered( &self, room_search: impl Into>, - ) -> Result { + ) -> HttpResult { let request = room_search.into(); self.send(request, None).await } @@ -1906,7 +1906,7 @@ impl Client { let txn_id = txn_id.unwrap_or_else(Uuid::new_v4).to_string(); let request = send_message_event::Request::new(room_id, &txn_id, &content); - self.send(request, None).await + Ok(self.send(request, None).await?) } } @@ -1954,7 +1954,7 @@ impl Client { &self, request: Request, config: Option, - ) -> Result + ) -> HttpResult where Request: OutgoingRequest + Debug, HttpError: From>, @@ -1966,8 +1966,9 @@ impl Client { pub(crate) async fn send_to_device( &self, request: &ToDeviceRequest, - ) -> Result { + ) -> HttpResult { let txn_id_string = request.txn_id_string(); + let request = RumaToDeviceRequest::new_raw( request.event_type.as_str(), &txn_id_string, @@ -2000,7 +2001,7 @@ impl Client { /// } /// # }); /// ``` - pub async fn devices(&self) -> Result { + pub async fn devices(&self) -> HttpResult { let request = get_devices::Request::new(); self.send(request, None).await @@ -2057,7 +2058,7 @@ impl Client { &self, devices: &[DeviceIdBox], auth_data: Option>, - ) -> Result { + ) -> HttpResult { let mut request = delete_devices::Request::new(devices); request.auth = auth_data; @@ -2965,7 +2966,7 @@ impl Client { } /// Gets information about the owner of a given access token. - pub async fn whoami(&self) -> Result { + pub async fn whoami(&self) -> HttpResult { let request = whoami::Request::new(); self.send(request, None).await } @@ -3414,12 +3415,8 @@ mod test { }); if let Err(err) = client.register(user).await { - if let crate::Error::Http(HttpError::UiaaError(FromHttpResponseError::Http( - ServerError::Known(UiaaResponse::MatrixError(client_api::Error { - kind, - message, - status_code, - })), + if let HttpError::UiaaError(FromHttpResponseError::Http(ServerError::Known( + UiaaResponse::MatrixError(client_api::Error { kind, message, status_code }), ))) = err { if let client_api::error::ErrorKind::Forbidden = kind { diff --git a/matrix_sdk/src/error.rs b/matrix_sdk/src/error.rs index 6ebb9716..3a794008 100644 --- a/matrix_sdk/src/error.rs +++ b/matrix_sdk/src/error.rs @@ -40,6 +40,9 @@ use url::ParseError as UrlParseError; /// Result type of the rust-sdk. pub type Result = std::result::Result; +/// Result type of a pure HTTP request. +pub type HttpResult = std::result::Result; + /// An HTTP error, representing either a connection error or an error while /// converting the raw HTTP response into a Matrix response. #[derive(Error, Debug)] @@ -182,6 +185,30 @@ pub enum RoomKeyImportError { Export(#[from] KeyExportError), } +impl HttpError { + /// Try to destructure the error into an universal interactive auth info. + /// + /// Some requests require universal interactive auth, doing such a request + /// will always fail the first time with a 401 status code, the response + /// body will contain info how the client can authenticate. + /// + /// The request will need to be retried, this time containing additional + /// authentication data. + /// + /// This method is an convenience method to get to the info the server + /// returned on the first, failed request. + pub fn uiaa_response(&self) -> Option<&UiaaInfo> { + if let HttpError::UiaaError(FromHttpResponseError::Http(ServerError::Known( + UiaaError::AuthResponse(i), + ))) = self + { + Some(i) + } else { + None + } + } +} + impl Error { /// Try to destructure the error into an universal interactive auth info. /// diff --git a/matrix_sdk/src/lib.rs b/matrix_sdk/src/lib.rs index 92dffbe1..47cf6661 100644 --- a/matrix_sdk/src/lib.rs +++ b/matrix_sdk/src/lib.rs @@ -107,7 +107,7 @@ pub use client::{Client, ClientConfig, LoopCtrl, RequestConfig, SyncSettings}; #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] pub use device::Device; -pub use error::{Error, HttpError, Result}; +pub use error::{Error, HttpError, HttpResult, Result}; pub use http_client::HttpSend; pub use room_member::RoomMember; #[cfg(not(target_arch = "wasm32"))] diff --git a/matrix_sdk/src/room/common.rs b/matrix_sdk/src/room/common.rs index 859e2ae2..091074e1 100644 --- a/matrix_sdk/src/room/common.rs +++ b/matrix_sdk/src/room/common.rs @@ -14,6 +14,7 @@ use ruma::{ }; use crate::{ + error::HttpResult, media::{MediaFormat, MediaRequest, MediaType}, room::RoomType, BaseRoom, Client, Result, RoomMember, @@ -143,7 +144,7 @@ impl Common { pub async fn messages( &self, request: impl Into>, - ) -> Result { + ) -> HttpResult { let request = request.into(); self.client.send(request, None).await } diff --git a/matrix_sdk/src/room/joined.rs b/matrix_sdk/src/room/joined.rs index 5de22dc6..048ba7a3 100644 --- a/matrix_sdk/src/room/joined.rs +++ b/matrix_sdk/src/room/joined.rs @@ -46,7 +46,7 @@ use ruma::{ #[cfg(feature = "encryption")] use tracing::instrument; -use crate::{room::Common, BaseRoom, Client, Error, Result, RoomType}; +use crate::{error::HttpResult, room::Common, BaseRoom, Client, HttpError, Result, RoomType}; const TYPING_NOTICE_TIMEOUT: Duration = Duration::from_secs(4); const TYPING_NOTICE_RESEND_TIMEOUT: Duration = Duration::from_secs(3); @@ -562,7 +562,7 @@ impl Joined { &self, content: impl Into, state_key: &str, - ) -> Result { + ) -> HttpResult { let content = content.into(); let request = send_state_event::Request::new(self.inner.room_id(), state_key, &content); @@ -606,7 +606,7 @@ impl Joined { event_id: &EventId, reason: Option<&str>, txn_id: Option, - ) -> Result { + ) -> HttpResult { let txn_id = txn_id.unwrap_or_else(Uuid::new_v4).to_string(); let request = assign!(redact_event::Request::new(self.inner.room_id(), event_id, &txn_id), { @@ -642,8 +642,8 @@ impl Joined { /// room.set_tag("u.work", tag_info ); /// # }) /// ``` - pub async fn set_tag(&self, tag: &str, tag_info: TagInfo) -> Result { - let user_id = self.client.user_id().await.ok_or(Error::AuthenticationRequired)?; + pub async fn set_tag(&self, tag: &str, tag_info: TagInfo) -> HttpResult { + let user_id = self.client.user_id().await.ok_or(HttpError::AuthenticationRequired)?; let request = create_tag::Request::new(&user_id, self.inner.room_id(), tag, tag_info); self.client.send(request, None).await } @@ -654,8 +654,8 @@ impl Joined { /// /// # Arguments /// * `tag` - The tag to remove. - pub async fn remove_tag(&self, tag: &str) -> Result { - let user_id = self.client.user_id().await.ok_or(Error::AuthenticationRequired)?; + pub async fn remove_tag(&self, tag: &str) -> HttpResult { + let user_id = self.client.user_id().await.ok_or(HttpError::AuthenticationRequired)?; let request = delete_tag::Request::new(&user_id, self.inner.room_id(), tag); self.client.send(request, None).await } diff --git a/matrix_sdk_appservice/src/error.rs b/matrix_sdk_appservice/src/error.rs index 1a65a66b..9c0fda4c 100644 --- a/matrix_sdk_appservice/src/error.rs +++ b/matrix_sdk_appservice/src/error.rs @@ -87,3 +87,9 @@ impl From for Error { Self::WarpRejection(format!("{:?}", rejection)) } } + +impl From for Error { + fn from(e: matrix_sdk::HttpError) -> Self { + matrix_sdk::Error::from(e).into() + } +} diff --git a/matrix_sdk_appservice/src/lib.rs b/matrix_sdk_appservice/src/lib.rs index 037d292e..46269c14 100644 --- a/matrix_sdk_appservice/src/lib.rs +++ b/matrix_sdk_appservice/src/lib.rs @@ -94,7 +94,7 @@ use matrix_sdk::{ bytes::Bytes, event_handler::{EventHandler, EventHandlerResult, SyncEvent}, reqwest::Url, - Client, ClientConfig, HttpError, Session, + Client, ClientConfig, Session, }; use regex::Regex; use ruma::{ @@ -402,9 +402,9 @@ impl AppService { match client.register(request).await { Ok(_) => (), Err(error) => match error { - matrix_sdk::Error::Http(HttpError::UiaaError(FromHttpResponseError::Http( + matrix_sdk::HttpError::UiaaError(FromHttpResponseError::Http( ServerError::Known(UiaaResponse::MatrixError(ref matrix_error)), - ))) => { + )) => { match matrix_error.kind { ErrorKind::UserInUse => { // TODO: persist the fact that we registered that user