Merge branch 'dont-expect-reqwest-http-request' into 'next'

fix: do not expect that all http requests are valid reqwest requests

Closes #396

See merge request famedly/conduit!611

src/api/appservice_server.rs

@@ -13,11 +13,16 @@ use tracing::warn;
 pub(crate) async fn send_request<T: OutgoingRequest>(
     registration: Registration,
     request: T,
-) -> Option<Result<T::IncomingResponse>>
+) -> Result<Option<T::IncomingResponse>>
 where
     T: Debug,
 {
-    let destination = registration.url?;
+    let destination = match registration.url {
+        Some(url) => url,
+        None => {
+            return Ok(None);
+        }
+    };
 
     let hs_token = registration.hs_token.as_str();
 
@@ -45,8 +50,7 @@ where
     );
     *http_request.uri_mut() = parts.try_into().expect("our manipulation is always valid");
 
-    let mut reqwest_request = reqwest::Request::try_from(http_request)
-        .expect("all http requests are valid reqwest requests");
+    let mut reqwest_request = reqwest::Request::try_from(http_request)?;
 
     *reqwest_request.timeout_mut() = Some(Duration::from_secs(30));
 
@@ -63,7 +67,7 @@ where
                 "Could not send request to appservice {:?} at {}: {}",
                 registration.id, destination, e
             );
-            return Some(Err(e.into()));
+            return Err(e.into());
         }
     };
 
@@ -100,11 +104,11 @@ where
             .expect("reqwest body is valid http body"),
     );
 
-    Some(response.map_err(|_| {
+    response.map(Some).map_err(|_| {
         warn!(
             "Appservice returned invalid response bytes {}\n{}",
             destination, url
         );
         Error::BadServerResponse("Server returned bad response.")
-    }))
+    })
 }

src/api/client_server/alias.rs

@@ -102,20 +102,18 @@ pub(crate) async fn get_alias_helper(
         None => {
             for appservice in services().appservice.read().await.values() {
                 if appservice.aliases.is_match(room_alias.as_str())
-                    && if let Some(opt_result) = services()
-                        .sending
-                        .send_appservice_request(
-                            appservice.registration.clone(),
-                            appservice::query::query_room_alias::v1::Request {
-                                room_alias: room_alias.clone(),
-                            },
-                        )
-                        .await
-                    {
-                        opt_result.is_ok()
-                    } else {
-                        false
-                    }
+                    && matches!(
+                        services()
+                            .sending
+                            .send_appservice_request(
+                                appservice.registration.clone(),
+                                appservice::query::query_room_alias::v1::Request {
+                                    room_alias: room_alias.clone(),
+                                },
+                            )
+                            .await,
+                        Ok(Some(_opt_result))
+                    )
                 {
                     room_id = Some(
                         services()

src/api/server_server.rs

@@ -233,8 +233,7 @@ where
         }
     }
 
-    let reqwest_request = reqwest::Request::try_from(http_request)
-        .expect("all http requests are valid reqwest requests");
+    let reqwest_request = reqwest::Request::try_from(http_request)?;
 
     let url = reqwest_request.url().clone();
 

src/service/pusher/mod.rs

@@ -66,8 +66,7 @@ impl Service {
             })?
             .map(|body| body.freeze());
 
-        let reqwest_request = reqwest::Request::try_from(http_request)
-            .expect("all http requests are valid reqwest requests");
+        let reqwest_request = reqwest::Request::try_from(http_request)?;
 
         // TODO: we could keep this very short and let expo backoff do it's thing...
         //*reqwest_request.timeout_mut() = Some(Duration::from_secs(5));

src/service/sending/mod.rs

@@ -512,10 +512,8 @@ impl Service {
                 )
                 .await
                 {
-                    None => Ok(kind.clone()),
-                    Some(op_resp) => op_resp
-                        .map(|_response| kind.clone())
-                        .map_err(|e| (kind.clone(), e)),
+                    Ok(_) => Ok(kind.clone()),
+                    Err(e) => Err((kind.clone(), e)),
                 };
 
                 drop(permit);
@@ -710,7 +708,7 @@ impl Service {
         &self,
         registration: Registration,
         request: T,
-    ) -> Option<Result<T::IncomingResponse>>
+    ) -> Result<Option<T::IncomingResponse>>
     where
         T: Debug,
     {