Bubble up errors in bank_fork_utils instead of exiting process

core/src/validator.rs

@@ -1801,7 +1801,8 @@ fn load_blockstore(
                 .map(|service| service.sender()),
             accounts_update_notifier,
             exit,
-        );
+        )
+        .map_err(|err| err.to_string())?;
 
     // Before replay starts, set the callbacks in each of the banks in BankForks so that
     // all dropped banks come through the `pruned_banks_receiver` channel. This way all bank

ledger-tool/src/ledger_utils.rs

@@ -16,9 +16,7 @@ use {
             AccessType, BlockstoreOptions, BlockstoreRecoveryMode, LedgerColumnOptions,
             ShredStorageType,
         },
-        blockstore_processor::{
-            self, BlockstoreProcessorError, ProcessOptions, TransactionStatusSender,
-        },
+        blockstore_processor::{self, ProcessOptions, TransactionStatusSender},
     },
     solana_measure::measure,
     solana_rpc::transaction_status_service::TransactionStatusService,
@@ -71,7 +69,7 @@ pub fn load_and_process_ledger(
     process_options: ProcessOptions,
     snapshot_archive_path: Option<PathBuf>,
     incremental_snapshot_archive_path: Option<PathBuf>,
-) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), BlockstoreProcessorError> {
+) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), String> {
     let bank_snapshots_dir = if blockstore.is_primary_access() {
         blockstore.ledger_path().join("snapshot")
     } else {
@@ -245,7 +243,8 @@ pub fn load_and_process_ledger(
             None, // Maybe support this later, though
             accounts_update_notifier,
             exit.clone(),
-        );
+        )
+        .map_err(|err| err.to_string())?;
     let block_verification_method = value_t!(
         arg_matches,
         "block_verification_method",
@@ -345,7 +344,8 @@ pub fn load_and_process_ledger(
         None, // Maybe support this later, though
         &accounts_background_request_sender,
     )
-    .map(|_| (bank_forks, starting_snapshot_hashes));
+    .map(|_| (bank_forks, starting_snapshot_hashes))
+    .map_err(|err| err.to_string());
 
     exit.store(true, Ordering::Relaxed);
     accounts_background_service.join().unwrap();

ledger/src/bank_forks_utils.rs

@@ -25,18 +25,50 @@ use {
     solana_sdk::genesis_config::GenesisConfig,
     std::{
         path::PathBuf,
-        process, result,
+        result,
         sync::{atomic::AtomicBool, Arc, RwLock},
     },
+    thiserror::Error,
 };
 
+#[derive(Error, Debug)]
+pub enum BankForksUtilsError {
+    #[error("accounts path(s) not present when booting from snapshot")]
+    AccountPathsNotPresent,
+
+    #[error(
+        "failed to load bank: {source}, full snapshot archive: {full_snapshot_archive}, \
+        incremental snapshot archive: {incremental_snapshot_archive}"
+    )]
+    BankFromSnapshotsArchive {
+        source: snapshot_utils::SnapshotError,
+        full_snapshot_archive: String,
+        incremental_snapshot_archive: String,
+    },
+
+    #[error(
+        "there is no local state to startup from. \
+        Ensure --{flag} is NOT set to \"{value}\" and restart"
+    )]
+    NoBankSnapshotDirectory { flag: String, value: String },
+
+    #[error("failed to load bank: {source}, snapshot: {path}")]
+    BankFromSnapshotsDirectory {
+        source: snapshot_utils::SnapshotError,
+        path: PathBuf,
+    },
+
+    #[error("failed to process blockstore from root: {0}")]
+    ProcessBlockstoreFromRoot(#[source] BlockstoreProcessorError),
+}
+
 pub type LoadResult = result::Result<
     (
         Arc<RwLock<BankForks>>,
         LeaderScheduleCache,
         Option<StartingSnapshotHashes>,
     ),
-    BlockstoreProcessorError,
+    BankForksUtilsError,
 >;
 
 /// Load the banks via genesis or a snapshot then processes all full blocks in blockstore
@@ -68,8 +100,7 @@ pub fn load(
         entry_notification_sender,
         accounts_update_notifier,
         exit,
-    );
-
+    )?;
     blockstore_processor::process_blockstore_from_root(
         blockstore,
         &bank_forks,
@@ -80,7 +111,9 @@ pub fn load(
         entry_notification_sender,
         &AbsRequestSender::default(),
     )
-    .map(|_| (bank_forks, leader_schedule_cache, starting_snapshot_hashes))
+    .map_err(BankForksUtilsError::ProcessBlockstoreFromRoot)?;
+
+    Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -95,11 +128,7 @@ pub fn load_bank_forks(
     entry_notification_sender: Option<&EntryNotifierSender>,
     accounts_update_notifier: Option<AccountsUpdateNotifier>,
     exit: Arc<AtomicBool>,
-) -> (
-    Arc<RwLock<BankForks>>,
-    LeaderScheduleCache,
-    Option<StartingSnapshotHashes>,
-) {
+) -> LoadResult {
     fn get_snapshots_to_load(
         snapshot_config: Option<&SnapshotConfig>,
     ) -> Option<(
@@ -157,7 +186,7 @@ pub fn load_bank_forks(
                 process_options,
                 accounts_update_notifier,
                 exit,
-            );
+            )?;
             (bank_forks, Some(starting_snapshot_hashes))
         } else {
             info!("Processing ledger from genesis");
@@ -193,7 +222,7 @@ pub fn load_bank_forks(
             .for_each(|hard_fork_slot| root_bank.register_hard_fork(*hard_fork_slot));
     }
 
-    (bank_forks, leader_schedule_cache, starting_snapshot_hashes)
+    Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -207,11 +236,10 @@ fn bank_forks_from_snapshot(
     process_options: &ProcessOptions,
     accounts_update_notifier: Option<AccountsUpdateNotifier>,
     exit: Arc<AtomicBool>,
-) -> (Arc<RwLock<BankForks>>, StartingSnapshotHashes) {
+) -> Result<(Arc<RwLock<BankForks>>, StartingSnapshotHashes), BankForksUtilsError> {
     // Fail hard here if snapshot fails to load, don't silently continue
     if account_paths.is_empty() {
-        error!("Account paths not present when booting from snapshot");
-        process::exit(1);
+        return Err(BankForksUtilsError::AccountPathsNotPresent);
     }
 
     let latest_snapshot_archive_slot = std::cmp::max(
@@ -261,29 +289,21 @@ fn bank_forks_from_snapshot(
             accounts_update_notifier,
             exit,
         )
-        .unwrap_or_else(|err| {
-            error!(
-                "Failed to load bank: {err} \
-                \nfull snapshot archive: {} \
-                \nincremental snapshot archive: {}",
-                full_snapshot_archive_info.path().display(),
-                incremental_snapshot_archive_info
-                    .as_ref()
-                    .map(|archive| archive.path().display().to_string())
-                    .unwrap_or("none".to_string()),
-            );
-            process::exit(1);
-        });
+        .map_err(|err| BankForksUtilsError::BankFromSnapshotsArchive {
+            source: err,
+            full_snapshot_archive: full_snapshot_archive_info.path().display().to_string(),
+            incremental_snapshot_archive: incremental_snapshot_archive_info
+                .as_ref()
+                .map(|archive| archive.path().display().to_string())
+                .unwrap_or("none".to_string()),
+        })?;
         bank
     } else {
-        let Some(bank_snapshot) = latest_bank_snapshot else {
-            error!(
-                "There is no local state to startup from. Ensure --{} is *not* set to \"{}\" and restart.",
-                use_snapshot_archives_at_startup::cli::LONG_ARG,
-                UseSnapshotArchivesAtStartup::Never.to_string(),
-            );
-            process::exit(1);
-        };
+        let bank_snapshot =
+            latest_bank_snapshot.ok_or_else(|| BankForksUtilsError::NoBankSnapshotDirectory {
+                flag: use_snapshot_archives_at_startup::cli::LONG_ARG.to_string(),
+                value: UseSnapshotArchivesAtStartup::Never.to_string(),
+            })?;
 
         // If a newer snapshot archive was downloaded, it is possible that its slot is
         // higher than the local bank we will load.  Did the user intend for this?
@@ -318,14 +338,10 @@ fn bank_forks_from_snapshot(
             accounts_update_notifier,
             exit,
         )
-        .unwrap_or_else(|err| {
-            error!(
-                "Failed to load bank: {err} \
-                \nsnapshot: {}",
-                bank_snapshot.snapshot_path().display(),
-            );
-            process::exit(1);
-        });
+        .map_err(|err| BankForksUtilsError::BankFromSnapshotsDirectory {
+            source: err,
+            path: bank_snapshot.snapshot_path(),
+        })?;
         bank
     };
 
@@ -349,5 +365,5 @@ fn bank_forks_from_snapshot(
         incremental: incremental_snapshot_hash,
     };
 
-    (BankForks::new_rw_arc(bank), starting_snapshot_hashes)
+    Ok((BankForks::new_rw_arc(bank), starting_snapshot_hashes))
 }

ledger/src/blockstore_processor.rs

@@ -751,7 +751,8 @@ pub fn test_process_blockstore(
         None,
         None,
         exit,
-    );
+    )
+    .unwrap();
 
     process_blockstore_from_root(
         blockstore,