From 6befbf725abc244c6e938f3109b90b40c4ebe53a Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 22 Aug 2024 10:45:32 -0700 Subject: [PATCH] rest of the rest of the --- ic-agent/src/agent/agent_test.rs | 112 +++---------------------------- ic-agent/src/agent/builder.rs | 12 ++++ ic-agent/src/agent/mod.rs | 49 ++++++-------- ic-agent/src/lib.rs | 4 -- 4 files changed, 41 insertions(+), 136 deletions(-) diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs index 204aed68..82c5f322 100644 --- a/ic-agent/src/agent/agent_test.rs +++ b/ic-agent/src/agent/agent_test.rs @@ -1,94 +1,37 @@ // Disable these tests without the reqwest feature. -#![cfg(feature = "reqwest")] use self::mock::{ assert_mock, assert_single_mock, assert_single_mock_count, mock, mock_additional, }; -use crate::{ - agent::{http_transport::ReqwestTransport, Status}, - export::Principal, - Agent, AgentError, Certificate, -}; +use crate::{agent::Status, export::Principal, Agent, AgentError, Certificate}; use candid::{Encode, Nat}; use futures_util::FutureExt; use ic_certification::{Delegation, Label}; use ic_transport_types::{ NodeSignature, QueryResponse, RejectCode, RejectResponse, ReplyResponse, TransportCallResponse, }; -use reqwest::Client; use std::{collections::BTreeMap, str::FromStr, sync::Arc, time::Duration}; #[cfg(all(target_family = "wasm", feature = "wasm-bindgen"))] use wasm_bindgen_test::wasm_bindgen_test; -use crate::agent::http_transport::route_provider::{RoundRobinRouteProvider, RouteProvider}; +use crate::agent::route_provider::{RoundRobinRouteProvider, RouteProvider}; #[cfg(all(target_family = "wasm", feature = "wasm-bindgen"))] wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); -fn make_transport(url: &str) -> ReqwestTransport { - let transport = ReqwestTransport::create(url).unwrap(); - #[cfg(feature = "experimental_sync_call")] - { - transport.with_use_call_v3_endpoint() - } - #[cfg(not(feature = "experimental_sync_call"))] - { - transport - } -} - fn make_agent(url: &str) -> Agent { - Agent::builder() - .with_transport(make_transport(url)) - .with_verify_query_signatures(false) - .build() - .unwrap() + let builder = Agent::builder().with_url(url); + #[cfg(feature = "experimental_sync_call")] + let builder = builder.with_call_v3_endpoint(); + builder.with_verify_query_signatures(false).build().unwrap() } fn make_agent_with_route_provider( route_provider: Arc, tcp_retries: usize, ) -> Agent { - let client = Client::builder() - .build() - .expect("Could not create HTTP client."); Agent::builder() - .with_transport( - ReqwestTransport::create_with_client_route(route_provider, client) - .unwrap() - .with_max_tcp_errors_retries(tcp_retries), - ) - .with_verify_query_signatures(false) - .build() - .unwrap() -} - -#[cfg(feature = "hyper")] -fn make_agent_with_hyper_transport_route_provider( - route_provider: Arc, - tcp_retries: usize, -) -> Agent { - use super::http_transport::HyperTransport; - use http_body_util::Full; - use hyper_rustls::{HttpsConnector, HttpsConnectorBuilder}; - use hyper_util::{ - client::legacy::{connect::HttpConnector, Client as LegacyClient}, - rt::TokioExecutor, - }; - use std::collections::VecDeque; - - let connector = HttpsConnectorBuilder::new() - .with_webpki_roots() - .https_or_http() - .enable_http1() - .enable_http2() - .build(); - let client: LegacyClient, Full>> = - LegacyClient::builder(TokioExecutor::new()).build(connector); - let transport = HyperTransport::create_with_service_route(route_provider, client) - .unwrap() - .with_max_tcp_errors_retries(tcp_retries); - Agent::builder() - .with_transport(transport) + .with_arc_route_provider(route_provider) + .with_max_tcp_error_retries(tcp_retries) .with_verify_query_signatures(false) .build() .unwrap() @@ -96,7 +39,7 @@ fn make_agent_with_hyper_transport_route_provider( fn make_untimed_agent(url: &str) -> Agent { Agent::builder() - .with_transport(ReqwestTransport::create(url).unwrap()) + .with_url(url) .with_verify_query_signatures(false) .with_ingress_expiry(Some(Duration::from_secs(u32::MAX as _))) .build() @@ -105,7 +48,7 @@ fn make_untimed_agent(url: &str) -> Agent { fn make_certifying_agent(url: &str) -> Agent { Agent::builder() - .with_transport(ReqwestTransport::create(url).unwrap()) + .with_url(url) .with_ingress_expiry(Some(Duration::from_secs(u32::MAX as _))) .build() .unwrap() @@ -426,41 +369,6 @@ async fn reqwest_client_status_okay_when_request_retried() -> Result<(), AgentEr Ok(()) } -#[cfg_attr(not(target_family = "wasm"), tokio::test)] -#[cfg(feature = "hyper")] -async fn hyper_client_status_okay_when_request_retried() -> Result<(), AgentError> { - let map = BTreeMap::new(); - let response = serde_cbor::Value::Map(map); - let (read_mock, url) = mock( - "GET", - "/api/v2/status", - 200, - serde_cbor::to_vec(&response)?, - Some("application/cbor"), - ) - .await; - // Without retry request should fail. - let non_working_url = "http://127.0.0.1:4444"; - let tcp_retries = 0; - let route_provider = RoundRobinRouteProvider::new(vec![non_working_url, &url]).unwrap(); - let agent = - make_agent_with_hyper_transport_route_provider(Arc::new(route_provider), tcp_retries); - let result = agent.status().await; - assert!(result.is_err()); - - // With retry request should succeed. - let tcp_retries = 1; - let route_provider = RoundRobinRouteProvider::new(vec![non_working_url, &url]).unwrap(); - let agent = - make_agent_with_hyper_transport_route_provider(Arc::new(route_provider), tcp_retries); - let result = agent.status().await; - - assert_mock(read_mock).await; - - assert!(result.is_ok()); - Ok(()) -} - #[cfg_attr(not(target_family = "wasm"), tokio::test)] #[cfg_attr(target_family = "wasm", wasm_bindgen_test)] // test that the agent (re)tries to reach the server. diff --git a/ic-agent/src/agent/builder.rs b/ic-agent/src/agent/builder.rs index 36c2ce62..660b81e5 100644 --- a/ic-agent/src/agent/builder.rs +++ b/ic-agent/src/agent/builder.rs @@ -123,4 +123,16 @@ impl AgentBuilder { self.config.use_call_v3_endpoint = true; self } + + /// Retry up to the specified number of times upon encountering underlying TCP errors. + pub fn with_max_tcp_error_retries(mut self, retries: usize) -> Self { + self.config.max_tcp_error_retries = retries; + self + } + + /// Don't accept HTTP bodies any larger than `max_size` bytes. + pub fn with_max_response_body_size(mut self, max_size: usize) -> Self { + self.config.max_response_body_size = Some(max_size); + self + } } diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs index fe483394..70593c04 100644 --- a/ic-agent/src/agent/mod.rs +++ b/ic-agent/src/agent/mod.rs @@ -1891,10 +1891,10 @@ impl<'agent> IntoFuture for UpdateBuilder<'agent> { } } -#[cfg(all(test, feature = "reqwest", not(target_family = "wasm")))] +#[cfg(all(test, not(target_family = "wasm")))] mod offline_tests { use super::*; - use futures_util::future::pending; + use tokio::net::TcpListener; // Any tests that involve the network should go in agent_test, not here. #[test] @@ -1921,36 +1921,25 @@ mod offline_tests { #[tokio::test] async fn client_ratelimit() { - struct SlowTransport(Arc>); - impl Transport for SlowTransport { - fn call( - &self, - _effective_canister_id: Principal, - _envelope: Vec, - ) -> AgentFuture { - *self.0.lock().unwrap() += 1; - Box::pin(pending()) - } - fn query(&self, _: Principal, _: Vec) -> AgentFuture> { - *self.0.lock().unwrap() += 1; - Box::pin(pending()) - } - fn read_state(&self, _: Principal, _: Vec) -> AgentFuture> { - *self.0.lock().unwrap() += 1; - Box::pin(pending()) - } - fn read_subnet_state(&self, _: Principal, _: Vec) -> AgentFuture> { - *self.0.lock().unwrap() += 1; - Box::pin(pending()) - } - fn status(&self) -> AgentFuture> { - *self.0.lock().unwrap() += 1; - Box::pin(pending()) - } - } + let mock_server = TcpListener::bind("127.0.0.1:0").await.unwrap(); let count = Arc::new(Mutex::new(0)); + let port = mock_server.local_addr().unwrap().port(); + tokio::spawn({ + let count = count.clone(); + async move { + loop { + let (mut conn, _) = mock_server.accept().await.unwrap(); + *count.lock().unwrap() += 1; + tokio::spawn( + // read all data, never reply + async move { tokio::io::copy(&mut conn, &mut tokio::io::sink()).await }, + ); + } + } + }); let agent = Agent::builder() - .with_transport(SlowTransport(count.clone())) + .with_http_client(Client::builder().http1_only().build().unwrap()) + .with_url(format!("http://127.0.0.1:{port}")) .with_max_concurrent_requests(2) .build() .unwrap(); diff --git a/ic-agent/src/lib.rs b/ic-agent/src/lib.rs index bb249132..a333016b 100644 --- a/ic-agent/src/lib.rs +++ b/ic-agent/src/lib.rs @@ -108,10 +108,6 @@ )] #![cfg_attr(not(target_family = "wasm"), warn(clippy::future_not_send))] #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] - -#[cfg(all(feature = "hyper", target_family = "wasm"))] -compile_error!("Feature `hyper` cannot be used from WASM."); - pub mod agent; pub mod export; pub mod identity;