From 111f916a781f54a1a7c43d2630d704a3b43078a2 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Wed, 4 Dec 2024 17:52:02 +0100 Subject: [PATCH] feat(WidgetDriver): Pass Matrix API errors to the widget (#4241) Currently the WidgetDriver just returns unspecified error strings to the widget that can be used to display an issue description to the user. It is not helpful to run code like a retry or other error mitigation logic. Here it is proposed to add standardized errors for issues that every widget driver implementation can run into (all matrix cs api errors): https://github.com/matrix-org/matrix-spec-proposals/pull/2762#discussion_r1838804895 This PR forwards the errors that occur during the widget processing to the widget in the correct format. NOTE: It does not include request Url and http Headers. See also: https://github.com/matrix-org/matrix-spec-proposals/pull/2762#discussion_r1839802292 Co-authored-by: Benjamin Bouvier --- crates/matrix-sdk/src/test_utils/mocks.rs | 130 ++++++++++++++ .../src/widget/machine/driver_req.rs | 4 +- .../src/widget/machine/from_widget.rs | 71 +++++++- .../matrix-sdk/src/widget/machine/incoming.rs | 7 +- crates/matrix-sdk/src/widget/machine/mod.rs | 163 +++++++++++------- .../src/widget/machine/tests/capabilities.rs | 2 +- .../src/widget/machine/tests/openid.rs | 2 +- crates/matrix-sdk/src/widget/matrix.rs | 12 +- crates/matrix-sdk/src/widget/mod.rs | 20 +-- crates/matrix-sdk/tests/integration/widget.rs | 103 ++++++++--- 10 files changed, 390 insertions(+), 124 deletions(-) diff --git a/crates/matrix-sdk/src/test_utils/mocks.rs b/crates/matrix-sdk/src/test_utils/mocks.rs index f1637e20dac..9c583b95b5e 100644 --- a/crates/matrix-sdk/src/test_utils/mocks.rs +++ b/crates/matrix-sdk/src/test_utils/mocks.rs @@ -31,6 +31,7 @@ use ruma::{ directory::PublicRoomsChunk, events::{AnyStateEvent, AnyTimelineEvent, MessageLikeEventType, StateEventType}, serde::Raw, + time::Duration, MxcUri, OwnedEventId, OwnedRoomId, RoomId, ServerName, }; use serde::Deserialize; @@ -952,6 +953,72 @@ impl<'a> MockEndpoint<'a, RoomSendEndpoint> { } } + /// Ensures the event was sent as a delayed event. + /// + /// Note: works with *any* room. + /// + /// # Examples + /// + /// see also [`MatrixMockServer::mock_room_send`] for more context. + /// + /// ``` + /// # tokio_test::block_on(async { + /// use matrix_sdk::{ + /// ruma::{ + /// api::client::delayed_events::{delayed_message_event, DelayParameters}, + /// events::{message::MessageEventContent, AnyMessageLikeEventContent}, + /// room_id, + /// time::Duration, + /// TransactionId, + /// }, + /// test_utils::mocks::MatrixMockServer, + /// }; + /// use serde_json::json; + /// use wiremock::ResponseTemplate; + /// + /// let mock_server = MatrixMockServer::new().await; + /// let client = mock_server.client_builder().build().await; + /// + /// mock_server.mock_room_state_encryption().plain().mount().await; + /// + /// let room = mock_server.sync_joined_room(&client, room_id!("!room_id:localhost")).await; + /// + /// mock_server + /// .mock_room_send() + /// .with_delay(Duration::from_millis(500)) + /// .respond_with(ResponseTemplate::new(200).set_body_json(json!({"delay_id":"$some_id"}))) + /// .mock_once() + /// .mount() + /// .await; + /// + /// let response_not_mocked = + /// room.send_raw("m.room.message", json!({ "body": "Hello world" })).await; + /// + /// // A non delayed event should not be mocked by the server. + /// assert!(response_not_mocked.is_err()); + /// + /// let r = delayed_message_event::unstable::Request::new( + /// room.room_id().to_owned(), + /// TransactionId::new(), + /// DelayParameters::Timeout { timeout: Duration::from_millis(500) }, + /// &AnyMessageLikeEventContent::Message(MessageEventContent::plain("hello world")), + /// ) + /// .unwrap(); + /// + /// let response = room.client().send(r, None).await.unwrap(); + /// // The delayed `m.room.message` event type should be mocked by the server. + /// assert_eq!("$some_id", response.delay_id); + /// # anyhow::Ok(()) }); + /// ``` + pub fn with_delay(self, delay: Duration) -> Self { + Self { + mock: self + .mock + .and(query_param("org.matrix.msc4140.delay", delay.as_millis().to_string())), + ..self + } + } + /// Returns a send endpoint that emulates success, i.e. the event has been /// sent with the given event id. /// @@ -1117,6 +1184,69 @@ impl<'a> MockEndpoint<'a, RoomSendStateEndpoint> { Self { mock: self.mock.and(path_regex(Self::generate_path_regexp(&self.endpoint))), ..self } } + /// Ensures the event was sent as a delayed event. + /// + /// Note: works with *any* room. + /// + /// # Examples + /// + /// see also [`MatrixMockServer::mock_room_send`] for more context. + /// + /// ``` + /// # tokio_test::block_on(async { + /// use matrix_sdk::{ + /// ruma::{ + /// api::client::delayed_events::{delayed_state_event, DelayParameters}, + /// events::{room::create::RoomCreateEventContent, AnyStateEventContent}, + /// room_id, + /// time::Duration, + /// }, + /// test_utils::mocks::MatrixMockServer, + /// }; + /// use wiremock::ResponseTemplate; + /// use serde_json::json; + /// + /// let mock_server = MatrixMockServer::new().await; + /// let client = mock_server.client_builder().build().await; + /// + /// mock_server.mock_room_state_encryption().plain().mount().await; + /// + /// let room = mock_server.sync_joined_room(&client, room_id!("!room_id:localhost")).await; + /// + /// mock_server + /// .mock_room_send_state() + /// .with_delay(Duration::from_millis(500)) + /// .respond_with(ResponseTemplate::new(200).set_body_json(json!({"delay_id":"$some_id"}))) + /// .mock_once() + /// .mount() + /// .await; + /// + /// let response_not_mocked = room.send_state_event(RoomCreateEventContent::new_v11()).await; + /// // A non delayed event should not be mocked by the server. + /// assert!(response_not_mocked.is_err()); + /// + /// let r = delayed_state_event::unstable::Request::new( + /// room.room_id().to_owned(), + /// "".to_owned(), + /// DelayParameters::Timeout { timeout: Duration::from_millis(500) }, + /// &AnyStateEventContent::RoomCreate(RoomCreateEventContent::new_v11()), + /// ) + /// .unwrap(); + /// let response = room.client().send(r, None).await.unwrap(); + /// // The delayed `m.room.message` event type should be mocked by the server. + /// assert_eq!("$some_id", response.delay_id); + /// + /// # anyhow::Ok(()) }); + /// ``` + pub fn with_delay(self, delay: Duration) -> Self { + Self { + mock: self + .mock + .and(query_param("org.matrix.msc4140.delay", delay.as_millis().to_string())), + ..self + } + } + /// /// ``` /// # tokio_test::block_on(async { diff --git a/crates/matrix-sdk/src/widget/machine/driver_req.rs b/crates/matrix-sdk/src/widget/machine/driver_req.rs index feeba69888b..675a070696d 100644 --- a/crates/matrix-sdk/src/widget/machine/driver_req.rs +++ b/crates/matrix-sdk/src/widget/machine/driver_req.rs @@ -74,9 +74,11 @@ 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, &mut WidgetMachine) -> Vec + response_handler: impl FnOnce(Result, &mut WidgetMachine) -> Vec + Send + 'static, ) { diff --git a/crates/matrix-sdk/src/widget/machine/from_widget.rs b/crates/matrix-sdk/src/widget/machine/from_widget.rs index e70d53cf7d5..ed5384631ea 100644 --- a/crates/matrix-sdk/src/widget/machine/from_widget.rs +++ b/crates/matrix-sdk/src/widget/machine/from_widget.rs @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt; - +use as_variant::as_variant; use ruma::{ - api::client::delayed_events::{ - delayed_message_event, delayed_state_event, update_delayed_event, + api::client::{ + delayed_events::{delayed_message_event, delayed_state_event, update_delayed_event}, + error::{ErrorBody, StandardErrorBody}, }, events::{AnyTimelineEvent, MessageLikeEventType, StateEventType}, serde::Raw, @@ -25,7 +25,7 @@ use ruma::{ use serde::{Deserialize, Serialize}; use super::{SendEventRequest, UpdateDelayedEventRequest}; -use crate::widget::StateKeySelector; +use crate::{widget::StateKeySelector, Error, HttpError, RumaApiError}; #[derive(Deserialize, Debug)] #[serde(tag = "action", rename_all = "snake_case", content = "data")] @@ -41,28 +41,85 @@ pub(super) enum FromWidgetRequest { DelayedEventUpdate(UpdateDelayedEventRequest), } +/// The full response a client sends to a [`FromWidgetRequest`] in case of an +/// error. #[derive(Serialize)] pub(super) struct FromWidgetErrorResponse { error: FromWidgetError, } impl FromWidgetErrorResponse { - pub(super) fn new(e: impl fmt::Display) -> Self { - Self { error: FromWidgetError { message: e.to_string() } } + /// Create a error response to send to the widget from an http error. + pub(crate) fn from_http_error(error: HttpError) -> Self { + let message = error.to_string(); + let matrix_api_error = as_variant!(error, HttpError::Api(ruma::api::error::FromHttpResponseError::Server(RumaApiError::ClientApi(err))) => err); + + Self { + error: FromWidgetError { + message, + matrix_api_error: matrix_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(), + response: StandardErrorBody { kind, message }, + }), + _ => None, + }), + }, + } + } + + /// 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) => FromWidgetErrorResponse::from_http_error(e), + // For UnknownError's we do not want to have the `unknown error` bit in the message. + // Hence we only convert the inner error to a string. + Error::UnknownError(e) => FromWidgetErrorResponse::from_string(e.to_string()), + _ => FromWidgetErrorResponse::from_string(error.to_string()), + } + } + + /// Create a error response to send to the widget from a string. + pub(crate) fn from_string>(error: S) -> Self { + Self { error: FromWidgetError { message: error.into(), matrix_api_error: None } } } } +/// Serializable section of an error response send by the client as a +/// response to a [`FromWidgetRequest`]. #[derive(Serialize)] struct FromWidgetError { + /// 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, + + /// Optional matrix error hinting at workarounds for specific errors. + matrix_api_error: Option, +} + +/// Serializable section of a widget response that represents a matrix error. +#[derive(Serialize)] +struct FromWidgetMatrixErrorBody { + /// Status code of the http response. + http_status: u32, + + /// 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: StandardErrorBody, } +/// The serializable section of a widget response containing the supported +/// versions. #[derive(Serialize)] pub(super) struct SupportedApiVersionsResponse { supported_versions: Vec, } 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 index 7d165b59c99..d90f3740869 100644 --- a/crates/matrix-sdk/src/widget/machine/incoming.rs +++ b/crates/matrix-sdk/src/widget/machine/incoming.rs @@ -37,8 +37,11 @@ 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. - response: Result, + /// Result of the request: the response data, or a matrix sdk error. + /// + /// Http errors will be forwarded to the widget in a specified format so + /// the widget can parse the error. + response: Result, }, /// The `MatrixDriver` notified the `WidgetMachine` of a new matrix event. diff --git a/crates/matrix-sdk/src/widget/machine/mod.rs b/crates/matrix-sdk/src/widget/machine/mod.rs index ca000d37632..e367b5cc199 100644 --- a/crates/matrix-sdk/src/widget/machine/mod.rs +++ b/crates/matrix-sdk/src/widget/machine/mod.rs @@ -14,7 +14,7 @@ //! No I/O logic of the [`WidgetDriver`]. -use std::{fmt, iter, time::Duration}; +use std::{iter, time::Duration}; use driver_req::UpdateDelayedEventRequest; use from_widget::UpdateDelayedEventResponse; @@ -48,10 +48,11 @@ use self::{ #[cfg(doc)] use super::WidgetDriver; use super::{ - capabilities, + capabilities::{SEND_DELAYED_EVENT, UPDATE_DELAYED_EVENT}, filter::{MatrixEventContent, MatrixEventFilterInput}, Capabilities, StateKeySelector, }; +use crate::Result; mod driver_req; mod from_widget; @@ -212,18 +213,23 @@ impl WidgetMachine { ) -> Vec { let request = match raw_request.deserialize() { Ok(r) => r, - Err(e) => return vec![Self::send_from_widget_error_response(raw_request, e)], + Err(e) => { + return vec![Self::send_from_widget_err_response( + raw_request, + FromWidgetErrorResponse::from_error(crate::Error::SerdeJson(e)), + )] + } }; match request { FromWidgetRequest::SupportedApiVersions {} => { let response = SupportedApiVersionsResponse::new(); - vec![Self::send_from_widget_response(raw_request, response)] + vec![Self::send_from_widget_response(raw_request, Ok(response))] } FromWidgetRequest::ContentLoaded {} => { let mut response = - vec![Self::send_from_widget_response(raw_request, JsonObject::new())]; + vec![Self::send_from_widget_response(raw_request, Ok(JsonObject::new()))]; if self.capabilities.is_unset() { response.append(&mut self.negotiate_capabilities()); } @@ -256,24 +262,22 @@ impl WidgetMachine { }); let response = - Self::send_from_widget_response(raw_request, OpenIdResponse::Pending); + Self::send_from_widget_response(raw_request, Ok(OpenIdResponse::Pending)); iter::once(response).chain(request_action).collect() } FromWidgetRequest::DelayedEventUpdate(req) => { 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)]; + return vec![Self::send_from_widget_error_string_response( + raw_request, + "Received send update delayed event request before capabilities were negotiated" + )]; }; if !capabilities.update_delayed_event { - return vec![Self::send_from_widget_error_response( + return vec![Self::send_from_widget_error_string_response( raw_request, - format!( - "Not allowed: missing the {} capability.", - capabilities::UPDATE_DELAYED_EVENT - ), + format!("Not allowed: missing the {UPDATE_DELAYED_EVENT} capability."), )]; } @@ -282,13 +286,14 @@ impl WidgetMachine { action: req.action, delay_id: req.delay_id, }); - - request.then(|res, _machine| { - vec![Self::send_from_widget_result_response( + request.then(|result, _machine| { + vec![Self::send_from_widget_response( raw_request, // This is mapped to another type because the update_delay_event::Response // does not impl Serialize - res.map(Into::::into), + result + .map(Into::::into) + .map_err(FromWidgetErrorResponse::from_error), )] }); @@ -303,15 +308,17 @@ impl WidgetMachine { raw_request: Raw, ) -> Option { 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)); + return Some(Self::send_from_widget_error_string_response( + raw_request, + "Received read event request before capabilities were negotiated", + )); }; match request { ReadEventRequest::ReadMessageLikeEvent { event_type, limit } => { if !capabilities.read.iter().any(|f| f.matches_message_like_event_type(&event_type)) { - return Some(Self::send_from_widget_error_response( + return Some(Self::send_from_widget_error_string_response( raw_request, "Not allowed to read message like event", )); @@ -324,18 +331,24 @@ impl WidgetMachine { let (request, action) = self.send_matrix_driver_request(request); request.then(|result, machine| { - let response = result.and_then(|mut events| { - let CapabilitiesState::Negotiated(capabilities) = &machine.capabilities - else { - let err = "Received read event request before capabilities negotiation"; - return Err(err.into()); - }; - - events.retain(|e| capabilities.raw_event_matches_read_filter(e)); - Ok(ReadEventResponse { events }) - }); + let response = match &machine.capabilities { + CapabilitiesState::Unset => Err(FromWidgetErrorResponse::from_string( + "Received read event request before capabilities negotiation", + )), + CapabilitiesState::Negotiating => { + Err(FromWidgetErrorResponse::from_string( + "Received read event request while capabilities were negotiating", + )) + } + CapabilitiesState::Negotiated(capabilities) => result + .map(|mut events| { + events.retain(|e| capabilities.raw_event_matches_read_filter(e)); + ReadEventResponse { events } + }) + .map_err(FromWidgetErrorResponse::from_error), + }; - vec![Self::send_from_widget_result_response(raw_request, response)] + vec![Self::send_from_widget_response(raw_request, response)] }); action @@ -364,12 +377,14 @@ impl WidgetMachine { let request = ReadStateEventRequest { event_type, state_key }; let (request, action) = self.send_matrix_driver_request(request); request.then(|result, _machine| { - let response = result.map(|events| ReadEventResponse { events }); - vec![Self::send_from_widget_result_response(raw_request, response)] + let response = result + .map(|events| ReadEventResponse { events }) + .map_err(FromWidgetErrorResponse::from_error); + vec![Self::send_from_widget_response(raw_request, response)] }); action } else { - Some(Self::send_from_widget_error_response( + Some(Self::send_from_widget_error_string_response( raw_request, "Not allowed to read state event", )) @@ -400,17 +415,14 @@ impl WidgetMachine { }; if !capabilities.send_delayed_event && request.delay.is_some() { - return Some(Self::send_from_widget_error_response( + return Some(Self::send_from_widget_error_string_response( raw_request, - format!( - "Not allowed: missing the {} capability.", - capabilities::SEND_DELAYED_EVENT - ), + 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( + return Some(Self::send_from_widget_error_string_response( raw_request, "Not allowed to send event", )); @@ -422,7 +434,10 @@ impl WidgetMachine { if let Ok(r) = result.as_mut() { r.set_room_id(machine.room_id.clone()); } - vec![Self::send_from_widget_result_response(raw_request, result)] + vec![Self::send_from_widget_response( + raw_request, + result.map_err(FromWidgetErrorResponse::from_error), + )] }); action @@ -465,7 +480,7 @@ impl WidgetMachine { fn process_matrix_driver_response( &mut self, request_id: Uuid, - response: Result, + response: Result, ) -> Vec { match self.pending_matrix_driver_requests.extract(&request_id) { Ok(request) => request @@ -479,39 +494,55 @@ impl WidgetMachine { } } - #[instrument(skip_all)] - fn send_from_widget_response( + fn send_from_widget_error_string_response( raw_request: Raw, - response_data: impl Serialize, + error: impl Into, ) -> Action { - let f = || { - let mut object = raw_request.deserialize_as::>>()?; - let response_data = serde_json::value::to_raw_value(&response_data)?; - object.insert("response".to_owned(), response_data); - serde_json::to_string(&object) - }; - - // SAFETY: we expect the raw request to be a valid JSON map, to which we add a - // new field. - let serialized = f().expect("error when attaching response to incoming request"); - - Action::SendToWidget(serialized) + Self::send_from_widget_err_response( + raw_request, + FromWidgetErrorResponse::from_string(error), + ) } - fn send_from_widget_error_response( + fn send_from_widget_err_response( raw_request: Raw, - error: impl fmt::Display, + error: FromWidgetErrorResponse, ) -> Action { - Self::send_from_widget_response(raw_request, FromWidgetErrorResponse::new(error)) + Self::send_from_widget_response( + raw_request, + Err::(error), + ) } - fn send_from_widget_result_response( + fn send_from_widget_response( raw_request: Raw, - result: Result, + result: Result, ) -> Action { + // we do not want tho expose this to never allow sending arbitrary errors. + // Errors always need to be `FromWidgetErrorResponse`. + #[instrument(skip_all)] + fn send_response_data( + raw_request: Raw, + response_data: impl Serialize, + ) -> Action { + let f = || { + let mut object = + raw_request.deserialize_as::>>()?; + let response_data = serde_json::value::to_raw_value(&response_data)?; + object.insert("response".to_owned(), response_data); + serde_json::to_string(&object) + }; + + // SAFETY: we expect the raw request to be a valid JSON map, to which we add a + // new field. + let serialized = f().expect("error when attaching response to incoming request"); + + Action::SendToWidget(serialized) + } + match result { - Ok(res) => Self::send_from_widget_response(raw_request, res), - Err(msg) => Self::send_from_widget_error_response(raw_request, msg), + Ok(res) => send_response_data(raw_request, res), + Err(error_response) => send_response_data(raw_request, error_response), } } @@ -613,7 +644,7 @@ impl ToWidgetRequestMeta { } type MatrixDriverResponseFn = - Box, &mut WidgetMachine) -> Vec + Send>; + Box, &mut WidgetMachine) -> Vec + Send>; pub(crate) struct MatrixDriverRequestMeta { response_fn: Option, diff --git a/crates/matrix-sdk/src/widget/machine/tests/capabilities.rs b/crates/matrix-sdk/src/widget/machine/tests/capabilities.rs index 633d7880163..0b221a3a17f 100644 --- a/crates/matrix-sdk/src/widget/machine/tests/capabilities.rs +++ b/crates/matrix-sdk/src/widget/machine/tests/capabilities.rs @@ -114,7 +114,7 @@ fn test_capabilities_failure_results_into_empty_capabilities() { machine.process(IncomingMessage::MatrixDriverResponse { request_id, - response: Err("OHMG!".into()), + response: Err(crate::Error::UnknownError("OHMG!".into())), }) }; diff --git a/crates/matrix-sdk/src/widget/machine/tests/openid.rs b/crates/matrix-sdk/src/widget/machine/tests/openid.rs index 2aef7f66963..bebd67faf99 100644 --- a/crates/matrix-sdk/src/widget/machine/tests/openid.rs +++ b/crates/matrix-sdk/src/widget/machine/tests/openid.rs @@ -154,7 +154,7 @@ fn test_openid_fail_results_in_response_blocked() { machine.process(IncomingMessage::MatrixDriverResponse { request_id, - response: Err("Unlucky one".into()), + response: Err(crate::Error::UnknownError("Unlucky one".into())), }) }; diff --git a/crates/matrix-sdk/src/widget/matrix.rs b/crates/matrix-sdk/src/widget/matrix.rs index 84e25de4c31..6365779d333 100644 --- a/crates/matrix-sdk/src/widget/matrix.rs +++ b/crates/matrix-sdk/src/widget/matrix.rs @@ -38,9 +38,7 @@ use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; use tracing::error; use super::{machine::SendEventResponse, StateKeySelector}; -use crate::{ - event_handler::EventHandlerDropGuard, room::MessagesOptions, HttpResult, Result, Room, -}; +use crate::{event_handler::EventHandlerDropGuard, room::MessagesOptions, Error, Result, Room}; /// Thin wrapper around a [`Room`] that provides functionality relevant for /// widgets. @@ -55,9 +53,9 @@ impl MatrixDriver { } /// Requests an OpenID token for the current user. - pub(crate) async fn get_open_id(&self) -> HttpResult { + pub(crate) async fn get_open_id(&self) -> Result { let user_id = self.room.own_user_id().to_owned(); - self.room.client.send(OpenIdRequest::new(user_id), None).await + self.room.client.send(OpenIdRequest::new(user_id), None).await.map_err(Error::Http) } /// Reads the latest `limit` events of a given `event_type` from the room. @@ -172,9 +170,9 @@ impl MatrixDriver { &self, delay_id: String, action: UpdateAction, - ) -> HttpResult { + ) -> Result { let r = delayed_events::update_delayed_event::unstable::Request::new(delay_id, action); - self.room.client.send(r, None).await + self.room.client.send(r, None).await.map_err(Error::Http) } /// Starts forwarding new room events. Once the returned `EventReceiver` diff --git a/crates/matrix-sdk/src/widget/mod.rs b/crates/matrix-sdk/src/widget/mod.rs index 4f75a006b3a..c67e0f6cfee 100644 --- 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, HttpError, Result}; +use crate::{room::Room, Result}; mod capabilities; mod filter; @@ -202,23 +202,19 @@ impl WidgetDriver { Ok(MatrixDriverResponse::CapabilitiesAcquired(obtained)) } - MatrixDriverRequestData::GetOpenId => matrix_driver - .get_open_id() - .await - .map(MatrixDriverResponse::OpenIdReceived) - .map_err(|e| e.to_string()), + MatrixDriverRequestData::GetOpenId => { + matrix_driver.get_open_id().await.map(MatrixDriverResponse::OpenIdReceived) + } MatrixDriverRequestData::ReadMessageLikeEvent(cmd) => matrix_driver .read_message_like_events(cmd.event_type.clone(), cmd.limit) .await - .map(MatrixDriverResponse::MatrixEventRead) - .map_err(|e| e.to_string()), + .map(MatrixDriverResponse::MatrixEventRead), MatrixDriverRequestData::ReadStateEvent(cmd) => matrix_driver .read_state_events(cmd.event_type.clone(), &cmd.state_key) .await - .map(MatrixDriverResponse::MatrixEventRead) - .map_err(|e| e.to_string()), + .map(MatrixDriverResponse::MatrixEventRead), MatrixDriverRequestData::SendMatrixEvent(req) => { let SendEventRequest { event_type, state_key, content, delay } = req; @@ -233,14 +229,12 @@ impl WidgetDriver { .send(event_type, state_key, content, delay_event_parameter) .await .map(MatrixDriverResponse::MatrixEventSent) - .map_err(|e: crate::Error| e.to_string()) } MatrixDriverRequestData::UpdateDelayedEvent(req) => matrix_driver .update_delayed_event(req.delay_id, req.action) .await - .map(MatrixDriverResponse::MatrixDelayedEventUpdate) - .map_err(|e: HttpError| e.to_string()), + .map(MatrixDriverResponse::MatrixDelayedEventUpdate), }; // Forward the matrix driver response to the incoming message stream. diff --git a/crates/matrix-sdk/tests/integration/widget.rs b/crates/matrix-sdk/tests/integration/widget.rs index c2f2b03c076..1ed2b91dc7f 100644 --- a/crates/matrix-sdk/tests/integration/widget.rs +++ b/crates/matrix-sdk/tests/integration/widget.rs @@ -38,7 +38,7 @@ use ruma::{ name::RoomNameEventContent, topic::RoomTopicEventContent, }, - StateEventType, + MessageLikeEventType, StateEventType, }, owned_room_id, serde::JsonObject, @@ -48,7 +48,7 @@ use serde::Serialize; use serde_json::{json, Value as JsonValue}; use tracing::error; use wiremock::{ - matchers::{header, method, path_regex, query_param}, + matchers::{method, path_regex}, Mock, ResponseTemplate, }; @@ -245,13 +245,12 @@ async fn test_read_messages() { "end": "t47409-4357353_219380_26003_2269", "start": "t392-516_47314_0_7_1_1_1_11444_1" }); - Mock::given(method("GET")) - .and(path_regex(r"^/_matrix/client/v3/rooms/.*/messages$")) - .and(header("authorization", "Bearer 1234")) - .and(query_param("limit", "2")) + mock_server + .mock_room_messages() + .limit(2) .respond_with(ResponseTemplate::new(200).set_body_json(response_json)) - .expect(1) - .mount(mock_server.server()) + .mock_once() + .mount() .await; // Ask the driver to read messages @@ -512,8 +511,6 @@ async fn test_send_room_message() { assert_eq!(msg["action"], "send_event"); let event_id = msg["response"]["event_id"].as_str().unwrap(); assert_eq!(event_id, "$foobar"); - - // Make sure the event-sending endpoint was hit exactly once } #[async_test] @@ -554,8 +551,6 @@ async fn test_send_room_name() { assert_eq!(msg["action"], "send_event"); let event_id = msg["response"]["event_id"].as_str().unwrap(); assert_eq!(event_id, "$foobar"); - - // Make sure the event-sending endpoint was hit exactly once } #[async_test] @@ -570,15 +565,15 @@ async fn test_send_delayed_message_event() { ]), ) .await; - - Mock::given(method("PUT")) - .and(path_regex(r"^/_matrix/client/v3/rooms/.*/send/m.room.message/.*")) - .and(query_param("org.matrix.msc4140.delay", "1000")) + mock_server + .mock_room_send() + .with_delay(Duration::from_millis(1000)) + .for_type(MessageLikeEventType::RoomMessage) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "delay_id": "1234", }))) - .expect(1) - .mount(mock_server.server()) + .mock_once() + .mount() .await; send_request( @@ -602,8 +597,6 @@ async fn test_send_delayed_message_event() { assert_eq!(msg["action"], "send_event"); let delay_id = msg["response"]["delay_id"].as_str().unwrap(); assert_eq!(delay_id, "1234"); - - // Make sure the event-sending endpoint was hit exactly once } #[async_test] @@ -619,14 +612,15 @@ async fn test_send_delayed_state_event() { ) .await; - Mock::given(method("PUT")) - .and(path_regex(r"^/_matrix/client/v3/rooms/.*/state/m.room.name/.*")) - .and(query_param("org.matrix.msc4140.delay", "1000")) + mock_server + .mock_room_send_state() + .with_delay(Duration::from_millis(1000)) + .for_type(StateEventType::RoomName) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "delay_id": "1234", }))) - .expect(1) - .mount(mock_server.server()) + .mock_once() + .mount() .await; send_request( @@ -650,8 +644,65 @@ async fn test_send_delayed_state_event() { assert_eq!(msg["action"], "send_event"); let delay_id = msg["response"]["delay_id"].as_str().unwrap(); assert_eq!(delay_id, "1234"); +} + +#[async_test] +async fn test_fail_sending_delay_rate_limit() { + let (_, mock_server, driver_handle) = run_test_driver(false).await; + + negotiate_capabilities( + &driver_handle, + json!([ + "org.matrix.msc4157.send.delayed_event", + "org.matrix.msc2762.send.event:m.room.message" + ]), + ) + .await; + + mock_server + .mock_room_send() + .respond_with(ResponseTemplate::new(400).set_body_json(json!({ + "errcode": "M_LIMIT_EXCEEDED", + "error": "Sending too many delay events" + }))) + .mock_once() + .mount() + .await; + + send_request( + &driver_handle, + "send-room-message", + "send_event", + json!({ + "type": "m.room.message", + "content": { + "msgtype": "m.text", + "body": "Message from a widget!", + }, + "delay":1000, + }), + ) + .await; - // Make sure the event-sending endpoint was hit exactly once + let msg = recv_message(&driver_handle).await; + assert_eq!(msg["api"], "fromWidget"); + assert_eq!(msg["action"], "send_event"); + // Receive the response in the correct widget error response format + assert_eq!( + msg["response"], + json!({ + "error": { + "matrix_api_error": { + "http_status": 400, + "response": { + "errcode": "M_LIMIT_EXCEEDED", + "error": "Sending too many delay events" + }, + }, + "message": "the server returned an error: [400 / M_LIMIT_EXCEEDED] Sending too many delay events" + } + }) + ); } #[async_test]