Compare commits

...
Author SHA1 Message Date
willamhouandZaki 7b1fc331c6 fix(tunnel): add missing .unwrap() in stdout_drain_prevents_zombie test
CustomTunnel::new was changed to return Result<Self> but this test
callsite was missed. Fixes compilation error.

[skip-regression-check]
2026-03-22 19:49:11 -07:00
ccfcd4bfde refactor: propagate error from Client::builder instead of silent fallback
Address review feedback: `unwrap_or_else(|_| Client::new())` silently
drops the configured timeout on builder failure. Change `new()` to
return `Result<Self>` and propagate with `?` instead.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: Happy <[email protected]>
2026-03-22 19:49:11 -07:00
fe805cce14 perf(tunnel): reuse HTTP client in CustomTunnel health checks
Previously, `health_check()` created a new `reqwest::Client` on every
call. Since health checks run periodically, this caused unnecessary TLS
session setup and connection pool churn. Store a shared client on the
struct instead, created once in the constructor with a 5s timeout.

Fixes #1039

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: Happy <[email protected]>
2026-03-22 19:49:11 -07:00
2 changed files with 24 additions and 14 deletions
+23 -13
View File
@@ -1,6 +1,6 @@
//! Custom tunnel via an arbitrary shell command. //! Custom tunnel via an arbitrary shell command.
use anyhow::{Result, bail}; use anyhow::{Context, Result, bail};
use tokio::io::AsyncBufReadExt; use tokio::io::AsyncBufReadExt;
use tokio::process::Command; use tokio::process::Command;
@@ -27,6 +27,7 @@ pub struct CustomTunnel {
url_pattern: Option<String>, url_pattern: Option<String>,
proc: SharedProcess, proc: SharedProcess,
url: SharedUrl, url: SharedUrl,
http_client: reqwest::Client,
} }
impl CustomTunnel { impl CustomTunnel {
@@ -34,14 +35,19 @@ impl CustomTunnel {
start_command: String, start_command: String,
health_url: Option<String>, health_url: Option<String>,
url_pattern: Option<String>, url_pattern: Option<String>,
) -> Self { ) -> Result<Self> {
Self { let http_client = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(5))
.build()
.context("failed to create HTTP client for tunnel health checks")?;
Ok(Self {
start_command, start_command,
health_url, health_url,
url_pattern, url_pattern,
proc: new_shared_process(), proc: new_shared_process(),
url: new_shared_url(), url: new_shared_url(),
} http_client,
})
} }
} }
@@ -140,9 +146,9 @@ impl Tunnel for CustomTunnel {
async fn health_check(&self) -> bool { async fn health_check(&self) -> bool {
if let Some(ref url) = self.health_url { if let Some(ref url) = self.health_url {
return reqwest::Client::new() return self
.http_client
.get(url) .get(url)
.timeout(std::time::Duration::from_secs(5))
.send() .send()
.await .await
.is_ok(); .is_ok();
@@ -173,7 +179,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn empty_command_returns_error() { async fn empty_command_returns_error() {
let tunnel = CustomTunnel::new(" ".into(), None, None); let tunnel = CustomTunnel::new(" ".into(), None, None).unwrap();
let result = tunnel.start("127.0.0.1", 8080).await; let result = tunnel.start("127.0.0.1", 8080).await;
assert!(result.is_err()); assert!(result.is_err());
assert!( assert!(
@@ -186,7 +192,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn start_without_pattern_returns_local() { async fn start_without_pattern_returns_local() {
let tunnel = CustomTunnel::new("sleep 1".into(), None, None); let tunnel = CustomTunnel::new("sleep 1".into(), None, None).unwrap();
let url = tunnel.start("127.0.0.1", 4455).await.unwrap(); let url = tunnel.start("127.0.0.1", 4455).await.unwrap();
assert_eq!(url, "http://127.0.0.1:4455"); assert_eq!(url, "http://127.0.0.1:4455");
tunnel.stop().await.unwrap(); tunnel.stop().await.unwrap();
@@ -198,7 +204,8 @@ mod tests {
"echo https://public.example".into(), "echo https://public.example".into(),
None, None,
Some("public.example".into()), Some("public.example".into()),
); )
.unwrap();
let url = tunnel.start("localhost", 9999).await.unwrap(); let url = tunnel.start("localhost", 9999).await.unwrap();
assert_eq!(url, "https://public.example"); assert_eq!(url, "https://public.example");
tunnel.stop().await.unwrap(); tunnel.stop().await.unwrap();
@@ -213,7 +220,8 @@ mod tests {
r"printf http://internal:1234\nhttps://real.tunnel.io/abc\n".into(), r"printf http://internal:1234\nhttps://real.tunnel.io/abc\n".into(),
None, None,
Some("tunnel.io".into()), Some("tunnel.io".into()),
); )
.unwrap();
let url = tunnel.start("localhost", 9999).await.unwrap(); let url = tunnel.start("localhost", 9999).await.unwrap();
assert_eq!(url, "https://real.tunnel.io/abc"); assert_eq!(url, "https://real.tunnel.io/abc");
tunnel.stop().await.unwrap(); tunnel.stop().await.unwrap();
@@ -225,7 +233,8 @@ mod tests {
"echo http://{host}:{port}".into(), "echo http://{host}:{port}".into(),
None, None,
Some("http://".into()), Some("http://".into()),
); )
.unwrap();
let url = tunnel.start("10.1.2.3", 4321).await.unwrap(); let url = tunnel.start("10.1.2.3", 4321).await.unwrap();
assert_eq!(url, "http://10.1.2.3:4321"); assert_eq!(url, "http://10.1.2.3:4321");
tunnel.stop().await.unwrap(); tunnel.stop().await.unwrap();
@@ -238,7 +247,8 @@ mod tests {
"sleep 1".into(), "sleep 1".into(),
Some("http://192.0.2.1:9999/healthz".into()), Some("http://192.0.2.1:9999/healthz".into()),
None, None,
); )
.unwrap();
assert!( assert!(
!tunnel.health_check().await, !tunnel.health_check().await,
"Health check should fail for unreachable URL" "Health check should fail for unreachable URL"
@@ -271,7 +281,7 @@ mod tests {
// `yes` floods stdout indefinitely; without the drain task the pipe // `yes` floods stdout indefinitely; without the drain task the pipe
// buffer fills (64 KB) and the child blocks on write(), becoming a // buffer fills (64 KB) and the child blocks on write(), becoming a
// zombie. With draining the child stays alive and stop() can kill it. // zombie. With draining the child stays alive and stop() can kill it.
let tunnel = CustomTunnel::new("yes".into(), None, None); let tunnel = CustomTunnel::new("yes".into(), None, None).unwrap();
let url = tunnel.start("127.0.0.1", 19999).await.unwrap(); let url = tunnel.start("127.0.0.1", 19999).await.unwrap();
assert_eq!(url, "http://127.0.0.1:19999"); assert_eq!(url, "http://127.0.0.1:19999");
+1 -1
View File
@@ -171,7 +171,7 @@ pub fn create_tunnel(config: &TunnelProviderConfig) -> Result<Option<Box<dyn Tun
cu.start_command.clone(), cu.start_command.clone(),
cu.health_url.clone(), cu.health_url.clone(),
cu.url_pattern.clone(), cu.url_pattern.clone(),
)))) )?)))
} }
other => bail!( other => bail!(