From a285c8d3becf5812b03f9acaaaa09c8be0002609 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Mon, 11 May 2026 16:11:59 +0530 Subject: [PATCH] feat(http): add lazy HTTP client initialization --- crates/forge_infra/src/http.rs | 162 +++++++++++--------- crates/forge_infra/src/mcp_client.rs | 48 +++--- crates/forge_tracker/src/collect/posthog.rs | 26 ++-- 3 files changed, 128 insertions(+), 108 deletions(-) diff --git a/crates/forge_infra/src/http.rs b/crates/forge_infra/src/http.rs index 228cc902ef..db126482ce 100644 --- a/crates/forge_infra/src/http.rs +++ b/crates/forge_infra/src/http.rs @@ -1,6 +1,6 @@ use std::fs; use std::path::PathBuf; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; use std::time::Duration; use anyhow::Context; @@ -19,7 +19,8 @@ const VERSION: &str = match option_env!("APP_VERSION") { }; pub struct ForgeHttpInfra { - client: Client, + client: OnceLock, + config: ForgeConfig, debug_requests: Option, file: Arc, } @@ -36,88 +37,103 @@ fn to_reqwest_tls(tls: TlsVersion) -> reqwest::tls::Version { impl ForgeHttpInfra { /// Creates a new [`ForgeHttpInfra`] from a resolved [`ForgeConfig`]. + /// + /// The underlying HTTP client is built lazily on first use. pub fn new(config: ForgeConfig, file_writer: Arc) -> Self { - let http = config.http.unwrap_or(forge_config::HttpConfig { - connect_timeout_secs: 30, - read_timeout_secs: 900, - pool_idle_timeout_secs: 90, - pool_max_idle_per_host: 5, - max_redirects: 10, - hickory: false, - tls_backend: TlsBackend::Default, - min_tls_version: None, - max_tls_version: None, - adaptive_window: true, - keep_alive_interval_secs: Some(60), - keep_alive_timeout_secs: 10, - keep_alive_while_idle: true, - accept_invalid_certs: false, - root_cert_paths: None, - }); - - let mut client = reqwest::Client::builder() - .connect_timeout(Duration::from_secs(http.connect_timeout_secs)) - .read_timeout(Duration::from_secs(http.read_timeout_secs)) - .pool_idle_timeout(Duration::from_secs(http.pool_idle_timeout_secs)) - .pool_max_idle_per_host(http.pool_max_idle_per_host) - .redirect(Policy::limited(http.max_redirects)) - .hickory_dns(http.hickory) - // HTTP/2 configuration from config - .http2_adaptive_window(http.adaptive_window) - .http2_keep_alive_interval(http.keep_alive_interval_secs.map(Duration::from_secs)) - .http2_keep_alive_timeout(Duration::from_secs(http.keep_alive_timeout_secs)) - .http2_keep_alive_while_idle(http.keep_alive_while_idle); - - // Add root certificates from config - if let Some(ref cert_paths) = http.root_cert_paths { - for cert_path in cert_paths { - match fs::read(cert_path) { - Ok(buf) => { - if let Ok(cert) = Certificate::from_pem(&buf) { - client = client.add_root_certificate(cert); - } else if let Ok(cert) = Certificate::from_der(&buf) { - client = client.add_root_certificate(cert); - } else { + Self { + debug_requests: config.debug_requests.clone(), + client: OnceLock::new(), + config, + file: file_writer, + } + } + + /// Returns a reference to the underlying [`Client`], building it on first + /// call. + fn client(&self) -> &Client { + self.client.get_or_init(|| { + let http = self + .config + .http + .clone() + .unwrap_or(forge_config::HttpConfig { + connect_timeout_secs: 30, + read_timeout_secs: 900, + pool_idle_timeout_secs: 90, + pool_max_idle_per_host: 5, + max_redirects: 10, + hickory: false, + tls_backend: TlsBackend::Default, + min_tls_version: None, + max_tls_version: None, + adaptive_window: true, + keep_alive_interval_secs: Some(60), + keep_alive_timeout_secs: 10, + keep_alive_while_idle: true, + accept_invalid_certs: false, + root_cert_paths: None, + }); + + let mut client = reqwest::Client::builder() + .connect_timeout(Duration::from_secs(http.connect_timeout_secs)) + .read_timeout(Duration::from_secs(http.read_timeout_secs)) + .pool_idle_timeout(Duration::from_secs(http.pool_idle_timeout_secs)) + .pool_max_idle_per_host(http.pool_max_idle_per_host) + .redirect(Policy::limited(http.max_redirects)) + .hickory_dns(http.hickory) + // HTTP/2 configuration from config + .http2_adaptive_window(http.adaptive_window) + .http2_keep_alive_interval(http.keep_alive_interval_secs.map(Duration::from_secs)) + .http2_keep_alive_timeout(Duration::from_secs(http.keep_alive_timeout_secs)) + .http2_keep_alive_while_idle(http.keep_alive_while_idle); + + // Add root certificates from config + if let Some(ref cert_paths) = http.root_cert_paths { + for cert_path in cert_paths { + match fs::read(cert_path) { + Ok(buf) => { + if let Ok(cert) = Certificate::from_pem(&buf) { + client = client.add_root_certificate(cert); + } else if let Ok(cert) = Certificate::from_der(&buf) { + client = client.add_root_certificate(cert); + } else { + warn!( + "Failed to parse certificate as PEM or DER format, cert = {}", + cert_path + ); + } + } + Err(error) => { warn!( - "Failed to parse certificate as PEM or DER format, cert = {}", - cert_path + "Failed to read certificate file, path = {}, error = {}", + cert_path, error ); } } - Err(error) => { - warn!( - "Failed to read certificate file, path = {}, error = {}", - cert_path, error - ); - } } } - } - if http.accept_invalid_certs { - client = client.danger_accept_invalid_certs(true); - } + if http.accept_invalid_certs { + client = client.danger_accept_invalid_certs(true); + } - if let Some(version) = http.min_tls_version { - client = client.min_tls_version(to_reqwest_tls(version)); - } + if let Some(version) = http.min_tls_version { + client = client.min_tls_version(to_reqwest_tls(version)); + } - if let Some(version) = http.max_tls_version { - client = client.max_tls_version(to_reqwest_tls(version)); - } + if let Some(version) = http.max_tls_version { + client = client.max_tls_version(to_reqwest_tls(version)); + } - match http.tls_backend { - TlsBackend::Rustls => { - client = client.use_rustls_tls(); + match http.tls_backend { + TlsBackend::Rustls => { + client = client.use_rustls_tls(); + } + TlsBackend::Default => {} } - TlsBackend::Default => {} - } - Self { - debug_requests: config.debug_requests, - client: client.build().unwrap(), - file: file_writer, - } + client.build().unwrap() + }) } async fn get(&self, url: &Url, headers: Option) -> anyhow::Result { @@ -162,7 +178,7 @@ impl ForgeHttpInfra { where B: FnOnce(&Client) -> reqwest::RequestBuilder, { - let response = request_builder(&self.client) + let response = request_builder(self.client()) .send() .await .with_context(|| format_http_context(None, method, url))?; @@ -256,7 +272,7 @@ impl ForgeHttpInfra { self.write_debug_request(&body); - self.client + self.client() .post(url.clone()) .headers(request_headers) .body(body) diff --git a/crates/forge_infra/src/mcp_client.rs b/crates/forge_infra/src/mcp_client.rs index 23e246a1f3..da2309149a 100644 --- a/crates/forge_infra/src/mcp_client.rs +++ b/crates/forge_infra/src/mcp_client.rs @@ -35,7 +35,7 @@ type RmcpClient = RunningService; #[derive(Clone)] pub struct ForgeMcpClient { client: Arc>>>, - http_client: Arc, + http_client: Arc>>, config: McpServerConfig, env_vars: BTreeMap, environment: Environment, @@ -62,30 +62,9 @@ impl ForgeMcpClient { env_vars: &BTreeMap, environment: Environment, ) -> Self { - // Try to resolve config early so we can extract headers for the HTTP client. - // If resolution fails, fall back to a plain client (headers will be missing - // but the error will surface when create_connection is called). - let resolved = resolve_http_templates( - match &config { - McpServerConfig::Http(http) => http.clone(), - McpServerConfig::Stdio(_) => McpHttpServer { - url: String::new(), - headers: BTreeMap::new(), - timeout: None, - disable: false, - oauth: forge_domain::McpOAuthSetting::default(), - }, - }, - env_vars, - ); - - let http_client = resolved - .and_then(|http| Self::build_http_client(&http)) - .unwrap_or_default(); - Self { client: Default::default(), - http_client: Arc::new(http_client), + http_client: Arc::new(OnceLock::new()), config, env_vars: env_vars.clone(), environment, @@ -500,7 +479,28 @@ impl ForgeMcpClient { // to prevent file descriptor leaks. Each reqwest::Client manages its // own connection pool, so creating new clients for each connection // leads to "Too many open files" errors. - self.http_client.clone() + self.http_client + .get_or_init(|| { + let resolved = resolve_http_templates( + match &self.config { + McpServerConfig::Http(http) => http.clone(), + McpServerConfig::Stdio(_) => McpHttpServer { + url: String::new(), + headers: BTreeMap::new(), + timeout: None, + disable: false, + oauth: forge_domain::McpOAuthSetting::default(), + }, + }, + &self.env_vars, + ); + Arc::new( + resolved + .and_then(|http| Self::build_http_client(&http)) + .unwrap_or_default(), + ) + }) + .clone() } async fn list(&self) -> anyhow::Result> { diff --git a/crates/forge_tracker/src/collect/posthog.rs b/crates/forge_tracker/src/collect/posthog.rs index d7c3213a29..2e09fc5dea 100644 --- a/crates/forge_tracker/src/collect/posthog.rs +++ b/crates/forge_tracker/src/collect/posthog.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::OnceLock; use std::time::Duration; use chrono::NaiveDateTime; @@ -13,21 +14,24 @@ use crate::Event; pub struct Tracker { api_secret: &'static str, - client: Client, + client: OnceLock, } impl Tracker { pub fn new(api_secret: &'static str) -> Self { - // Configure HTTP client with connection pooling similar to forge_provider - let client = Client::builder() - .connect_timeout(Duration::from_secs(10)) - .read_timeout(Duration::from_secs(30)) - .pool_idle_timeout(Duration::from_secs(90)) - .pool_max_idle_per_host(5) - .build() - .expect("Failed to build HTTP client for PostHog tracker"); + Self { api_secret, client: OnceLock::new() } + } - Self { api_secret, client } + fn client(&self) -> &Client { + self.client.get_or_init(|| { + reqwest::Client::builder() + .connect_timeout(Duration::from_secs(10)) + .read_timeout(Duration::from_secs(30)) + .pool_idle_timeout(Duration::from_secs(90)) + .pool_max_idle_per_host(5) + .build() + .expect("Failed to build HTTP client for PostHog tracker") + }) } } @@ -96,7 +100,7 @@ impl Collect for Tracker { // TODO: move http request to a dispatch async fn collect(&self, event: Event) -> Result<()> { let request = self.create_request(event)?; - self.client.execute(request).await?; + self.client().execute(request).await?; Ok(()) }