Let Builder emit BuildError rather than panic

Author: tnull

README.md

@@ -23,7 +23,7 @@ fn main() {
 	builder.set_esplora_server("https://blockstream.info/testnet/api".to_string());
 	builder.set_gossip_source_rgs("https://rapidsync.lightningdevkit.org/testnet/snapshot".to_string());
 
-	let node = builder.build();
+	let node = builder.build().unwrap();
 
 	node.start().unwrap();
 

bindings/ldk_node.udl

@@ -17,6 +17,7 @@ interface Builder {
 	[Name=from_config]
 	constructor(Config config);
 	void set_entropy_seed_path(string seed_path);
+	[Throws=BuildError]
 	void set_entropy_seed_bytes(sequence<u8> seed_bytes);
 	void set_entropy_bip39_mnemonic(Mnemonic mnemonic, string? passphrase);
 	void set_esplora_server(string esplora_server_url);
@@ -25,6 +26,7 @@ interface Builder {
 	void set_storage_dir_path(string storage_dir_path);
 	void set_network(Network network);
 	void set_listening_address(NetAddress listening_address);
+	[Throws=BuildError]
 	LDKNode build();
 };
 
@@ -110,6 +112,16 @@ enum NodeError {
 	"InsufficientFunds",
 };
 
+[Error]
+enum BuildError {
+	"InvalidSeedBytes",
+	"InvalidSystemTime",
+	"IOReadFailed",
+	"IOWriteFailed",
+	"StoragePathAccessFailed",
+	"WalletSetupFailed",
+};
+
 [Enum]
 interface Event {
 	PaymentSuccessful( PaymentHash payment_hash );

src/builder.rs

@@ -44,6 +44,7 @@ use bitcoin::BlockHash;
 
 use std::convert::TryInto;
 use std::default::Default;
+use std::fmt;
 use std::fs;
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::SystemTime;
@@ -66,6 +67,42 @@ enum GossipSourceConfig {
 	RapidGossipSync(String),
 }
 
+/// An error encountered during building a [`Node`].
+///
+/// [`Node`]: crate::Node
+#[derive(Debug, Clone)]
+pub enum BuildError {
+	/// The given seed bytes are invalid, e.g, are of invalid length.
+	InvalidSeedBytes,
+	/// The current system time is invalid, clocks might have gone backwards.
+	InvalidSystemTime,
+	/// We failed to read data from the [`KVStore`].
+	IOReadFailed,
+	/// We failed to write data to the [`KVStore`].
+	IOWriteFailed,
+	/// We failed to access the given `storage_dir_path`.
+	StoragePathAccessFailed,
+	/// We failed to setup the onchain wallet.
+	WalletSetupFailed,
+}
+
+impl fmt::Display for BuildError {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		match *self {
+			Self::InvalidSeedBytes => write!(f, "Given seed bytes are invalid."),
+			Self::InvalidSystemTime => {
+				write!(f, "System time is invalid. Clocks might have gone back in time.")
+			}
+			Self::IOReadFailed => write!(f, "Failed to read from store."),
+			Self::IOWriteFailed => write!(f, "Failed to write to store."),
+			Self::StoragePathAccessFailed => write!(f, "Failed to access the given storage path."),
+			Self::WalletSetupFailed => write!(f, "Failed to setup onchain wallet."),
+		}
+	}
+}
+
+impl std::error::Error for BuildError {}
+
 /// A builder for an [`Node`] instance, allowing to set some configuration and module choices from
 /// the getgo.
 ///
@@ -111,13 +148,14 @@ impl NodeBuilder {
 	/// Configures the [`Node`] instance to source its wallet entropy from the given 64 seed bytes.
 	///
 	/// **Note:** Panics if the length of the given `seed_bytes` differs from 64.
-	pub fn set_entropy_seed_bytes(&mut self, seed_bytes: Vec<u8>) {
+	pub fn set_entropy_seed_bytes(&mut self, seed_bytes: Vec<u8>) -> Result<(), BuildError> {
 		if seed_bytes.len() != WALLET_KEYS_SEED_LEN {
-			panic!("Failed to set seed due to invalid length.");
+			return Err(BuildError::InvalidSeedBytes);
 		}
 		let mut bytes = [0u8; WALLET_KEYS_SEED_LEN];
 		bytes.copy_from_slice(&seed_bytes);
 		self.entropy_source_config = Some(EntropySourceConfig::SeedBytes(bytes));
+		Ok(())
 	}
 
 	/// Configures the [`Node`] instance to source its wallet entropy from a [BIP 39] mnemonic.
@@ -167,26 +205,28 @@ impl NodeBuilder {
 
 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
 	/// previously configured.
-	pub fn build(&self) -> Node<SqliteStore> {
+	pub fn build(&self) -> Result<Node<SqliteStore>, BuildError> {
 		let storage_dir_path = self.config.storage_dir_path.clone();
-		fs::create_dir_all(storage_dir_path.clone()).expect("Failed to create LDK data directory");
+		fs::create_dir_all(storage_dir_path.clone())
+			.map_err(|_| BuildError::StoragePathAccessFailed)?;
 		let kv_store = Arc::new(SqliteStore::new(storage_dir_path.into()));
 		self.build_with_store(kv_store)
 	}
 
 	/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
 	/// previously configured.
-	pub fn build_with_fs_store(&self) -> Node<FilesystemStore> {
+	pub fn build_with_fs_store(&self) -> Result<Node<FilesystemStore>, BuildError> {
 		let storage_dir_path = self.config.storage_dir_path.clone();
-		fs::create_dir_all(storage_dir_path.clone()).expect("Failed to create LDK data directory");
+		fs::create_dir_all(storage_dir_path.clone())
+			.map_err(|_| BuildError::StoragePathAccessFailed)?;
 		let kv_store = Arc::new(FilesystemStore::new(storage_dir_path.into()));
 		self.build_with_store(kv_store)
 	}
 
 	/// Builds a [`Node`] instance according to the options previously configured.
 	pub fn build_with_store<K: KVStore + Sync + Send + 'static>(
 		&self, kv_store: Arc<K>,
-	) -> Node<K> {
+	) -> Result<Node<K>, BuildError> {
 		let config = Arc::new(self.config.clone());
 
 		let runtime = Arc::new(RwLock::new(None));
@@ -239,7 +279,7 @@ impl ArcedNodeBuilder {
 	/// Configures the [`Node`] instance to source its wallet entropy from the given 64 seed bytes.
 	///
 	/// **Note:** Panics if the length of the given `seed_bytes` differs from 64.
-	pub fn set_entropy_seed_bytes(&self, seed_bytes: Vec<u8>) {
+	pub fn set_entropy_seed_bytes(&self, seed_bytes: Vec<u8>) -> Result<(), BuildError> {
 		self.inner.write().unwrap().set_entropy_seed_bytes(seed_bytes)
 	}
 
@@ -289,21 +329,21 @@ impl ArcedNodeBuilder {
 
 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
 	/// previously configured.
-	pub fn build(&self) -> Arc<Node<SqliteStore>> {
-		Arc::new(self.inner.read().unwrap().build())
+	pub fn build(&self) -> Result<Arc<Node<SqliteStore>>, BuildError> {
+		self.inner.read().unwrap().build().map(Arc::new)
 	}
 
 	/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
 	/// previously configured.
-	pub fn build_with_fs_store(&self) -> Arc<Node<FilesystemStore>> {
-		Arc::new(self.inner.read().unwrap().build_with_fs_store())
+	pub fn build_with_fs_store(&self) -> Result<Arc<Node<FilesystemStore>>, BuildError> {
+		self.inner.read().unwrap().build_with_fs_store().map(Arc::new)
 	}
 
 	/// Builds a [`Node`] instance according to the options previously configured.
 	pub fn build_with_store<K: KVStore + Sync + Send + 'static>(
 		&self, kv_store: Arc<K>,
-	) -> Arc<Node<K>> {
-		Arc::new(self.inner.read().unwrap().build_with_store(kv_store))
+	) -> Result<Arc<Node<K>>, BuildError> {
+		self.inner.read().unwrap().build_with_store(kv_store).map(Arc::new)
 	}
 }
 
@@ -313,12 +353,12 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 	chain_data_source_config: Option<&'a ChainDataSourceConfig>,
 	gossip_source_config: Option<&'a GossipSourceConfig>, kv_store: Arc<K>,
 	runtime: Arc<RwLock<Option<tokio::runtime::Runtime>>>,
-) -> Node<K> {
+) -> Result<Node<K>, BuildError> {
 	let ldk_data_dir = format!("{}/ldk", config.storage_dir_path);
-	fs::create_dir_all(ldk_data_dir.clone()).expect("Failed to create LDK data directory");
+	fs::create_dir_all(ldk_data_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
 
 	let bdk_data_dir = format!("{}/bdk", config.storage_dir_path);
-	fs::create_dir_all(bdk_data_dir.clone()).expect("Failed to create BDK data directory");
+	fs::create_dir_all(bdk_data_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
 
 	// Initialize the Logger
 	let log_file_path = format!(
@@ -346,15 +386,15 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 	};
 
 	let xprv = bitcoin::util::bip32::ExtendedPrivKey::new_master(config.network, &seed_bytes)
-		.expect("Failed to read wallet master key");
+		.map_err(|_| BuildError::InvalidSeedBytes)?;
 
 	let wallet_name = bdk::wallet::wallet_name_from_descriptor(
 		Bip84(xprv, bdk::KeychainKind::External),
 		Some(Bip84(xprv, bdk::KeychainKind::Internal)),
 		config.network,
 		&Secp256k1::new(),
 	)
-	.expect("Failed to derive on-chain wallet name");
+	.map_err(|_| BuildError::WalletSetupFailed)?;
 
 	let database_path = format!("{}/bdk_wallet_{}.sqlite", config.storage_dir_path, wallet_name);
 	let database = SqliteDatabase::new(database_path);
@@ -365,7 +405,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 		config.network,
 		database,
 	)
-	.expect("Failed to set up on-chain wallet");
+	.map_err(|_| BuildError::WalletSetupFailed)?;
 
 	let (blockchain, tx_sync) = match chain_data_source_config {
 		Some(ChainDataSourceConfig::Esplora(server_url)) => {
@@ -401,7 +441,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 	// Initialize the KeysManager
 	let cur_time = SystemTime::now()
 		.duration_since(SystemTime::UNIX_EPOCH)
-		.expect("System time error: Clock may have gone backwards");
+		.map_err(|_| BuildError::InvalidSystemTime)?;
 	let ldk_seed_bytes: [u8; 32] = xprv.private_key.secret_bytes();
 	let keys_manager = Arc::new(KeysManager::new(
 		&ldk_seed_bytes,
@@ -418,7 +458,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 				if e.kind() == std::io::ErrorKind::NotFound {
 					Arc::new(NetworkGraph::new(config.network, Arc::clone(&logger)))
 				} else {
-					panic!("Failed to read network graph: {}", e.to_string());
+					return Err(BuildError::IOReadFailed);
 				}
 			}
 		};
@@ -438,7 +478,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 					Arc::clone(&logger),
 				)))
 			} else {
-				panic!("Failed to read scorer: {}", e.to_string());
+				return Err(BuildError::IOReadFailed);
 			}
 		}
 	};
@@ -462,7 +502,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 				Vec::new()
 			} else {
 				log_error!(logger, "Failed to read channel monitors: {}", e.to_string());
-				panic!("Failed to read channel monitors: {}", e.to_string());
+				return Err(BuildError::IOReadFailed);
 			}
 		}
 	};
@@ -489,8 +529,10 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 				channel_monitor_references,
 			);
 			let (_hash, channel_manager) =
-				<(BlockHash, ChannelManager<K>)>::read(&mut reader, read_args)
-					.expect("Failed to read channel manager from store");
+				<(BlockHash, ChannelManager<K>)>::read(&mut reader, read_args).map_err(|e| {
+					log_error!(logger, "Failed to read channel manager from KVStore: {}", e);
+					BuildError::IOReadFailed
+				})?;
 			channel_manager
 		} else {
 			// We're starting a fresh node.
@@ -535,7 +577,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 
 	let cur_time = SystemTime::now()
 		.duration_since(SystemTime::UNIX_EPOCH)
-		.expect("System time error: Clock may have gone backwards");
+		.map_err(|_| BuildError::InvalidSystemTime)?;
 
 	// Initialize the GossipSource
 	// Use the configured gossip source, if the user set one, otherwise default to P2PNetwork.
@@ -552,7 +594,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 				Arc::clone(&kv_store),
 				Arc::clone(&logger),
 			)
-			.expect("Persistence failed");
+			.map_err(|_| BuildError::IOWriteFailed)?;
 			p2p_source
 		}
 		GossipSourceConfig::RapidGossipSync(rgs_server) => {
@@ -590,7 +632,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 
 	let peer_manager = Arc::new(PeerManager::new(
 		msg_handler,
-		cur_time.as_secs().try_into().expect("System time error"),
+		cur_time.as_secs().try_into().map_err(|_| BuildError::InvalidSystemTime)?,
 		&ephemeral_bytes,
 		Arc::clone(&logger),
 		IgnoringMessageHandler {},
@@ -602,8 +644,8 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 		Ok(payments) => {
 			Arc::new(PaymentStore::new(payments, Arc::clone(&kv_store), Arc::clone(&logger)))
 		}
-		Err(e) => {
-			panic!("Failed to read payment information: {}", e.to_string());
+		Err(_) => {
+			return Err(BuildError::IOReadFailed);
 		}
 	};
 
@@ -614,7 +656,7 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 			if e.kind() == std::io::ErrorKind::NotFound {
 				Arc::new(EventQueue::new(Arc::clone(&kv_store), Arc::clone(&logger)))
 			} else {
-				panic!("Failed to read event queue: {}", e.to_string());
+				return Err(BuildError::IOReadFailed);
 			}
 		}
 	};
@@ -625,14 +667,14 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 			if e.kind() == std::io::ErrorKind::NotFound {
 				Arc::new(PeerStore::new(Arc::clone(&kv_store), Arc::clone(&logger)))
 			} else {
-				panic!("Failed to read peer store: {}", e.to_string());
+				return Err(BuildError::IOReadFailed);
 			}
 		}
 	};
 
 	let (stop_sender, stop_receiver) = tokio::sync::watch::channel(());
 
-	Node {
+	Ok(Node {
 		runtime,
 		stop_sender,
 		stop_receiver,
@@ -651,5 +693,5 @@ fn build_with_store_internal<'a, K: KVStore + Sync + Send + 'static>(
 		scorer,
 		peer_store,
 		payment_store,
-	}
+	})
 }

src/lib.rs

@@ -38,7 +38,7 @@
 //! 	builder.set_esplora_server("https://blockstream.info/testnet/api".to_string());
 //! 	builder.set_gossip_source_rgs("https://rapidsync.lightningdevkit.org/testnet/snapshot".to_string());
 //!
-//! 	let node = builder.build();
+//! 	let node = builder.build().unwrap();
 //!
 //! 	node.start().unwrap();
 //!
@@ -111,6 +111,7 @@ use {bip39::Mnemonic, bitcoin::OutPoint, lightning::ln::PaymentSecret, uniffi_ty
 
 #[cfg(feature = "uniffi")]
 pub use builder::ArcedNodeBuilder as Builder;
+pub use builder::BuildError;
 #[cfg(not(feature = "uniffi"))]
 pub use builder::NodeBuilder as Builder;
 
@@ -1312,7 +1313,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 	/// # config.network = Network::Regtest;
 	/// # config.storage_dir_path = "/tmp/ldk_node_test/".to_string();
 	/// # let builder = Builder::from_config(config);
-	/// # let node = builder.build();
+	/// # let node = builder.build().unwrap();
 	/// node.list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound);
 	/// ```
 	pub fn list_payments_with_filter<F: FnMut(&&PaymentDetails) -> bool>(