From cccf7a01b988a33082c3d12511528edb9160ecc4 Mon Sep 17 00:00:00 2001 From: Timo Date: Sun, 10 Nov 2024 23:31:06 +0100 Subject: [PATCH 1/2] feat(WidgetDriver): Pass matrix api errors to the widget --- .../src/widget/machine/driver_req.rs | 2 +- .../src/widget/machine/from_widget.rs | 62 ++++++++++++-- .../matrix-sdk/src/widget/machine/incoming.rs | 2 +- crates/matrix-sdk/src/widget/machine/mod.rs | 84 +++++++++++-------- .../src/widget/machine/tests/capabilities.rs | 2 +- .../src/widget/machine/tests/openid.rs | 2 +- crates/matrix-sdk/src/widget/matrix.rs | 18 ++-- crates/matrix-sdk/src/widget/mod.rs | 17 ++-- crates/matrix-sdk/tests/integration/widget.rs | 67 +++++++++++++-- 9 files changed, 181 insertions(+), 75 deletions(-) diff --git a/crates/matrix-sdk/src/widget/machine/driver_req.rs b/crates/matrix-sdk/src/widget/machine/driver_req.rs index feeba69888b..95f1766696d 100644 --- a/crates/matrix-sdk/src/widget/machine/driver_req.rs +++ b/crates/matrix-sdk/src/widget/machine/driver_req.rs @@ -76,7 +76,7 @@ where 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 7dcb3c86ad7..bb535a4fa37 100644 --- a/crates/matrix-sdk/src/widget/machine/from_widget.rs +++ b/crates/matrix-sdk/src/widget/machine/from_widget.rs @@ -12,20 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt; +use std::collections::HashMap; 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, }, events::{AnyTimelineEvent, MessageLikeEventType, StateEventType}, serde::Raw, OwnedEventId, OwnedRoomId, }; use serde::{Deserialize, Serialize}; +use serde_json::{json, Value}; use super::{SendEventRequest, UpdateDelayedEventRequest}; -use crate::widget::StateKeySelector; +use crate::{widget::StateKeySelector, Error, HttpError}; #[derive(Deserialize, Debug)] #[serde(tag = "action", rename_all = "snake_case", content = "data")] @@ -46,17 +48,63 @@ pub(super) struct FromWidgetErrorResponse { error: FromWidgetError, } -impl FromWidgetErrorResponse { - pub(super) fn new(e: impl fmt::Display) -> Self { - Self { error: FromWidgetError { message: e.to_string() } } +impl From<&HttpError> for FromWidgetErrorResponse { + fn from(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, + }, + ), + }, + } + } +} + +impl From<&Error> for FromWidgetErrorResponse { + fn from(error: &Error) -> Self { + match &error { + Error::Http(e) => e.into(), + Error::UnknownError(e) => e.to_string().into(), + _ => error.to_string().into(), + } + } +} + +impl From 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_string().into() } } #[derive(Serialize)] struct FromWidgetError { message: String, + matrix_api_error: Option, } +#[derive(Serialize)] +struct FromWidgetMatrixErrorBody { + http_status: u32, + // TODO: figure out the which type to use here. + http_headers: HashMap, + url: String, + response: Value, +} #[derive(Serialize)] pub(super) struct SupportedApiVersionsResponse { supported_versions: Vec, diff --git a/crates/matrix-sdk/src/widget/machine/incoming.rs b/crates/matrix-sdk/src/widget/machine/incoming.rs index e0ca2c1b968..dcaa7395984 100644 --- a/crates/matrix-sdk/src/widget/machine/incoming.rs +++ b/crates/matrix-sdk/src/widget/machine/incoming.rs @@ -38,7 +38,7 @@ pub(crate) enum IncomingMessage { request_id: Uuid, /// The result of the request: response data or error message. - response: Result, + 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 eb0c521878e..2993dd0bf51 100644 --- a/crates/matrix-sdk/src/widget/machine/mod.rs +++ b/crates/matrix-sdk/src/widget/machine/mod.rs @@ -16,7 +16,7 @@ #![warn(unreachable_pub)] -use std::{fmt, iter, time::Duration}; +use std::{iter, time::Duration}; use driver_req::UpdateDelayedEventRequest; use from_widget::UpdateDelayedEventResponse; @@ -50,11 +50,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::widget::EventFilter; +use crate::{widget::EventFilter, Error}; mod driver_req; mod from_widget; @@ -203,7 +203,10 @@ 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_error_response(raw_request, (&Error::SerdeJson(e)).into())] + } }; match request { @@ -252,15 +255,13 @@ 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)]; + return vec![self.send_from_widget_error_response(raw_request, text.into())]; }; if !capabilities.update_delayed_event { return vec![self.send_from_widget_error_response( raw_request, - format!( - "Not allowed: missing the {} capability.", - capabilities::UPDATE_DELAYED_EVENT - ), + format!("Not allowed: missing the {UPDATE_DELAYED_EVENT} capability.") + .into(), )]; } let (request, request_action) = @@ -268,12 +269,15 @@ impl WidgetMachine { action: req.action, delay_id: req.delay_id, }); - request.then(|res, machine| { + request.then(|result, machine| { vec![machine.send_from_widget_result_response( raw_request, // This is mapped to another type because the update_delay_event::Response // does not impl Serialize - res.map(Into::::into), + match result { + Ok(response) => Ok(Into::::into(response)), + Err(e) => Err((&e).into()), + }, )] }); request_action.map(|a| vec![a]).unwrap_or_default() @@ -288,7 +292,7 @@ impl WidgetMachine { ) -> 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_response(raw_request, text.into())); }; match request { @@ -297,7 +301,7 @@ 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", + "Not allowed to read message like event".into(), )); } @@ -306,16 +310,21 @@ impl WidgetMachine { let request = ReadMessageLikeEventRequest { event_type, limit }; 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 (result, &machine.capabilities) { + (Ok(mut events), CapabilitiesState::Negotiated(capabilities)) => { + events.retain(|e| capabilities.raw_event_matches_read_filter(e)); + Ok(ReadEventResponse { events }) + } + (Ok(_), CapabilitiesState::Unset) => { + Err("Received read event request before capabilities negotiation" + .into()) + } + (Ok(_), CapabilitiesState::Negotiating) => { + Err("Received read event request while capabilities were negotiating" + .into()) + } + (Err(e), _) => Err((&e).into()), + }; vec![machine.send_from_widget_result_response(raw_request, response)] }); action @@ -343,14 +352,16 @@ 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 }); + let response = result + .map(|events| ReadEventResponse { events }) + .map_err(|e| (&e).into()); 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", + "Not allowed to read state event".into(), )) } } @@ -380,15 +391,15 @@ 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 {} capability.", - capabilities::SEND_DELAYED_EVENT - ), + format!("Not allowed: missing the {SEND_DELAYED_EVENT} capability.").into(), )); } 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"), + self.send_from_widget_error_response( + raw_request, + "Not allowed to send event".into(), + ), ); } @@ -397,7 +408,8 @@ impl WidgetMachine { 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)] + vec![machine + .send_from_widget_result_response(raw_request, result.map_err(|e| (&e).into()))] }); action } @@ -439,7 +451,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 @@ -472,15 +484,15 @@ impl WidgetMachine { fn send_from_widget_error_response( &self, 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, error) } fn send_from_widget_result_response( &self, raw_request: Raw, - result: Result, + result: Result, ) -> Action { match result { Ok(res) => self.send_from_widget_response(raw_request, res), @@ -586,7 +598,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 3c06da002ae..b4b77fa1d2a 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 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 f5b248b3c0b..9aaf30387c5 100644 --- a/crates/matrix-sdk/src/widget/machine/tests/openid.rs +++ b/crates/matrix-sdk/src/widget/machine/tests/openid.rs @@ -162,7 +162,7 @@ fn 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 e30e342d60e..f6c51eda072 100644 --- a/crates/matrix-sdk/src/widget/matrix.rs +++ b/crates/matrix-sdk/src/widget/matrix.rs @@ -37,9 +37,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. @@ -54,9 +52,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. @@ -64,7 +62,7 @@ impl MatrixDriver { &self, event_type: MessageLikeEventType, limit: u32, - ) -> Result>> { + ) -> Result>, Error> { let options = assign!(MessagesOptions::backward(), { limit: limit.into(), filter: assign!(RoomEventFilter::default(), { @@ -80,7 +78,7 @@ impl MatrixDriver { &self, event_type: StateEventType, state_key: &StateKeySelector, - ) -> Result>> { + ) -> Result>, Error> { 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)), @@ -118,7 +116,7 @@ impl MatrixDriver { state_key: Option, content: Box, delayed_event_parameters: Option, - ) -> Result { + ) -> Result { let type_str = event_type.to_string(); if let Some(redacts) = from_raw_json_value::(&content) @@ -167,9 +165,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 d5f7109e4ff..409c0f0d8df 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, Error, Result}; mod capabilities; mod filter; @@ -195,7 +195,7 @@ impl ProcessingContext { self.to_widget_tx.send(msg).await.map_err(|_| ())?; } Action::MatrixDriverRequest { request_id, data } => { - let response = match data { + let response: Result = match data { MatrixDriverRequestData::AcquireCapabilities(cmd) => { let obtained = self .capabilities_provider @@ -208,22 +208,19 @@ impl ProcessingContext { .matrix_driver .get_open_id() .await - .map(MatrixDriverResponse::OpenIdReceived) - .map_err(|e| e.to_string()), + .map(MatrixDriverResponse::OpenIdReceived), MatrixDriverRequestData::ReadMessageLikeEvent(cmd) => self .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) => self .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; @@ -238,15 +235,13 @@ impl ProcessingContext { .send(event_type, state_key, content, delay_event_parameter) .await .map(MatrixDriverResponse::MatrixEventSent) - .map_err(|e: crate::Error| e.to_string()) } MatrixDriverRequestData::UpdateDelayedEvent(req) => self .matrix_driver .update_delayed_event(req.delay_id, req.action) .await - .map(MatrixDriverResponse::MatrixDelayedEventUpdate) - .map_err(|e: HttpError| e.to_string()), + .map(MatrixDriverResponse::MatrixDelayedEventUpdate), }; self.events_tx diff --git a/crates/matrix-sdk/tests/integration/widget.rs b/crates/matrix-sdk/tests/integration/widget.rs index 909880dcd40..84f51d7dfbd 100644 --- a/crates/matrix-sdk/tests/integration/widget.rs +++ b/crates/matrix-sdk/tests/integration/widget.rs @@ -544,8 +544,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] @@ -585,8 +583,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] @@ -632,8 +628,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] @@ -679,8 +673,67 @@ 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::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/v3/rooms/.*/send/m.room.message/.*$")) + .respond_with(ResponseTemplate::new(400).set_body_json(json!({ + "errcode": "M_LIMIT_EXCEEDED", + "error": "Sending too many delay events" + }))) + .expect(1) + .mount(mock_server.server()) + .await; - // Make sure the event-sending endpoint was hit exactly once + 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; + + 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_headers": {}, + "http_status": 400, + "response": { + "errcode": "M_LIMIT_EXCEEDED", + "error": "Sending too many delay events" + }, + "url": "" + }, + "message": "the server returned an error: [400 / M_LIMIT_EXCEEDED] Sending too many delay events" + } + }) + ); } #[async_test] From 6b3af4e3e2f86d48357ca7052a0a40cbd4305481 Mon Sep 17 00:00:00 2001 From: Timo Date: Wed, 13 Nov 2024 16:35:37 +0100 Subject: [PATCH 2/2] fix clippy --- crates/matrix-sdk/src/widget/machine/from_widget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/widget/machine/from_widget.rs b/crates/matrix-sdk/src/widget/machine/from_widget.rs index bb535a4fa37..8a65bd58c4b 100644 --- a/crates/matrix-sdk/src/widget/machine/from_widget.rs +++ b/crates/matrix-sdk/src/widget/machine/from_widget.rs @@ -87,7 +87,7 @@ impl From for FromWidgetErrorResponse { impl From<&str> for FromWidgetErrorResponse { fn from(error: &str) -> Self { - error.to_string().into() + error.to_owned().into() } }