fix: log backend send error and fixing unit tests

G-Core · Feb 20, 2025 · 3dfc571 · 3dfc571
1 parent 8495180
commit 3dfc571
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 46 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -35,7 +35,7 @@ jobs:
         run: cargo build --all-features
 
       - name: Unit Tests
-        run: cargo test --all --exclude http-backend
+        run: cargo test --all
 
       - name: Check formatting
         run: cargo fmt --all -- --check

diff --git a/crates/http-backend/Cargo.toml b/crates/http-backend/Cargo.toml
@@ -22,4 +22,4 @@ smol_str = {workspace = true}
 [dev-dependencies]
 claims = "0.8"
 tracing-test = "0.2"
-mock-http-connector = "0.3"
+mock-http-connector = { git = "https://github.com/nmoutschen/mock-http-connector.git", branch = "hyper-1.0", default-features = false, features = ["hyper_1"]}
diff --git a/crates/http-backend/src/lib.rs b/crates/http-backend/src/lib.rs
@@ -13,7 +13,6 @@ use hyper_util::client::legacy::connect::{Connect, HttpConnector};
 use hyper_util::client::legacy::Client;
 use hyper_util::rt::TokioExecutor;
 use pin_project::pin_project;
-use smol_str::{SmolStr, ToSmolStr};
 use tokio::net::TcpStream;
 use tower_service::Service;
 use tracing::{debug, trace, warn};
@@ -24,7 +23,7 @@ use reactor::gcore::fastedge::{
     http_client::Host,
 };
 
-type HeaderNameList = Vec<SmolStr>;
+type HeaderNameList = Vec<HeaderName>;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum BackendStrategy {
@@ -112,8 +111,8 @@ impl<C> Backend<C> {
         self.uri.to_owned()
     }
 
-    pub fn propagate_header_names(&self) -> Vec<SmolStr> {
-        self.propagate_header_names.to_owned()
+    pub fn propagate_header_names(&self) -> HeaderNameList {
+        self.propagate_header_names.clone()
     }
 
     /// Propagate filtered headers from original requests
@@ -132,7 +131,7 @@ impl<C> Backend<C> {
         }
         let headers = headers.into_iter().filter(|(k, _)| {
             if let Some(name) = k {
-                self.propagate_header_names.contains(&name.to_smolstr())
+                self.propagate_header_names.contains(name)
             } else {
                 false
             }
@@ -227,7 +226,7 @@ impl<C> Backend<C> {
                         !self
                             .propagate_header_names
                             .iter()
-                            .any(|name| name.eq_ignore_ascii_case(k))
+                            .any(|name| name.eq(k.as_str()))
                     })
                     .collect::<Vec<(String, String)>>();
 
@@ -279,14 +278,14 @@ where
             self.max_sub_requests -= 1;
         }
 
-        let request = self
-            .make_request(req)
-            .map_err(|_| HttpError::RequestError)?;
-        let res = self
-            .client
-            .request(request)
-            .await
-            .map_err(|_| HttpError::RequestError)?;
+        let request = self.make_request(req).map_err(|error| {
+            warn!(cause=?error, "making request to backend");
+            HttpError::RequestError
+        })?;
+        let res = self.client.request(request).await.map_err(|error| {
+            warn!(cause=?error, "sending request to backend");
+            HttpError::RequestError
+        })?;
 
         let status = res.status().as_u16();
         let (parts, body) = res.into_parts();
@@ -311,7 +310,10 @@ where
         let body_bytes = body
             .collect()
             .await
-            .map_err(|_| HttpError::RequestError)?
+            .map_err(|error| {
+                warn!(cause=?error, "receiving body from backend");
+                HttpError::RequestError
+            })?
             .to_bytes();
         let body = Some(body_bytes.to_vec());
 
@@ -494,15 +496,14 @@ mod tests {
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://example.com/path".to_string(),
             headers: vec![("header01".to_string(), "01".to_string())],
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -527,15 +528,14 @@ mod tests {
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://example.com/path".to_string(),
             headers: vec![("header01".to_string(), "01".to_string())],
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::REQUEST_TIMEOUT, res.status);
     }
 
@@ -560,7 +560,7 @@ mod tests {
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "/path".to_string(),
@@ -571,7 +571,6 @@ mod tests {
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -599,7 +598,6 @@ mod tests {
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -623,7 +621,7 @@ mod tests {
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://rust-lang.org".to_string(),
@@ -638,7 +636,6 @@ mod tests {
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -661,7 +658,7 @@ mod tests {
         let connector = builder.build();
         let mut backend =
             Backend::<mock_http_connector::Connector>::builder(BackendStrategy::FastEdge)
-                .propagate_headers_names(vec!["Propagate-Header".to_string()])
+                .propagate_headers_names(vec!["Propagate-Header".parse().unwrap()])
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
@@ -670,15 +667,14 @@ mod tests {
             claims::assert_ok!("VALUE".try_into()),
         );
         headers.insert("Propagate-Header", claims::assert_ok!("VALUE".try_into()));
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://example.com".to_string(),
             headers: vec![("header01".to_string(), "01".to_string())],
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -700,21 +696,20 @@ mod tests {
         let connector = builder.build();
         let mut backend =
             Backend::<mock_http_connector::Connector>::builder(BackendStrategy::FastEdge)
-                .propagate_headers_names(vec!["Propagate-Header".to_string()])
+                .propagate_headers_names(vec!["Propagate-Header".parse().unwrap()])
                 .uri(assert_ok!("http://be.server/backend_path/".parse()))
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
 
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://example.com/path/".to_string(),
             headers: vec![("header01".to_string(), "01".to_string())],
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
     }
 
@@ -736,29 +731,26 @@ mod tests {
         let connector = builder.build();
         let mut backend =
             Backend::<mock_http_connector::Connector>::builder(BackendStrategy::FastEdge)
-                .propagate_headers_names(vec!["Propagate-Header".to_string()])
+                .propagate_headers_names(vec!["Propagate-Header".parse().unwrap()])
                 .max_sub_requests(2)
                 .build(connector);
         let mut headers = HeaderMap::new();
         headers.insert("Server_name", claims::assert_ok!("server".try_into()));
 
-        claims::assert_ok!(backend.propagate_headers(&headers));
+        claims::assert_ok!(backend.propagate_headers(headers));
         let req = Request {
             method: Method::Get,
             uri: "http://example.com/".to_string(),
             headers: vec![("header01".to_string(), "01".to_string())],
             body: None,
         };
         let res = claims::assert_ok!(backend.send_request(req.clone()).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
 
         let res = claims::assert_ok!(backend.send_request(req.clone()).await);
-        let res = claims::assert_ok!(res);
         assert_eq!(http::StatusCode::OK, res.status);
 
-        let res = claims::assert_ok!(backend.send_request(req).await);
-        let res = claims::assert_err!(res);
-        assert_eq!("too-many-requests", res.name());
+        let error = claims::assert_err!(backend.send_request(req).await);
+        assert_eq!("too-many-requests", error.name());
     }
 }
diff --git a/crates/http-service/src/executor/wasi_http.rs b/crates/http-service/src/executor/wasi_http.rs
@@ -13,7 +13,6 @@ use http_body_util::{BodyExt, Full};
 use hyper::body::Body;
 use runtime::{store::StoreBuilder, InstancePre};
 use secret::{Secret, SecretStrategy};
-use smol_str::ToSmolStr;
 use wasmtime_wasi_http::bindings::http::types::Scheme;
 use wasmtime_wasi_http::bindings::ProxyPre;
 use wasmtime_wasi_http::{body::HyperOutgoingBody, WasiHttpView};
@@ -88,7 +87,7 @@ where
             .headers
             .iter()
             .filter_map(|(k, v)| {
-                if propagate_header_names.contains(&k.to_smolstr()) {
+                if propagate_header_names.contains(k) {
                     Some((k.to_owned(), v.to_owned()))
                 } else {
                     None

diff --git a/crates/http-service/src/state.rs b/crates/http-service/src/state.rs
@@ -6,14 +6,13 @@ use http::{header, HeaderMap, HeaderName, Uri};
 use http_backend::Backend;
 use runtime::BackendRequest;
 use secret::{Secret, SecretStrategy};
-use smol_str::{SmolStr, ToSmolStr};
 use tracing::instrument;
 
 pub struct HttpState<C, T: SecretStrategy> {
     pub(super) http_backend: Backend<C>,
     pub(super) uri: Uri,
     pub(super) propagate_headers: HeaderMap,
-    pub(super) propagate_header_names: Vec<SmolStr>,
+    pub(super) propagate_header_names: Vec<HeaderName>,
     pub(super) dictionary: Dictionary,
     pub(super) secret: Secret<T>,
 }
@@ -59,8 +58,7 @@ impl<C, T: SecretStrategy> BackendRequest for HttpState<C, T> {
             .into_iter()
             .filter_map(|(k, v)| k.map(|k| (k, v)))
             .filter(|(k, _)| {
-                !FILTER_HEADERS.contains(k)
-                    && !self.propagate_header_names.contains(&k.to_smolstr())
+                !FILTER_HEADERS.contains(k) && !self.propagate_header_names.contains(k)
             })
             .collect::<HeaderMap>();
 

diff --git a/src/main.rs b/src/main.rs
@@ -108,7 +108,12 @@ async fn main() -> anyhow::Result<()> {
             let mut builder =
                 Backend::<HttpsConnector<HttpConnector>>::builder(BackendStrategy::Direct);
 
-            builder.propagate_headers_names(run.propagate_headers);
+            builder.propagate_headers_names(
+                run.propagate_headers
+                    .into_iter()
+                    .filter_map(|h| h.parse().ok())
+                    .collect(),
+            );
 
             let backend = builder.build(backend_connector);
             let mut secrets = vec![];