From f9af7c2629bfe7ef1290efc516f4c57180ae6549 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 4 May 2024 09:17:40 -0400 Subject: [PATCH 1/2] refactor: make hyper and reqwest more similar. add tracing --- crates/transport-http/Cargo.toml | 4 +- crates/transport-http/src/hyper.rs | 76 +++++++++++++++++----------- crates/transport-http/src/reqwest.rs | 50 +++++++++++++----- 3 files changed, 86 insertions(+), 44 deletions(-) diff --git a/crates/transport-http/Cargo.toml b/crates/transport-http/Cargo.toml index 818172343e4..7782f31a682 100644 --- a/crates/transport-http/Cargo.toml +++ b/crates/transport-http/Cargo.toml @@ -20,6 +20,7 @@ serde_json = { workspace = true, optional = true } tower = { workspace = true, optional = true } reqwest = { workspace = true, features = ["json"], optional = true } +tracing = { workspace = true, optional = true } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] http-body-util = { workspace = true, optional = true } @@ -28,7 +29,7 @@ hyper-util = { workspace = true, features = ["full"], optional = true } [features] default = ["reqwest", "reqwest-default-tls"] -reqwest = ["dep:reqwest", "dep:alloy-json-rpc", "dep:serde_json", "dep:tower"] +reqwest = ["dep:reqwest", "dep:alloy-json-rpc", "dep:serde_json", "dep:tower", "dep:tracing"] hyper = [ "dep:hyper", "dep:hyper-util", @@ -36,6 +37,7 @@ hyper = [ "dep:alloy-json-rpc", "dep:serde_json", "dep:tower", + "dep:tracing" ] reqwest-default-tls = ["reqwest?/default-tls"] reqwest-native-tls = ["reqwest?/native-tls"] diff --git a/crates/transport-http/src/hyper.rs b/crates/transport-http/src/hyper.rs index 2590b6b667d..6b98ba1cbe4 100644 --- a/crates/transport-http/src/hyper.rs +++ b/crates/transport-http/src/hyper.rs @@ -9,6 +9,7 @@ use hyper::{ use hyper_util::client::legacy::{connect::Connect, Client}; use std::task; use tower::Service; +use tracing::{debug, debug_span, trace, Instrument}; impl Http>> where @@ -18,40 +19,57 @@ where /// Make a request. fn request_hyper(&self, req: RequestPacket) -> TransportFut<'static> { let this = self.clone(); - Box::pin(async move { - let ser = req.serialize().map_err(TransportError::ser_err)?; + let span = debug_span!("HyperTransport", url = %self.url); + Box::pin( + async move { + debug!(count = req.len(), "sending request packet to server"); + let ser = req.serialize().map_err(TransportError::ser_err)?; + // convert the Box into a hyper request + let body = Full::from(Bytes::from(>::from(>::from(ser)))); + let req = hyper::Request::builder() + .method(hyper::Method::POST) + .uri(this.url.as_str()) + .header( + header::CONTENT_TYPE, + header::HeaderValue::from_static("application/json"), + ) + .body(body) + .expect("request parts are valid"); - // convert the Box into a hyper request - let body = Full::from(Bytes::from(>::from(>::from(ser)))); - let req = hyper::Request::builder() - .method(hyper::Method::POST) - .uri(this.url.as_str()) - .header(header::CONTENT_TYPE, header::HeaderValue::from_static("application/json")) - .body(body) - .expect("request parts are valid"); + let resp = this.client.request(req).await.map_err(TransportErrorKind::custom)?; + let status = resp.status(); - let resp = this.client.request(req).await.map_err(TransportErrorKind::custom)?; - let status = resp.status(); + debug!(%status, "received response from server"); - // Unpack data from the response body. We do this regardless of the status code, as we - // want to return the error in the body if there is one. - let body = - resp.into_body().collect().await.map_err(TransportErrorKind::custom)?.to_bytes(); + // Unpack data from the response body. We do this regardless of + // the status code, as we want to return the error in the body + // if there is one. + let body = resp + .into_body() + .collect() + .await + .map_err(TransportErrorKind::custom)? + .to_bytes(); - if status != hyper::StatusCode::OK { - return Err(TransportErrorKind::custom_str(&format!( - "HTTP error {status} with body: {}", - String::from_utf8_lossy(&body) - ))); - } + debug!(bytes = body.len(), "retrieved response body"); + trace!(body = %String::from_utf8_lossy(&body), "response body"); + + if status != hyper::StatusCode::OK { + return Err(TransportErrorKind::custom_str(&format!( + "HTTP error {status} with body: {}", + String::from_utf8_lossy(&body) + ))); + } - // Deser a Box from the body. If deser fails, return the - // body as a string in the error. If the body is not UTF8, this will - // fail and give the empty string in the error. - serde_json::from_slice(&body).map_err(|err| { - TransportError::deser_err(err, String::from_utf8_lossy(body.as_ref())) - }) - }) + // Deser a Box from the body. If deser fails, return + // the body as a string in the error. The conversion to String + // is lossy and may not cover all the bytes in the body. + serde_json::from_slice(&body).map_err(|err| { + TransportError::deser_err(err, String::from_utf8_lossy(body.as_ref())) + }) + } + .instrument(span), + ) } } diff --git a/crates/transport-http/src/reqwest.rs b/crates/transport-http/src/reqwest.rs index 40fd9cacc71..7740d0c0677 100644 --- a/crates/transport-http/src/reqwest.rs +++ b/crates/transport-http/src/reqwest.rs @@ -1,9 +1,9 @@ use crate::Http; use alloy_json_rpc::{RequestPacket, ResponsePacket}; use alloy_transport::{TransportError, TransportErrorKind, TransportFut}; -use reqwest::Response; use std::task; use tower::Service; +use tracing::{debug, debug_span, trace, Instrument}; use url::Url; impl Http { @@ -15,21 +15,43 @@ impl Http { /// Make a request. fn request_reqwest(&self, req: RequestPacket) -> TransportFut<'static> { let this = self.clone(); - Box::pin(async move { - let resp = this - .client - .post(this.url) - .json(&req) - .send() - .await - .and_then(Response::error_for_status) - .map_err(TransportErrorKind::custom)?; + let span: tracing::Span = debug_span!("ReqwestTransport", url = %self.url); + Box::pin( + async move { + let resp = this + .client + .post(this.url) + .json(&req) + .send() + .await + .map_err(TransportErrorKind::custom)?; + let status = resp.status(); - let body = resp.bytes().await.map_err(TransportErrorKind::custom)?; + debug!(%status, "received response from server"); - serde_json::from_slice(&body) - .map_err(|err| TransportError::deser_err(err, String::from_utf8_lossy(&body))) - }) + // Unpack data from the response body. We do this regardless of + // the status code, as we want to return the error in the body + // if there is one. + let body = resp.bytes().await.map_err(TransportErrorKind::custom)?; + + debug!(bytes = body.len(), "retrieved response body"); + trace!(body = %String::from_utf8_lossy(&body), "response body"); + + if status != reqwest::StatusCode::OK { + return Err(TransportErrorKind::custom_str(&format!( + "HTTP error {status} with body: {}", + String::from_utf8_lossy(&body) + ))); + } + + // Deser a Box from the body. If deser fails, return + // the body as a string in the error. The conversion to String + // is lossy and may not cover all the bytes in the body. + serde_json::from_slice(&body) + .map_err(|err| TransportError::deser_err(err, String::from_utf8_lossy(&body))) + } + .instrument(span), + ) } } From dde8a8d63df32dd6bcd0305e2504b0d4f07bbba8 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 4 May 2024 09:20:03 -0400 Subject: [PATCH 2/2] nit: add note about trace level --- crates/transport-http/src/hyper.rs | 2 +- crates/transport-http/src/reqwest.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/transport-http/src/hyper.rs b/crates/transport-http/src/hyper.rs index 6b98ba1cbe4..6d930028a68 100644 --- a/crates/transport-http/src/hyper.rs +++ b/crates/transport-http/src/hyper.rs @@ -51,7 +51,7 @@ where .map_err(TransportErrorKind::custom)? .to_bytes(); - debug!(bytes = body.len(), "retrieved response body"); + debug!(bytes = body.len(), "retrieved response body. Use `trace` for full body"); trace!(body = %String::from_utf8_lossy(&body), "response body"); if status != hyper::StatusCode::OK { diff --git a/crates/transport-http/src/reqwest.rs b/crates/transport-http/src/reqwest.rs index 7740d0c0677..343da4eb59a 100644 --- a/crates/transport-http/src/reqwest.rs +++ b/crates/transport-http/src/reqwest.rs @@ -34,7 +34,7 @@ impl Http { // if there is one. let body = resp.bytes().await.map_err(TransportErrorKind::custom)?; - debug!(bytes = body.len(), "retrieved response body"); + debug!(bytes = body.len(), "retrieved response body. Use `trace` for full body"); trace!(body = %String::from_utf8_lossy(&body), "response body"); if status != reqwest::StatusCode::OK {