Skip to content

Commit fe6ae8c

Browse files
toger5bnjbvr
authored andcommitted
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): matrix-org/matrix-spec-proposals#2762 (comment) 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: matrix-org/matrix-spec-proposals#2762 (comment) Co-authored-by: Benjamin Bouvier <[email protected]>
1 parent 9c244c3 commit fe6ae8c

File tree

10 files changed

+390
-124
lines changed

10 files changed

+390
-124
lines changed

crates/matrix-sdk/src/test_utils/mocks.rs

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use ruma::{
3131
directory::PublicRoomsChunk,
3232
events::{AnyStateEvent, AnyTimelineEvent, MessageLikeEventType, StateEventType},
3333
serde::Raw,
34+
time::Duration,
3435
MxcUri, OwnedEventId, OwnedRoomId, RoomId, ServerName,
3536
};
3637
use serde::Deserialize;
@@ -952,6 +953,72 @@ impl<'a> MockEndpoint<'a, RoomSendEndpoint> {
952953
}
953954
}
954955

956+
/// Ensures the event was sent as a delayed event.
957+
///
958+
/// Note: works with *any* room.
959+
///
960+
/// # Examples
961+
///
962+
/// see also [`MatrixMockServer::mock_room_send`] for more context.
963+
///
964+
/// ```
965+
/// # tokio_test::block_on(async {
966+
/// use matrix_sdk::{
967+
/// ruma::{
968+
/// api::client::delayed_events::{delayed_message_event, DelayParameters},
969+
/// events::{message::MessageEventContent, AnyMessageLikeEventContent},
970+
/// room_id,
971+
/// time::Duration,
972+
/// TransactionId,
973+
/// },
974+
/// test_utils::mocks::MatrixMockServer,
975+
/// };
976+
/// use serde_json::json;
977+
/// use wiremock::ResponseTemplate;
978+
///
979+
/// let mock_server = MatrixMockServer::new().await;
980+
/// let client = mock_server.client_builder().build().await;
981+
///
982+
/// mock_server.mock_room_state_encryption().plain().mount().await;
983+
///
984+
/// let room = mock_server.sync_joined_room(&client, room_id!("!room_id:localhost")).await;
985+
///
986+
/// mock_server
987+
/// .mock_room_send()
988+
/// .with_delay(Duration::from_millis(500))
989+
/// .respond_with(ResponseTemplate::new(200).set_body_json(json!({"delay_id":"$some_id"})))
990+
/// .mock_once()
991+
/// .mount()
992+
/// .await;
993+
///
994+
/// let response_not_mocked =
995+
/// room.send_raw("m.room.message", json!({ "body": "Hello world" })).await;
996+
///
997+
/// // A non delayed event should not be mocked by the server.
998+
/// assert!(response_not_mocked.is_err());
999+
///
1000+
/// let r = delayed_message_event::unstable::Request::new(
1001+
/// room.room_id().to_owned(),
1002+
/// TransactionId::new(),
1003+
/// DelayParameters::Timeout { timeout: Duration::from_millis(500) },
1004+
/// &AnyMessageLikeEventContent::Message(MessageEventContent::plain("hello world")),
1005+
/// )
1006+
/// .unwrap();
1007+
///
1008+
/// let response = room.client().send(r, None).await.unwrap();
1009+
/// // The delayed `m.room.message` event type should be mocked by the server.
1010+
/// assert_eq!("$some_id", response.delay_id);
1011+
/// # anyhow::Ok(()) });
1012+
/// ```
1013+
pub fn with_delay(self, delay: Duration) -> Self {
1014+
Self {
1015+
mock: self
1016+
.mock
1017+
.and(query_param("org.matrix.msc4140.delay", delay.as_millis().to_string())),
1018+
..self
1019+
}
1020+
}
1021+
9551022
/// Returns a send endpoint that emulates success, i.e. the event has been
9561023
/// sent with the given event id.
9571024
///
@@ -1117,6 +1184,69 @@ impl<'a> MockEndpoint<'a, RoomSendStateEndpoint> {
11171184
Self { mock: self.mock.and(path_regex(Self::generate_path_regexp(&self.endpoint))), ..self }
11181185
}
11191186

1187+
/// Ensures the event was sent as a delayed event.
1188+
///
1189+
/// Note: works with *any* room.
1190+
///
1191+
/// # Examples
1192+
///
1193+
/// see also [`MatrixMockServer::mock_room_send`] for more context.
1194+
///
1195+
/// ```
1196+
/// # tokio_test::block_on(async {
1197+
/// use matrix_sdk::{
1198+
/// ruma::{
1199+
/// api::client::delayed_events::{delayed_state_event, DelayParameters},
1200+
/// events::{room::create::RoomCreateEventContent, AnyStateEventContent},
1201+
/// room_id,
1202+
/// time::Duration,
1203+
/// },
1204+
/// test_utils::mocks::MatrixMockServer,
1205+
/// };
1206+
/// use wiremock::ResponseTemplate;
1207+
/// use serde_json::json;
1208+
///
1209+
/// let mock_server = MatrixMockServer::new().await;
1210+
/// let client = mock_server.client_builder().build().await;
1211+
///
1212+
/// mock_server.mock_room_state_encryption().plain().mount().await;
1213+
///
1214+
/// let room = mock_server.sync_joined_room(&client, room_id!("!room_id:localhost")).await;
1215+
///
1216+
/// mock_server
1217+
/// .mock_room_send_state()
1218+
/// .with_delay(Duration::from_millis(500))
1219+
/// .respond_with(ResponseTemplate::new(200).set_body_json(json!({"delay_id":"$some_id"})))
1220+
/// .mock_once()
1221+
/// .mount()
1222+
/// .await;
1223+
///
1224+
/// let response_not_mocked = room.send_state_event(RoomCreateEventContent::new_v11()).await;
1225+
/// // A non delayed event should not be mocked by the server.
1226+
/// assert!(response_not_mocked.is_err());
1227+
///
1228+
/// let r = delayed_state_event::unstable::Request::new(
1229+
/// room.room_id().to_owned(),
1230+
/// "".to_owned(),
1231+
/// DelayParameters::Timeout { timeout: Duration::from_millis(500) },
1232+
/// &AnyStateEventContent::RoomCreate(RoomCreateEventContent::new_v11()),
1233+
/// )
1234+
/// .unwrap();
1235+
/// let response = room.client().send(r, None).await.unwrap();
1236+
/// // The delayed `m.room.message` event type should be mocked by the server.
1237+
/// assert_eq!("$some_id", response.delay_id);
1238+
///
1239+
/// # anyhow::Ok(()) });
1240+
/// ```
1241+
pub fn with_delay(self, delay: Duration) -> Self {
1242+
Self {
1243+
mock: self
1244+
.mock
1245+
.and(query_param("org.matrix.msc4140.delay", delay.as_millis().to_string())),
1246+
..self
1247+
}
1248+
}
1249+
11201250
///
11211251
/// ```
11221252
/// # tokio_test::block_on(async {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,11 @@ where
7474
Self { request_meta: None, _phantom: PhantomData }
7575
}
7676

77+
/// Setup a callback function that will be called once the matrix driver has
78+
/// processed the request.
7779
pub(crate) fn then(
7880
self,
79-
response_handler: impl FnOnce(Result<T, String>, &mut WidgetMachine) -> Vec<Action>
81+
response_handler: impl FnOnce(Result<T, crate::Error>, &mut WidgetMachine) -> Vec<Action>
8082
+ Send
8183
+ 'static,
8284
) {

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

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use std::fmt;
16-
15+
use as_variant::as_variant;
1716
use ruma::{
18-
api::client::delayed_events::{
19-
delayed_message_event, delayed_state_event, update_delayed_event,
17+
api::client::{
18+
delayed_events::{delayed_message_event, delayed_state_event, update_delayed_event},
19+
error::{ErrorBody, StandardErrorBody},
2020
},
2121
events::{AnyTimelineEvent, MessageLikeEventType, StateEventType},
2222
serde::Raw,
@@ -25,7 +25,7 @@ use ruma::{
2525
use serde::{Deserialize, Serialize};
2626

2727
use super::{SendEventRequest, UpdateDelayedEventRequest};
28-
use crate::widget::StateKeySelector;
28+
use crate::{widget::StateKeySelector, Error, HttpError, RumaApiError};
2929

3030
#[derive(Deserialize, Debug)]
3131
#[serde(tag = "action", rename_all = "snake_case", content = "data")]
@@ -41,28 +41,85 @@ pub(super) enum FromWidgetRequest {
4141
DelayedEventUpdate(UpdateDelayedEventRequest),
4242
}
4343

44+
/// The full response a client sends to a [`FromWidgetRequest`] in case of an
45+
/// error.
4446
#[derive(Serialize)]
4547
pub(super) struct FromWidgetErrorResponse {
4648
error: FromWidgetError,
4749
}
4850

4951
impl FromWidgetErrorResponse {
50-
pub(super) fn new(e: impl fmt::Display) -> Self {
51-
Self { error: FromWidgetError { message: e.to_string() } }
52+
/// Create a error response to send to the widget from an http error.
53+
pub(crate) fn from_http_error(error: HttpError) -> Self {
54+
let message = error.to_string();
55+
let matrix_api_error = as_variant!(error, HttpError::Api(ruma::api::error::FromHttpResponseError::Server(RumaApiError::ClientApi(err))) => err);
56+
57+
Self {
58+
error: FromWidgetError {
59+
message,
60+
matrix_api_error: matrix_api_error.and_then(|api_error| match api_error.body {
61+
ErrorBody::Standard { kind, message } => Some(FromWidgetMatrixErrorBody {
62+
http_status: api_error.status_code.as_u16().into(),
63+
response: StandardErrorBody { kind, message },
64+
}),
65+
_ => None,
66+
}),
67+
},
68+
}
69+
}
70+
71+
/// Create a error response to send to the widget from a matrix sdk error.
72+
pub(crate) fn from_error(error: Error) -> Self {
73+
match error {
74+
Error::Http(e) => FromWidgetErrorResponse::from_http_error(e),
75+
// For UnknownError's we do not want to have the `unknown error` bit in the message.
76+
// Hence we only convert the inner error to a string.
77+
Error::UnknownError(e) => FromWidgetErrorResponse::from_string(e.to_string()),
78+
_ => FromWidgetErrorResponse::from_string(error.to_string()),
79+
}
80+
}
81+
82+
/// Create a error response to send to the widget from a string.
83+
pub(crate) fn from_string<S: Into<String>>(error: S) -> Self {
84+
Self { error: FromWidgetError { message: error.into(), matrix_api_error: None } }
5285
}
5386
}
5487

88+
/// Serializable section of an error response send by the client as a
89+
/// response to a [`FromWidgetRequest`].
5590
#[derive(Serialize)]
5691
struct FromWidgetError {
92+
/// Unspecified error message text that caused this widget action to
93+
/// fail.
94+
///
95+
/// This is useful to prompt the user on an issue but cannot be used to
96+
/// decide on how to deal with the error.
5797
message: String,
98+
99+
/// Optional matrix error hinting at workarounds for specific errors.
100+
matrix_api_error: Option<FromWidgetMatrixErrorBody>,
101+
}
102+
103+
/// Serializable section of a widget response that represents a matrix error.
104+
#[derive(Serialize)]
105+
struct FromWidgetMatrixErrorBody {
106+
/// Status code of the http response.
107+
http_status: u32,
108+
109+
/// Standard error response including the `errorcode` and the `error`
110+
/// message as defined in the [spec](https://spec.matrix.org/v1.12/client-server-api/#standard-error-response).
111+
response: StandardErrorBody,
58112
}
59113

114+
/// The serializable section of a widget response containing the supported
115+
/// versions.
60116
#[derive(Serialize)]
61117
pub(super) struct SupportedApiVersionsResponse {
62118
supported_versions: Vec<ApiVersion>,
63119
}
64120

65121
impl SupportedApiVersionsResponse {
122+
/// The currently supported widget api versions from the rust widget driver.
66123
pub(super) fn new() -> Self {
67124
Self {
68125
supported_versions: vec![

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@ pub(crate) enum IncomingMessage {
3737
/// The ID of the request that this response corresponds to.
3838
request_id: Uuid,
3939

40-
/// The result of the request: response data or error message.
41-
response: Result<MatrixDriverResponse, String>,
40+
/// Result of the request: the response data, or a matrix sdk error.
41+
///
42+
/// Http errors will be forwarded to the widget in a specified format so
43+
/// the widget can parse the error.
44+
response: Result<MatrixDriverResponse, crate::Error>,
4245
},
4346

4447
/// The `MatrixDriver` notified the `WidgetMachine` of a new matrix event.

0 commit comments

Comments
 (0)