From 8d13e4f691bd3eab5a8af93c34e2b93bc5c52995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Desr=C3=A9?= Date: Wed, 7 Sep 2022 05:49:20 -0700 Subject: [PATCH] fix: Improve error management and reporting when using unix domain sockets (#228) --- iroh-gateway/src/core.rs | 10 ++++++---- iroh-one/src/config.rs | 12 ++++++------ iroh-one/src/main.rs | 14 ++++++++++++-- iroh-one/src/uds.rs | 31 +++++++++++++++++++++++-------- iroh-rpc-types/src/macros.rs | 9 +++++++++ 5 files changed, 56 insertions(+), 20 deletions(-) diff --git a/iroh-gateway/src/core.rs b/iroh-gateway/src/core.rs index 50b817d34..2694cbe9b 100644 --- a/iroh-gateway/src/core.rs +++ b/iroh-gateway/src/core.rs @@ -34,8 +34,9 @@ impl Core { bad_bits: Arc>>, ) -> anyhow::Result { tokio::spawn(async move { - // TODO: handle error - rpc::new(rpc_addr, Gateway::default()).await + if let Err(err) = rpc::new(rpc_addr, Gateway::default()).await { + tracing::error!("Failed to run gateway rpc handler: {}", err); + } }); let rpc_client = RpcClient::new(config.rpc_client().clone()).await?; let mut templates = HashMap::new(); @@ -58,8 +59,9 @@ impl Core { state: Arc, ) -> anyhow::Result { tokio::spawn(async move { - // TODO: handle error - rpc::new(rpc_addr, Gateway::default()).await + if let Err(err) = rpc::new(rpc_addr, Gateway::default()).await { + tracing::error!("Failed to run gateway rpc handler: {}", err); + } }); Ok(Self { state }) } diff --git a/iroh-one/src/config.rs b/iroh-one/src/config.rs index 72ade1f6e..a1c04bf83 100644 --- a/iroh-one/src/config.rs +++ b/iroh-one/src/config.rs @@ -59,11 +59,11 @@ impl Config { } } - /// When running in ipfsd mode, the resolver will use memory channels to + /// When running in single binary mode, the resolver will use memory channels to /// communicate with the p2p and store modules. /// The gateway itself is exposing a UDS rpc endpoint to be also usable /// as a single entry point for other system services if feature enabled. - pub fn default_ipfsd() -> RpcClientConfig { + pub fn default_rpc_config() -> RpcClientConfig { #[cfg(feature = "uds-gateway")] let path: PathBuf = TempDir::new("iroh").unwrap().path().join("ipfsd.http"); @@ -92,14 +92,14 @@ impl Default for Config { fn default() -> Self { #[cfg(feature = "uds-gateway")] let gateway_uds_path: PathBuf = TempDir::new("iroh").unwrap().path().join("ipfsd.http"); - let ipfsd = Self::default_ipfsd(); + let rpc_client = Self::default_rpc_config(); let metrics_config = MetricsConfig::default(); Self { - rpc_client: ipfsd.clone(), + rpc_client: rpc_client.clone(), metrics: metrics_config.clone(), gateway: iroh_gateway::config::Config::default(), - store: default_store_config(ipfsd.clone(), metrics_config.clone()), - p2p: default_p2p_config(ipfsd, metrics_config), + store: default_store_config(rpc_client.clone(), metrics_config.clone()), + p2p: default_p2p_config(rpc_client, metrics_config), #[cfg(feature = "uds-gateway")] gateway_uds_path: Some(gateway_uds_path), } diff --git a/iroh-one/src/main.rs b/iroh-one/src/main.rs index 28bb75f42..fcc25ed75 100644 --- a/iroh-one/src/main.rs +++ b/iroh-one/src/main.rs @@ -96,10 +96,20 @@ async fn main() -> Result<()> { let mut path = TempDir::new("iroh")?.path().join("ipfsd.http"); if let Some(uds_path) = config.gateway_uds_path { path = uds_path; + } else { + // Create the parent path when using the default value since it's likely + // it won't exist yet. + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(&parent); + } } - let uds_server = uds::uds_server(shared_state, path); + tokio::spawn(async move { - uds_server.await.unwrap(); + if let Some(uds_server) = uds::uds_server(shared_state, path) { + if let Err(err) = uds_server.await { + tracing::error!("Failure in http uds handler: {}", err); + } + } }) }; diff --git a/iroh-one/src/uds.rs b/iroh-one/src/uds.rs index 89f63abff..611fcf3fc 100644 --- a/iroh-one/src/uds.rs +++ b/iroh-one/src/uds.rs @@ -100,14 +100,29 @@ impl connect_info::Connected<&UnixStream> for UdsConnectInfo { pub fn uds_server( state: Arc, path: PathBuf, -) -> Server< - ServerAccept, - axum::extract::connect_info::IntoMakeServiceWithConnectInfo, +) -> Option< + Server< + ServerAccept, + axum::extract::connect_info::IntoMakeServiceWithConnectInfo, + >, > { let _ = std::fs::remove_file(&path); - let uds = UnixListener::bind(&path).unwrap(); - println!("Binding to UDS at {}", path.display()); - let app = get_app_routes(&state); - Server::builder(ServerAccept { uds }) - .serve(app.into_make_service_with_connect_info::()) + match UnixListener::bind(&path) { + Ok(uds) => { + tracing::debug!("Binding to UDS at {}", path.display()); + let app = get_app_routes(&state); + Some( + Server::builder(ServerAccept { uds }) + .serve(app.into_make_service_with_connect_info::()), + ) + } + Err(err) => { + tracing::error!( + "Failed to bind http uds socket at {}: {}", + path.display(), + err + ); + None + } + } } diff --git a/iroh-rpc-types/src/macros.rs b/iroh-rpc-types/src/macros.rs index 4b15464fe..cbfea11b9 100644 --- a/iroh-rpc-types/src/macros.rs +++ b/iroh-rpc-types/src/macros.rs @@ -37,6 +37,15 @@ macro_rules! proxy { anyhow::bail!("cannot bind socket: already exists: {}", path.display()); } } + + // If the parent directory doesn't exist, we'll fail to bind. + // Create a more precise error to recognize that case. + if let Some(parent) = path.parent() { + if !parent.exists() { + anyhow::bail!("socket parent directory doesn't exist: {}", parent.display()); + } + } + // Delete file on close struct UdsGuard(std::path::PathBuf); impl Drop for UdsGuard {