From 3dfc5711c60ba11488d04c362a3cfbd15ae24b6c Mon Sep 17 00:00:00 2001 From: Ruslan Pislari Date: Fri, 7 Feb 2025 09:35:45 +0200 Subject: [PATCH] fix: log backend send error and fixing unit tests --- .github/workflows/ci.yaml | 2 +- crates/http-backend/Cargo.toml | 2 +- crates/http-backend/src/lib.rs | 66 ++++++++----------- crates/http-service/src/executor/wasi_http.rs | 3 +- crates/http-service/src/state.rs | 6 +- src/main.rs | 7 +- 6 files changed, 40 insertions(+), 46 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fc1c663..bc0f6be 100644 --- 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 index e4273e5..6214312 100644 --- 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 index 08c4a80..1c4b38c 100644 --- 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; +type HeaderNameList = Vec; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum BackendStrategy { @@ -112,8 +111,8 @@ impl Backend { self.uri.to_owned() } - pub fn propagate_header_names(&self) -> Vec { - 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 Backend { } 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 Backend { !self .propagate_header_names .iter() - .any(|name| name.eq_ignore_ascii_case(k)) + .any(|name| name.eq(k.as_str())) }) .collect::>(); @@ -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,7 +496,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://example.com/path".to_string(), @@ -502,7 +504,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); } @@ -527,7 +528,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://example.com/path".to_string(), @@ -535,7 +536,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::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::::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,7 +667,7 @@ 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(), @@ -678,7 +675,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); } @@ -700,13 +696,13 @@ mod tests { let connector = builder.build(); let mut backend = Backend::::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(), @@ -714,7 +710,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); } @@ -736,13 +731,13 @@ mod tests { let connector = builder.build(); let mut backend = Backend::::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(), @@ -750,15 +745,12 @@ mod tests { 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 index 86a0cd2..5eeb2fb 100644 --- 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 index 74d2a2c..e7c4076 100644 --- 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 { pub(super) http_backend: Backend, pub(super) uri: Uri, pub(super) propagate_headers: HeaderMap, - pub(super) propagate_header_names: Vec, + pub(super) propagate_header_names: Vec, pub(super) dictionary: Dictionary, pub(super) secret: Secret, } @@ -59,8 +58,7 @@ impl BackendRequest for HttpState { .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::(); diff --git a/src/main.rs b/src/main.rs index 593f09a..e9adf16 100644 --- a/src/main.rs +++ b/src/main.rs @@ -108,7 +108,12 @@ async fn main() -> anyhow::Result<()> { let mut builder = Backend::>::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![];