review

toger5 · toger5 · commit 69b980fc5842 · 2024-11-26T18:18:32.000+01:00
diff --git a/crates/matrix-sdk/src/widget/machine/driver_req.rs b/crates/matrix-sdk/src/widget/machine/driver_req.rs
@@ -74,6 +74,8 @@ where
         Self { request_meta: None, _phantom: PhantomData }
     }
 
+    /// Setup a callback function that will be called once the matrix driver has
+    /// processed the request.
     pub(crate) fn then(
         self,
         response_handler: impl FnOnce(Result<T, crate::Error>, &mut WidgetMachine) -> Vec<Action>
diff --git a/crates/matrix-sdk/src/widget/machine/from_widget.rs b/crates/matrix-sdk/src/widget/machine/from_widget.rs
@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::collections::HashMap;
-
 use ruma::{
     api::client::{
         delayed_events::{delayed_message_event, delayed_state_event, update_delayed_event},
@@ -43,22 +41,23 @@ pub(super) enum FromWidgetRequest {
     DelayedEventUpdate(UpdateDelayedEventRequest),
 }
 
+/// The full response a client sends to a from widget request
+/// in case of an error.
 #[derive(Serialize)]
 pub(super) struct FromWidgetErrorResponse {
     error: FromWidgetError,
 }
 
-impl From<&HttpError> for FromWidgetErrorResponse {
-    fn from(error: &HttpError) -> Self {
+impl FromWidgetErrorResponse {
+    /// Create a error response to send to the widget from an http error.
+    pub(crate) fn from_http_error(error: &HttpError) -> Self {
         Self {
             error: FromWidgetError {
                 message: error.to_string(),
                 matrix_api_error: error.as_client_api_error().and_then(
                     |api_error| match &api_error.body {
                         ErrorBody::Standard { kind, message } => Some(FromWidgetMatrixErrorBody {
                             http_status: api_error.status_code.as_u16().into(),
-                            http_headers: HashMap::new(),
-                            url: "".to_owned(),
                             response: json!({"errcode": kind.to_string(), "error": message }),
                         }),
                         _ => None,
@@ -67,50 +66,55 @@ impl From<&HttpError> for FromWidgetErrorResponse {
             },
         }
     }
-}
 
-impl From<&Error> for FromWidgetErrorResponse {
-    fn from(error: &Error) -> Self {
+    /// Create a error response to send to the widget from a matrix sdk error.
+    pub(crate) fn from_error(error: &Error) -> Self {
         match &error {
-            Error::Http(e) => e.into(),
-            Error::UnknownError(e) => e.to_string().into(),
-            _ => error.to_string().into(),
+            Error::Http(e) => FromWidgetErrorResponse::from_http_error(e),
+            Error::UnknownError(e) => FromWidgetErrorResponse::from_string(e.to_string()),
+            _ => FromWidgetErrorResponse::from_string(error.to_string()),
         }
     }
-}
 
-impl From<String> for FromWidgetErrorResponse {
-    fn from(error: String) -> Self {
-        Self { error: FromWidgetError { message: error, matrix_api_error: None } }
-    }
-}
-
-impl From<&str> for FromWidgetErrorResponse {
-    fn from(error: &str) -> Self {
-        error.to_owned().into()
+    /// Create a error response to send to the widget from a string.
+    pub(crate) fn from_string<S: Into<String>>(error: S) -> Self {
+        Self { error: FromWidgetError { message: error.into(), matrix_api_error: None } }
     }
 }
 
+/// The serializable section of an error response send by the client as a
+/// response to a from widget request.
 #[derive(Serialize)]
 struct FromWidgetError {
+    /// A unspecified error message text that caused this widget action to fail.
+    /// This is useful to prompt the user on an issue but cannot be used to
+    /// decide on how to deal with the error.
     message: String,
+    /// An optional matrix error that contains specified
+    /// information and helps finding a work around for specific errors.
     matrix_api_error: Option<FromWidgetMatrixErrorBody>,
 }
 
+/// The serializable section of a widget response that represents a matrix
+/// error.
 #[derive(Serialize)]
 struct FromWidgetMatrixErrorBody {
+    /// The status code of the http response
     http_status: u32,
-    // TODO: figure out the which type to use here.
-    http_headers: HashMap<String, String>,
-    url: String,
+    /// The matrix standard error response including the `errorcode` and the
+    /// `error` message as defined in the spec: https://spec.matrix.org/v1.12/client-server-api/#standard-error-response
     response: Value,
 }
+
+/// The serializable section of a widget response containing the supported
+/// versions.
 #[derive(Serialize)]
 pub(super) struct SupportedApiVersionsResponse {
     supported_versions: Vec<ApiVersion>,
 }
 
 impl SupportedApiVersionsResponse {
+    /// The currently supported widget api versions from the rust widget driver.
     pub(super) fn new() -> Self {
         Self {
             supported_versions: vec![
diff --git a/crates/matrix-sdk/src/widget/machine/incoming.rs b/crates/matrix-sdk/src/widget/machine/incoming.rs
@@ -37,7 +37,9 @@ pub(crate) enum IncomingMessage {
         /// The ID of the request that this response corresponds to.
         request_id: Uuid,
 
-        /// The result of the request: response data or error message.
+        /// The result of the request: response data or matrix sdk error.
+        /// Http errors will be forwarded to the widget in a specified format
+        /// so the widget can parse the error.
         response: Result<MatrixDriverResponse, crate::Error>,
     },
 
diff --git a/crates/matrix-sdk/src/widget/machine/mod.rs b/crates/matrix-sdk/src/widget/machine/mod.rs
@@ -54,7 +54,7 @@ use super::{
     filter::{MatrixEventContent, MatrixEventFilterInput},
     Capabilities, StateKeySelector,
 };
-use crate::{widget::EventFilter, Error};
+use crate::{widget::EventFilter, Result};
 
 mod driver_req;
 mod from_widget;
@@ -204,8 +204,10 @@ impl WidgetMachine {
         let request = match raw_request.deserialize() {
             Ok(r) => r,
             Err(e) => {
-                return vec![self
-                    .send_from_widget_error_response(raw_request, (&Error::SerdeJson(e)).into())]
+                return vec![self.send_from_widget_error_response(
+                    raw_request,
+                    FromWidgetErrorResponse::from_error(&crate::Error::SerdeJson(e)),
+                )]
             }
         };
 
@@ -255,13 +257,17 @@ impl WidgetMachine {
                 let CapabilitiesState::Negotiated(capabilities) = &self.capabilities else {
                     let text =
                         "Received send update delayed event request before capabilities were negotiated";
-                    return vec![self.send_from_widget_error_response(raw_request, text.into())];
+                    return vec![self.send_from_widget_error_response(
+                        raw_request,
+                        FromWidgetErrorResponse::from_string(text),
+                    )];
                 };
                 if !capabilities.update_delayed_event {
                     return vec![self.send_from_widget_error_response(
                         raw_request,
-                        format!("Not allowed: missing the {UPDATE_DELAYED_EVENT} capability.")
-                            .into(),
+                        FromWidgetErrorResponse::from_string(format!(
+                            "Not allowed: missing the {UPDATE_DELAYED_EVENT} capability."
+                        )),
                     )];
                 }
                 let (request, request_action) =
@@ -276,7 +282,7 @@ impl WidgetMachine {
                         // does not impl Serialize
                         match result {
                             Ok(response) => Ok(Into::<UpdateDelayedEventResponse>::into(response)),
-                            Err(e) => Err((&e).into()),
+                            Err(e) => Err(FromWidgetErrorResponse::from_error(&e)), //---
                         },
                     )]
                 });
@@ -292,7 +298,10 @@ impl WidgetMachine {
     ) -> Option<Action> {
         let CapabilitiesState::Negotiated(capabilities) = &self.capabilities else {
             let text = "Received read event request before capabilities were negotiated";
-            return Some(self.send_from_widget_error_response(raw_request, text.into()));
+            return Some(self.send_from_widget_error_response(
+                raw_request,
+                FromWidgetErrorResponse::from_string(text),
+            ));
         };
 
         match request {
@@ -301,7 +310,9 @@ impl WidgetMachine {
                 if !capabilities.read.iter().any(filter_fn) {
                     return Some(self.send_from_widget_error_response(
                         raw_request,
-                        "Not allowed to read message like event".into(),
+                        FromWidgetErrorResponse::from_string(
+                            "Not allowed to read message like event",
+                        ),
                     ));
                 }
 
@@ -316,14 +327,16 @@ impl WidgetMachine {
                             Ok(ReadEventResponse { events })
                         }
                         (Ok(_), CapabilitiesState::Unset) => {
-                            Err("Received read event request before capabilities negotiation"
-                                .into())
+                            Err(FromWidgetErrorResponse::from_string(
+                                "Received read event request before capabilities negotiation",
+                            ))
                         }
                         (Ok(_), CapabilitiesState::Negotiating) => {
-                            Err("Received read event request while capabilities were negotiating"
-                                .into())
+                            Err(FromWidgetErrorResponse::from_string(
+                                "Received read event request while capabilities were negotiating",
+                            ))
                         }
-                        (Err(e), _) => Err((&e).into()),
+                        (Err(e), _) => Err(FromWidgetErrorResponse::from_error(&e)),
                     };
                     vec![machine.send_from_widget_result_response(raw_request, response)]
                 });
@@ -354,14 +367,14 @@ impl WidgetMachine {
                     request.then(|result, machine| {
                         let response = result
                             .map(|events| ReadEventResponse { events })
-                            .map_err(|e| (&e).into());
+                            .map_err(|e| FromWidgetErrorResponse::from_error(&e));
                         vec![machine.send_from_widget_result_response(raw_request, response)]
                     });
                     action
                 } else {
                     Some(self.send_from_widget_error_response(
                         raw_request,
-                        "Not allowed to read state event".into(),
+                        FromWidgetErrorResponse::from_string("Not allowed to read state event"),
                     ))
                 }
             }
@@ -391,25 +404,27 @@ impl WidgetMachine {
         if !capabilities.send_delayed_event && request.delay.is_some() {
             return Some(self.send_from_widget_error_response(
                 raw_request,
-                format!("Not allowed: missing the {SEND_DELAYED_EVENT} capability.").into(),
+                FromWidgetErrorResponse::from_string(format!(
+                    "Not allowed: missing the {SEND_DELAYED_EVENT} capability."
+                )),
             ));
         }
         if !capabilities.send.iter().any(|filter| filter.matches(&filter_in)) {
-            return Some(
-                self.send_from_widget_error_response(
-                    raw_request,
-                    "Not allowed to send event".into(),
-                ),
-            );
+            return Some(self.send_from_widget_error_response(
+                raw_request,
+                FromWidgetErrorResponse::from_string("Not allowed to send event"),
+            ));
         }
 
         let (request, action) = self.send_matrix_driver_request(request);
         request.then(|mut result, machine| {
             if let Ok(r) = result.as_mut() {
                 r.set_room_id(machine.room_id.clone());
             }
-            vec![machine
-                .send_from_widget_result_response(raw_request, result.map_err(|e| (&e).into()))]
+            vec![machine.send_from_widget_result_response(
+                raw_request,
+                result.map_err(|e| FromWidgetErrorResponse::from_error(&e)),
+            )]
         });
         action
     }
@@ -451,7 +466,7 @@ impl WidgetMachine {
     fn process_matrix_driver_response(
         &mut self,
         request_id: Uuid,
-        response: Result<MatrixDriverResponse, Error>,
+        response: Result<MatrixDriverResponse>,
     ) -> Vec<Action> {
         match self.pending_matrix_driver_requests.extract(&request_id) {
             Ok(request) => request
@@ -598,7 +613,7 @@ impl ToWidgetRequestMeta {
 }
 
 type MatrixDriverResponseFn =
-    Box<dyn FnOnce(Result<MatrixDriverResponse, Error>, &mut WidgetMachine) -> Vec<Action> + Send>;
+    Box<dyn FnOnce(Result<MatrixDriverResponse>, &mut WidgetMachine) -> Vec<Action> + Send>;
 
 pub(crate) struct MatrixDriverRequestMeta {
     response_fn: Option<MatrixDriverResponseFn>,
diff --git a/crates/matrix-sdk/src/widget/matrix.rs b/crates/matrix-sdk/src/widget/matrix.rs
@@ -52,7 +52,7 @@ impl MatrixDriver {
     }
 
     /// Requests an OpenID token for the current user.
-    pub(crate) async fn get_open_id(&self) -> Result<OpenIdResponse, Error> {
+    pub(crate) async fn get_open_id(&self) -> Result<OpenIdResponse> {
         let user_id = self.room.own_user_id().to_owned();
         self.room.client.send(OpenIdRequest::new(user_id), None).await.map_err(Error::Http)
     }
@@ -62,7 +62,7 @@ impl MatrixDriver {
         &self,
         event_type: MessageLikeEventType,
         limit: u32,
-    ) -> Result<Vec<Raw<AnyTimelineEvent>>, Error> {
+    ) -> Result<Vec<Raw<AnyTimelineEvent>>> {
         let options = assign!(MessagesOptions::backward(), {
             limit: limit.into(),
             filter: assign!(RoomEventFilter::default(), {
@@ -78,7 +78,7 @@ impl MatrixDriver {
         &self,
         event_type: StateEventType,
         state_key: &StateKeySelector,
-    ) -> Result<Vec<Raw<AnyTimelineEvent>>, Error> {
+    ) -> Result<Vec<Raw<AnyTimelineEvent>>> {
         let room_id = self.room.room_id();
         let convert = |sync_or_stripped_state| match sync_or_stripped_state {
             RawAnySyncOrStrippedState::Sync(ev) => Some(attach_room_id(ev.cast_ref(), room_id)),
@@ -116,7 +116,7 @@ impl MatrixDriver {
         state_key: Option<String>,
         content: Box<RawJsonValue>,
         delayed_event_parameters: Option<delayed_events::DelayParameters>,
-    ) -> Result<SendEventResponse, Error> {
+    ) -> Result<SendEventResponse> {
         let type_str = event_type.to_string();
 
         if let Some(redacts) = from_raw_json_value::<Value, serde_json::Error>(&content)
@@ -165,7 +165,7 @@ impl MatrixDriver {
         &self,
         delay_id: String,
         action: UpdateAction,
-    ) -> Result<delayed_events::update_delayed_event::unstable::Response, Error> {
+    ) -> Result<delayed_events::update_delayed_event::unstable::Response> {
         let r = delayed_events::update_delayed_event::unstable::Request::new(delay_id, action);
         self.room.client.send(r, None).await.map_err(Error::Http)
     }
diff --git a/crates/matrix-sdk/src/widget/mod.rs b/crates/matrix-sdk/src/widget/mod.rs
@@ -29,7 +29,7 @@ use self::{
     },
     matrix::MatrixDriver,
 };
-use crate::{room::Room, Error, Result};
+use crate::{room::Room, Result};
 
 mod capabilities;
 mod filter;
@@ -195,7 +195,7 @@ impl<T: CapabilitiesProvider> ProcessingContext<T> {
                 self.to_widget_tx.send(msg).await.map_err(|_| ())?;
             }
             Action::MatrixDriverRequest { request_id, data } => {
-                let response: Result<MatrixDriverResponse, Error> = match data {
+                let response = match data {
                     MatrixDriverRequestData::AcquireCapabilities(cmd) => {
                         let obtained = self
                             .capabilities_provider
diff --git a/crates/matrix-sdk/tests/integration/widget.rs b/crates/matrix-sdk/tests/integration/widget.rs
@@ -688,6 +688,8 @@ async fn test_fail_sending_delay_rate_limit() {
     )
     .await;
 
+    // TODO convert to mock_server.mock_room_send().for_type()... once for_type is
+    // available
     Mock::given(method("PUT"))
         .and(path_regex(r"^/_matrix/client/v3/rooms/.*/send/m.room.message/.*$"))
         .respond_with(ResponseTemplate::new(400).set_body_json(json!({
@@ -722,7 +724,6 @@ async fn test_fail_sending_delay_rate_limit() {
         json!({
                 "error": {
               "matrix_api_error": {
-                "http_headers": {},
                 "http_status": 400,
                 "response": {
                   "errcode": "M_LIMIT_EXCEEDED",

Original file line number	Diff line number	Diff line change
`@@ -74,6 +74,8 @@ where`
`74`	`74`	`Self { request_meta: None, _phantom: PhantomData }`
`75`	`75`	`}`
`76`	`76`
	`77`	`+ /// Setup a callback function that will be called once the matrix driver has`
	`78`	`+ /// processed the request.`
`77`	`79`	`pub(crate) fn then(`
`78`	`80`	`self,`
`79`	`81`	`response_handler: impl FnOnce(Result<T, crate::Error>, &mut WidgetMachine) -> Vec<Action>`