repo: propagate error when failing to look up backend type

lib/src/repo.rs

@@ -330,6 +330,20 @@ impl Default for StoreFactories {
     }
 }
 
+#[derive(Debug, Error)]
+pub enum StoreLoadError {
+    #[error("Unsupported {store} backend type '{store_type}'")]
+    UnsupportedType {
+        store: &'static str,
+        store_type: String,
+    },
+    #[error("Failed to read {store} backend type: {source}")]
+    ReadError {
+        store: &'static str,
+        source: io::Error,
+    },
+}
+
 impl StoreFactories {
     pub fn empty() -> Self {
         StoreFactories {
@@ -343,7 +357,7 @@ impl StoreFactories {
         self.backend_factories.insert(name.to_string(), factory);
     }
 
-    pub fn load_backend(&self, store_path: &Path) -> Box<dyn Backend> {
+    pub fn load_backend(&self, store_path: &Path) -> Result<Box<dyn Backend>, StoreLoadError> {
         // For compatibility with existing repos. TODO: Delete in 0.8+.
         if store_path.join("backend").is_file() {
             fs::rename(store_path.join("backend"), store_path.join("type"))
@@ -361,22 +375,27 @@ impl StoreFactories {
                 fs::write(store_path.join("type"), &inferred_type).unwrap();
                 inferred_type
             }
-            Err(_) => {
+            Err(err) => {
-                panic!("Failed to read backend type");
+                return Err(StoreLoadError::ReadError {
+                    store: "commit",
+                    source: err,
+                });
             }
         };
-        let backend_factory = self
+        let backend_factory = self.backend_factories.get(&backend_type).ok_or_else(|| {
-            .backend_factories
+            StoreLoadError::UnsupportedType {
-            .get(&backend_type)
+                store: "commit",
-            .expect("Unexpected backend type");
+                store_type: backend_type.to_string(),
-        backend_factory(store_path)
+            }
+        })?;
+        Ok(backend_factory(store_path))
     }
 
     pub fn add_op_store(&mut self, name: &str, factory: OpStoreFactory) {
         self.op_store_factories.insert(name.to_string(), factory);
     }
 
-    pub fn load_op_store(&self, store_path: &Path) -> Box<dyn OpStore> {
+    pub fn load_op_store(&self, store_path: &Path) -> Result<Box<dyn OpStore>, StoreLoadError> {
         let op_store_type = match fs::read_to_string(store_path.join("type")) {
             Ok(content) => content,
             Err(err) if err.kind() == ErrorKind::NotFound => {
@@ -385,15 +404,20 @@ impl StoreFactories {
                 fs::write(store_path.join("type"), &default_type).unwrap();
                 default_type
             }
-            Err(_) => {
+            Err(err) => {
-                panic!("Failed to read op_store type");
+                return Err(StoreLoadError::ReadError {
+                    store: "operation",
+                    source: err,
+                });
             }
         };
-        let op_store_factory = self
+        let op_store_factory = self.op_store_factories.get(&op_store_type).ok_or_else(|| {
-            .op_store_factories
+            StoreLoadError::UnsupportedType {
-            .get(&op_store_type)
+                store: "operation",
-            .expect("Unexpected op_store type");
+                store_type: op_store_type.to_string(),
-        op_store_factory(store_path)
+            }
+        })?;
+        Ok(op_store_factory(store_path))
     }
 
     pub fn add_op_heads_store(&mut self, name: &str, factory: OpHeadsStoreFactory) {
@@ -401,7 +425,10 @@ impl StoreFactories {
             .insert(name.to_string(), factory);
     }
 
-    pub fn load_op_heads_store(&self, store_path: &Path) -> Box<dyn OpHeadsStore> {
+    pub fn load_op_heads_store(
+        &self,
+        store_path: &Path,
+    ) -> Result<Box<dyn OpHeadsStore>, StoreLoadError> {
         let op_heads_store_type = match fs::read_to_string(store_path.join("type")) {
             Ok(content) => content,
             Err(err) if err.kind() == ErrorKind::NotFound => {
@@ -410,15 +437,21 @@ impl StoreFactories {
                 fs::write(store_path.join("type"), &default_type).unwrap();
                 default_type
             }
-            Err(_) => {
+            Err(err) => {
-                panic!("Failed to read op_heads_store type");
+                return Err(StoreLoadError::ReadError {
+                    store: "operation heads",
+                    source: err,
+                });
             }
         };
         let op_heads_store_factory = self
             .op_heads_store_factories
             .get(&op_heads_store_type)
-            .expect("Unexpected op_heads_store type");
+            .ok_or_else(|| StoreLoadError::UnsupportedType {
-        op_heads_store_factory(store_path)
+                store: "operation heads",
+                store_type: op_heads_store_type.to_string(),
+            })?;
+        Ok(op_heads_store_factory(store_path))
     }
 }
 
@@ -437,21 +470,21 @@ impl RepoLoader {
         user_settings: &UserSettings,
         repo_path: &Path,
         store_factories: &StoreFactories,
-    ) -> Self {
+    ) -> Result<Self, StoreLoadError> {
-        let store = Store::new(store_factories.load_backend(&repo_path.join("store")));
+        let store = Store::new(store_factories.load_backend(&repo_path.join("store"))?);
         let repo_settings = user_settings.with_repo(repo_path).unwrap();
-        let op_store = Arc::from(store_factories.load_op_store(&repo_path.join("op_store")));
+        let op_store = Arc::from(store_factories.load_op_store(&repo_path.join("op_store"))?);
         let op_heads_store =
-            Arc::from(store_factories.load_op_heads_store(&repo_path.join("op_heads")));
+            Arc::from(store_factories.load_op_heads_store(&repo_path.join("op_heads"))?);
         let index_store = Arc::new(IndexStore::load(repo_path.join("index")));
-        Self {
+        Ok(Self {
             repo_path: repo_path.to_path_buf(),
             repo_settings,
             store,
             op_store,
             op_heads_store,
             index_store,
-        }
+        })
     }
 
     pub fn repo_path(&self) -> &PathBuf {

lib/src/workspace.rs

@@ -27,6 +27,7 @@ use crate::op_store::{self, OpStore, OperationMetadata, WorkspaceId};
 use crate::operation::Operation;
 use crate::repo::{
     CheckOutCommitError, IoResultExt, PathError, ReadonlyRepo, Repo, RepoLoader, StoreFactories,
+    StoreLoadError,
 };
 use crate::settings::UserSettings;
 use crate::working_copy::WorkingCopy;
@@ -49,6 +50,8 @@ pub enum WorkspaceLoadError {
     RepoDoesNotExist(PathBuf),
     #[error("There is no Jujutsu repo in {0}")]
     NoWorkspaceHere(PathBuf),
+    #[error("Cannot read the repo: {0}")]
+    StoreLoadError(#[from] StoreLoadError),
     #[error("Repo path could not be interpreted as Unicode text")]
     NonUnicodePath,
     #[error(transparent)]
@@ -233,7 +236,8 @@ impl Workspace {
         store_factories: &StoreFactories,
     ) -> Result<Self, WorkspaceLoadError> {
         let loader = WorkspaceLoader::init(workspace_path)?;
-        Ok(loader.load(user_settings, store_factories)?)
+        let workspace = loader.load(user_settings, store_factories)?;
+        Ok(workspace)
     }
 
     pub fn workspace_root(&self) -> &PathBuf {
@@ -314,7 +318,7 @@ impl WorkspaceLoader {
         user_settings: &UserSettings,
         store_factories: &StoreFactories,
     ) -> Result<Workspace, WorkspaceLoadError> {
-        let repo_loader = RepoLoader::init(user_settings, &self.repo_dir, store_factories);
+        let repo_loader = RepoLoader::init(user_settings, &self.repo_dir, store_factories)?;
         let working_copy = WorkingCopy::load(
             repo_loader.store().clone(),
             self.workspace_root.clone(),

lib/tests/test_load_repo.rs

@@ -33,13 +33,13 @@ fn test_load_at_operation(use_git: bool) {
 
     // If we load the repo at head, we should not see the commit since it was
     // removed
-    let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default());
+    let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()).unwrap();
     let head_repo = loader.load_at_head(&settings).unwrap();
     assert!(!head_repo.view().heads().contains(commit.id()));
 
     // If we load the repo at the previous operation, we should see the commit since
     // it has not been removed yet
-    let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default());
+    let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()).unwrap();
     let old_repo = loader.load_at(repo.operation());
     assert!(old_repo.view().heads().contains(commit.id()));
 }

lib/testutils/src/lib.rs

@@ -153,6 +153,7 @@ impl TestWorkspace {
 
 pub fn load_repo_at_head(settings: &UserSettings, repo_path: &Path) -> Arc<ReadonlyRepo> {
     RepoLoader::init(settings, repo_path, &StoreFactories::default())
+        .unwrap()
         .load_at_head(settings)
         .unwrap()
 }

src/cli_util.rs

@@ -39,7 +39,7 @@ use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget, Works
 use jujutsu_lib::operation::Operation;
 use jujutsu_lib::repo::{
     CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo, RepoLoader,
-    RewriteRootCommit, StoreFactories,
+    RewriteRootCommit, StoreFactories, StoreLoadError,
 };
 use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
 use jujutsu_lib::revset::{
@@ -1203,6 +1203,16 @@ jj init --git-repo=.",
         )),
         WorkspaceLoadError::Path(e) => user_error(format!("{}: {}", e, e.error)),
         WorkspaceLoadError::NonUnicodePath => user_error(err.to_string()),
+        WorkspaceLoadError::StoreLoadError(err @ StoreLoadError::UnsupportedType { .. }) => {
+            CommandError::InternalError(format!(
+                "This version of the jj binary doesn't support this type of repo: {err}"
+            ))
+        }
+        WorkspaceLoadError::StoreLoadError(err @ StoreLoadError::ReadError { .. }) => {
+            CommandError::InternalError(format!(
+                "The repository appears broken or inaccessible: {err}"
+            ))
+        }
     }
 }
 

tests/common/mod.rs

@@ -103,6 +103,14 @@ impl TestEnvironment {
         self.normalize_output(&get_stderr_string(&assert))
     }
 
+    /// Run a `jj` command, check that it failed with code 255, and return its
+    /// stderr
+    #[must_use]
+    pub fn jj_cmd_internal_error(&self, current_dir: &Path, args: &[&str]) -> String {
+        let assert = self.jj_cmd(current_dir, args).assert().code(255).stdout("");
+        self.normalize_output(&get_stderr_string(&assert))
+    }
+
     pub fn env_root(&self) -> &Path {
         &self.env_root
     }

tests/test_global_opts.rs

@@ -14,6 +14,8 @@
 
 use std::ffi::OsString;
 
+use itertools::Itertools;
+
 use crate::common::{get_stderr_string, TestEnvironment};
 
 pub mod common;
@@ -172,6 +174,35 @@ fn test_no_workspace_directory() {
     "###);
 }
 
+#[test]
+fn test_broken_repo_structure() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    let store_type_path = repo_path
+        .join(".jj")
+        .join("repo")
+        .join("store")
+        .join("type");
+
+    // Test the error message when the commit backend is of unknown type.
+    std::fs::write(&store_type_path, "unknown").unwrap();
+    let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Internal error: This version of the jj binary doesn't support this type of repo: Unsupported commit backend type 'unknown'
+    "###);
+
+    // Test the error message when the file indicating the commit backend type
+    // cannot be read.
+    std::fs::remove_file(&store_type_path).unwrap();
+    std::fs::create_dir(&store_type_path).unwrap();
+    let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]);
+    // Trim off the OS-specific error message.
+    let stderr = stderr.split(':').take(3).join(":");
+    insta::assert_snapshot!(stderr, @"Internal error: The repository appears broken or inaccessible: Failed to read commit backend type");
+}
+
 #[test]
 fn test_color_config() {
     let mut test_env = TestEnvironment::default();