Issue #144: Refactor errors handling during branches tree printing

pageserver/src/branches.rs

@@ -118,7 +118,10 @@ pub fn init_repo(conf: &PageServerConf, repo_dir: &Path) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn get_branches(repository: &dyn Repository) -> Result<Vec<BranchInfo>> {
+pub(crate) fn get_branches(
+    conf: &PageServerConf,
+    repository: &dyn Repository,
+) -> Result<Vec<BranchInfo>> {
     // Each branch has a corresponding record (text file) in the refs/branches
     // with timeline_id.
     let branches_dir = std::path::Path::new("refs").join("branches");
@@ -134,9 +137,7 @@ pub(crate) fn get_branches(repository: &dyn Repository) -> Result<Vec<BranchInfo
                 .map(|timeline| timeline.get_last_valid_lsn())
                 .ok();
 
-            let ancestor_path = std::path::Path::new("timelines")
-                .join(timeline_id.to_string())
-                .join("ancestor");
+            let ancestor_path = conf.ancestor_path(timeline_id);
             let mut ancestor_id: Option<String> = None;
             let mut ancestor_lsn: Option<String> = None;
 
@@ -144,8 +145,18 @@ pub(crate) fn get_branches(repository: &dyn Repository) -> Result<Vec<BranchInfo
                 let ancestor = std::fs::read_to_string(ancestor_path)?;
                 let mut strings = ancestor.split('@');
 
-                ancestor_id = Some(strings.next().unwrap().to_owned());
-                ancestor_lsn = Some(strings.next().unwrap().to_owned());
+                ancestor_id = Some(
+                    strings
+                        .next()
+                        .with_context(|| "wrong branch ancestor point in time format")?
+                        .to_owned(),
+                );
+                ancestor_lsn = Some(
+                    strings
+                        .next()
+                        .with_context(|| "wrong branch ancestor point in time format")?
+                        .to_owned(),
+                );
             }
 
             Ok(BranchInfo {

pageserver/src/lib.rs

@@ -59,6 +59,10 @@ impl PageServerConf {
         self.timeline_path(timelineid).join("snapshots")
     }
 
+    fn ancestor_path(&self, timelineid: ZTimelineId) -> PathBuf {
+        self.timeline_path(timelineid).join("ancestor")
+    }
+
     //
     // Postgres distribution paths
     //

pageserver/src/page_service.rs

@@ -763,7 +763,8 @@ impl Connection {
             self.write_message_noflush(&BeMessage::DataRow(Bytes::from(branch)))?;
             self.write_message_noflush(&BeMessage::CommandComplete)?;
         } else if query_string.starts_with(b"branch_list") {
-            let branches = crate::branches::get_branches(&*page_cache::get_repository())?;
+            let branches =
+                crate::branches::get_branches(&self.conf, &*page_cache::get_repository())?;
             let branches_buf = serde_json::to_vec(&branches)?;
 
             self.write_message_noflush(&BeMessage::RowDescription)?;

zenith/src/main.rs

@@ -165,7 +165,7 @@ fn main() -> Result<()> {
 }
 
 // Print branches list as a tree-like structure.
-fn print_branches_tree(branches: Vec<BranchInfo>) {
+fn print_branches_tree(branches: Vec<BranchInfo>) -> Result<()> {
     let mut branches_hash: HashMap<String, BranchTreeEl> = HashMap::new();
 
     // Form a hash table of branch timeline_id -> BranchTreeEl.
@@ -184,7 +184,7 @@ fn print_branches_tree(branches: Vec<BranchInfo>) {
         if let Some(name) = &branch.ancestor_id {
             branches_hash
                 .get_mut(name)
-                .unwrap()
+                .with_context(|| "missing branch info in the HashMap")?
                 .children
                 .push(branch.timeline_id.to_string());
         }
@@ -199,9 +199,11 @@ fn print_branches_tree(branches: Vec<BranchInfo>) {
         // Start with root branches (no ancestors) first.
         // Now it is 'main' branch only, but things may change.
         if branch.info.ancestor_id.is_none() {
-            print_branch(0, 0, branch, &branches_hash);
+            print_branch(0, 0, branch, &branches_hash)?;
         }
     }
+
+    Ok(())
 }
 
 // Recursively print branch info with all its children.
@@ -210,18 +212,20 @@ fn print_branch(
     padding: usize,
     branch: &BranchTreeEl,
     branches: &HashMap<String, BranchTreeEl>,
-) {
+) -> Result<()> {
     let new_padding: usize;
 
     if nesting_level > 0 {
-        // Six extra chars for graphics, spaces and so on in addition to LSN length.
-        new_padding = padding + 6 + branch.info.ancestor_lsn.as_ref().unwrap().chars().count();
+        let lsn = branch
+            .info
+            .ancestor_lsn
+            .as_ref()
+            .with_context(|| "missing branch info in the HashMap")?;
 
-        print!(
-            "{}└─ @{}:",
-            " ".repeat(padding),
-            branch.info.ancestor_lsn.as_ref().unwrap()
-        );
+        // Six extra chars for graphics, spaces and so on in addition to LSN length.
+        new_padding = padding + 6 + lsn.chars().count();
+
+        print!("{}└─ @{}:", " ".repeat(padding), lsn);
     } else {
         new_padding = 1;
     }
@@ -232,10 +236,14 @@ fn print_branch(
         print_branch(
             nesting_level + 1,
             new_padding,
-            branches.get(child).unwrap(),
+            branches
+                .get(child)
+                .with_context(|| "missing branch info in the HashMap")?,
             branches,
-        );
+        )?;
     }
+
+    Ok(())
 }
 
 /// Returns a map of timeline IDs to branch_name@lsn strings.
@@ -267,8 +275,8 @@ fn handle_branch(branch_match: &ArgMatches, env: &local_env::LocalEnv) -> Result
         }
     } else {
         // No arguments, list branches
-        let branches = pageserver.branches_list().unwrap();
-        print_branches_tree(branches);
+        let branches = pageserver.branches_list()?;
+        print_branches_tree(branches)?;
     }
 
     Ok(())