refactor(sdk): Some error refactoring

crates/matrix-sdk/changelog.d/6574.changed.md

@@ -0,0 +1,3 @@
+[**breaking**] `RumaApiError` is now a type alias for `UiaaResponse`, because they have similar
+variants containing the same data. The `ClientApi` variant is now `MatrixError`, and the `Uiaa`
+variant is `AuthResponse`.

crates/matrix-sdk/src/authentication/oauth/mod.rs

@@ -225,7 +225,7 @@ use self::{
 };
 use super::{AuthData, SessionTokens};
 use crate::{
-    Client, HttpError, RefreshTokenError, Result,
+    Client, RefreshTokenError, Result,
     client::{SessionChange, caches::CachedValue},
     executor::spawn,
     utils::UrlOrQuery,
@@ -521,17 +521,11 @@ impl OAuth {
     async fn server_metadata_inner(
         &self,
     ) -> Result<AuthorizationServerMetadata, OAuthDiscoveryError> {
-        let is_endpoint_unsupported = |error: &HttpError| {
-            error
-                .as_client_api_error()
-                .is_some_and(|err| err.status_code == http::StatusCode::NOT_FOUND)
-        };
-
         let response =
             self.client.send(get_authorization_server_metadata::v1::Request::new()).await.map_err(
                 |error| {
-                    // If the endpoint returns a 404, i.e. the server doesn't support the endpoint.
-                    if is_endpoint_unsupported(&error) {
+                    // If the server doesn't support the endpoint.
+                    if error.is_endpoint_not_implemented() {
                         OAuthDiscoveryError::NotSupported
                     } else {
                         error.into()

crates/matrix-sdk/src/authentication/oauth/qrcode/mod.rs

@@ -33,7 +33,7 @@ pub use oauth2::{
     RequestTokenError, StandardErrorResponse,
     basic::{BasicErrorResponse, BasicRequestTokenError},
 };
-use ruma::api::error::{ErrorKind, FromHttpResponseError};
+use ruma::api::error::ErrorKind;
 use thiserror::Error;
 use tokio::sync::Mutex;
 use url::Url;
@@ -125,11 +125,8 @@ pub enum QRCodeLoginError {
 impl From<SecureChannelError> for QRCodeLoginError {
     fn from(e: SecureChannelError) -> Self {
         match e {
-            SecureChannelError::RendezvousChannel(HttpError::Api(ref boxed)) => {
-                if let FromHttpResponseError::Server(api_error) = boxed.as_ref()
-                    && let Some(ErrorKind::NotFound) =
-                        api_error.as_client_api_error().and_then(|e| e.error_kind())
-                {
+            SecureChannelError::RendezvousChannel(ref http_error) => {
+                if let Some(ErrorKind::NotFound) = http_error.client_api_error_kind() {
                     return Self::NotFound;
                 }
                 Self::SecureChannel(e)
@@ -195,11 +192,8 @@ pub enum QRCodeGrantLoginError {
 impl From<SecureChannelError> for QRCodeGrantLoginError {
     fn from(e: SecureChannelError) -> Self {
         match e {
-            SecureChannelError::RendezvousChannel(HttpError::Api(ref boxed)) => {
-                if let FromHttpResponseError::Server(api_error) = boxed.as_ref()
-                    && let Some(ErrorKind::NotFound) =
-                        api_error.as_client_api_error().and_then(|e| e.error_kind())
-                {
+            SecureChannelError::RendezvousChannel(ref http_error) => {
+                if let Some(ErrorKind::NotFound) = http_error.client_api_error_kind() {
                     return Self::NotFound;
                 }
                 Self::SecureChannel(e)

crates/matrix-sdk/src/authentication/oauth/qrcode/rendezvous_channel/msc_4108.rs

@@ -55,7 +55,7 @@ fn get_header(
 fn response_to_error(status: StatusCode, body: Vec<u8>) -> HttpError {
     match http::Response::builder().status(status).body(body).map_err(IntoHttpError::from) {
         Ok(response) => {
-            let error = FromHttpResponseError::<RumaApiError>::Server(RumaApiError::ClientApi(
+            let error = FromHttpResponseError::<RumaApiError>::Server(RumaApiError::MatrixError(
                 ruma::api::error::Error::from_http_response(response),
             ));
 

crates/matrix-sdk/src/authentication/oauth/tests.rs

@@ -762,15 +762,23 @@ async fn test_server_metadata_cache_refresh_lock() {
 #[async_test]
 async fn test_server_metadata() {
     let server = MatrixMockServer::new().await;
+    let oauth_server = server.oauth();
+
     let client = server.client_builder().unlogged().build().await;
     let oauth = client.oauth();
 
-    // The endpoint is not mocked so it is not supported.
+    // The endpoint is not supported.
+    oauth_server
+        .mock_server_metadata()
+        .error_unrecognized()
+        .mock_once()
+        .named("unrecognized auth metadata")
+        .mount()
+        .await;
     let error = oauth.server_metadata().await.unwrap_err();
     assert!(error.is_not_supported());
 
     // Mock the `GET /auth_metadata` endpoint.
-    let oauth_server = server.oauth();
     oauth_server.mock_server_metadata().ok().expect(1).named("auth_metadata").mount().await;
 
     oauth.server_metadata().await.unwrap();

crates/matrix-sdk/src/encryption/mod.rs

@@ -97,7 +97,7 @@ use self::{
     verification::{SasVerification, Verification, VerificationRequest},
 };
 use crate::{
-    Client, Error, HttpError, Result, Room, RumaApiError, TransmissionProgress,
+    Client, Error, HttpError, Result, Room, TransmissionProgress,
     attachment::Thumbnail,
     client::{ClientInner, WeakClient},
     cross_process_lock::CrossProcessLockGuard,
@@ -753,8 +753,8 @@ impl Client {
                 let response = self.keys_upload(r.request_id(), request).await;
 
                 if let Err(e) = &response {
-                    match e.as_ruma_api_error() {
-                        Some(RumaApiError::ClientApi(e)) if e.status_code == 400 => {
+                    match e.as_client_api_error() {
+                        Some(e) if e.status_code == 400 => {
                             if let ErrorBody::Standard(StandardErrorBody { message, .. }) = &e.body
                             {
                                 // This is one of the nastiest errors we can have. The server

crates/matrix-sdk/src/error.rs

@@ -59,30 +59,7 @@ pub type HttpResult<T> = std::result::Result<T, HttpError>;
 
 /// An error response from a Matrix API call, using a client API specific
 /// representation if the endpoint is from that.
-#[derive(Error, Debug)]
-pub enum RumaApiError {
-    /// A client API response error.
-    #[error(transparent)]
-    ClientApi(ruma::api::error::Error),
-
-    /// A user-interactive authentication API error.
-    ///
-    /// When registering or authenticating, the Matrix server can send a
-    /// `UiaaInfo` as the error type, this is a User-Interactive Authentication
-    /// API response. This represents an error with information about how to
-    /// authenticate the user.
-    #[error("User-Interactive Authentication required.")]
-    Uiaa(UiaaInfo),
-}
-
-impl RumaApiError {
-    /// If `self` is `ClientApi(e)`, returns `Some(e)`.
-    ///
-    /// Otherwise, returns `None`.
-    pub fn as_client_api_error(&self) -> Option<&ruma::api::error::Error> {
-        as_variant!(self, Self::ClientApi)
-    }
-}
+pub type RumaApiError = UiaaResponse;
 
 /// An HTTP error, representing either a connection error or an error while
 /// converting the raw HTTP response into a Matrix response.
@@ -134,35 +111,35 @@ impl HttpError {
             _ => None
         }
     }
-
-    /// Shorthand for
-    /// <code>.[as_ruma_api_error](Self::as_ruma_api_error)().[and_then](Option::and_then)([RumaApiError::as_client_api_error])</code>.
-    pub fn as_client_api_error(&self) -> Option<&ruma::api::error::Error> {
-        self.as_ruma_api_error().and_then(RumaApiError::as_client_api_error)
-    }
 }
 
 // Another impl block that's formatted with rustfmt.
 impl HttpError {
     /// If `self` is a server error in the `errcode` + `error` format expected
-    /// for client-API endpoints, returns the error kind (`errcode`).
+    /// for client API endpoints, returns it.
+    pub fn as_client_api_error(&self) -> Option<&ruma::api::error::Error> {
+        self.as_ruma_api_error().and_then(as_variant!(UiaaResponse::MatrixError))
+    }
+
+    /// If `self` is a server error in the `errcode` + `error` format expected
+    /// for client API endpoints, returns the error kind (`errcode`).
     pub fn client_api_error_kind(&self) -> Option<&ErrorKind> {
         self.as_client_api_error().and_then(ruma::api::error::Error::error_kind)
     }
 
-    /// Try to destructure the error into an universal interactive auth info.
+    /// Try to destructure the error into a user-interactive auth info.
     ///
-    /// Some requests require universal interactive auth, doing such a request
+    /// Some requests require user-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.
+    /// body will contain info on 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
+    /// This method is a convenience method to get to the info the server
     /// returned on the first, failed request.
     pub fn as_uiaa_response(&self) -> Option<&UiaaInfo> {
-        self.as_ruma_api_error().and_then(as_variant!(RumaApiError::Uiaa))
+        self.as_ruma_api_error().and_then(as_variant!(UiaaResponse::AuthResponse))
     }
 
     /// Returns whether an HTTP error response should be qualified as transient
@@ -180,6 +157,12 @@ impl HttpError {
             _ => RetryKind::Permanent,
         }
     }
+
+    /// If this is a server error, returns whether it matches the expected
+    /// format for an endpoint that is not implemented.
+    pub fn is_endpoint_not_implemented(&self) -> bool {
+        self.as_client_api_error().is_some_and(|error| error.is_endpoint_not_implemented())
+    }
 }
 
 impl From<FromHttpResponseError<RumaApiError>> for HttpError {
@@ -215,16 +198,16 @@ impl RetryKind {
     /// format defined in the [spec].
     ///
     /// [spec]: https://spec.matrix.org/v1.11/client-server-api/#standard-error-response
-    fn from_api_error(api_error: &RumaApiError) -> Self {
+    fn from_api_error(api_error: &UiaaResponse) -> Self {
         match api_error {
-            RumaApiError::ClientApi(client_error) => match client_error.error_kind() {
+            UiaaResponse::MatrixError(client_error) => match client_error.error_kind() {
                 Some(ErrorKind::LimitExceeded(limit_exceeded)) => {
                     RetryKind::from_retry_after(limit_exceeded.retry_after.as_ref())
                 }
                 Some(ErrorKind::Unrecognized) => RetryKind::Permanent,
                 _ => RetryKind::from_status_code(client_error.status_code),
             },
-            RumaApiError::Uiaa(_) => RetryKind::Permanent,
+            UiaaResponse::AuthResponse(_) => RetryKind::Permanent,
         }
     }
 
@@ -434,14 +417,14 @@ impl Error {
         as_variant!(self, Self::Http).and_then(|e| e.as_ruma_api_error())
     }
 
-    /// Shorthand for
-    /// <code>.[as_ruma_api_error](Self::as_ruma_api_error)().[and_then](Option::and_then)([RumaApiError::as_client_api_error])</code>.
+    /// If `self` is a server error in the `errcode` + `error` format expected
+    /// for client API endpoints, returns it.
     pub fn as_client_api_error(&self) -> Option<&ruma::api::error::Error> {
-        self.as_ruma_api_error().and_then(RumaApiError::as_client_api_error)
+        self.as_ruma_api_error().and_then(as_variant!(UiaaResponse::MatrixError))
     }
 
     /// If `self` is a server error in the `errcode` + `error` format expected
-    /// for client-API endpoints, returns the error kind (`errcode`).
+    /// for client API endpoints, returns the error kind (`errcode`).
     pub fn client_api_error_kind(&self) -> Option<&ErrorKind> {
         self.as_client_api_error().and_then(ruma::api::error::Error::error_kind)
     }
@@ -458,7 +441,7 @@ impl Error {
     /// This method is an convenience method to get to the info the server
     /// returned on the first, failed request.
     pub fn as_uiaa_response(&self) -> Option<&UiaaInfo> {
-        self.as_ruma_api_error().and_then(as_variant!(RumaApiError::Uiaa))
+        self.as_ruma_api_error().and_then(as_variant!(UiaaResponse::AuthResponse))
     }
 }
 
@@ -580,16 +563,7 @@ pub enum RoomKeyImportError {
 
 impl From<FromHttpResponseError<ruma::api::error::Error>> for HttpError {
     fn from(err: FromHttpResponseError<ruma::api::error::Error>) -> Self {
-        Self::Api(Box::new(err.map(RumaApiError::ClientApi)))
-    }
-}
-
-impl From<FromHttpResponseError<UiaaResponse>> for HttpError {
-    fn from(err: FromHttpResponseError<UiaaResponse>) -> Self {
-        Self::Api(Box::new(err.map(|e| match e {
-            UiaaResponse::AuthResponse(i) => RumaApiError::Uiaa(i),
-            UiaaResponse::MatrixError(e) => RumaApiError::ClientApi(e),
-        })))
+        Self::Api(Box::new(err.map(Into::into)))
     }
 }
 

crates/matrix-sdk/src/room/shared_room_history.rs

@@ -263,11 +263,7 @@ pub(crate) async fn maybe_accept_key_bundle(room: &Room, inviter: &UserId) -> Re
         Err(err) => {
             // If we encountered an HTTP client error, we should check the status code to
             // see if we have been sent a bogus link.
-            let Some(err) = err
-                .as_ruma_api_error()
-                .and_then(|e| e.as_client_api_error())
-                .and_then(|e| e.error_kind())
-            else {
+            let Some(err) = err.client_api_error_kind() else {
                 // Some other error occurred, which we may be able to recover from at the next
                 // client startup.
                 return Ok(());

crates/matrix-sdk/src/widget/machine/from_widget.rs

@@ -71,7 +71,7 @@ impl FromWidgetErrorResponse {
         let message = error.to_string();
         let matrix_api_error = match error {
             HttpError::Api(error) => {
-                as_variant!(*error, ruma::api::error::FromHttpResponseError::Server(RumaApiError::ClientApi(err)) => err)
+                as_variant!(*error, ruma::api::error::FromHttpResponseError::Server(RumaApiError::MatrixError(err)) => err)
             }
             _ => None,
         };

crates/matrix-sdk/tests/integration/matrix_auth.rs

@@ -2,7 +2,7 @@ use std::{collections::BTreeMap, sync::Mutex};
 
 use assert_matches::assert_matches;
 use matrix_sdk::{
-    AuthApi, AuthSession, Client, RumaApiError, SessionTokens,
+    AuthApi, AuthSession, Client, SessionTokens,
     authentication::matrix::MatrixSession,
     config::RequestConfig,
     test_utils::{logged_in_client_with_server, no_retry_test_client_with_server},
@@ -233,7 +233,7 @@ async fn test_login_error() {
         .await;
 
     if let Err(err) = client.matrix_auth().login_username("example", "wordpass").send().await {
-        if let Some(RumaApiError::ClientApi(api_err)) = err.as_ruma_api_error() {
+        if let Some(api_err) = err.as_client_api_error() {
             assert_eq!(api_err.status_code, http::StatusCode::from_u16(403).unwrap());
 
             if let api::error::ErrorBody::Standard(StandardErrorBody { kind, message, .. }) =