Merge pull request #13 from thunderstore-io/error-refactor

refactor error variants to be more module-specific

Author: ethangreen-dev

src/error.rs

@@ -1,141 +1,109 @@
 use std::path::{Path, PathBuf};
 
-use crate::ts::version::Version;
+use crate::ts::error::ApiError;
+
+use crate::game::error::GameError;
+use crate::package::error::PackageError;
+use crate::project::error::ProjectError;
 
-#[allow(clippy::enum_variant_names)]
 #[derive(Debug, thiserror::Error)]
 #[repr(u32)]
 pub enum Error {
-    #[error("An API error occurred.")]
-    ApiError {
-        source: reqwest::Error,
-        response_body: Option<String>,
-    } = 1,
+    #[error("{0}")]
+    Game(#[from] GameError),
 
-    #[error("A game import error occurred.")]
-    GameImportError(#[from] crate::game::import::Error),
+    #[error("{0}")]
+    Package(#[from] PackageError),
 
-    #[error("The file at {0} does not exist or is otherwise not accessible.")]
-    FileNotFound(PathBuf),
+    #[error("{0}")]
+    Project(#[from] ProjectError),
+
+    #[error("{0}")]
+    Api(#[from] ApiError),
+
+    #[error("{0}")]
+    Io(#[from] IoError),
+
+    #[error("{0}")]
+    JsonParse(#[from] serde_json::Error),
 
-    #[error("The directory at {0} does not exist or is otherwise not accessible.")]
-    DirectoryNotFound(PathBuf),
+    #[error("{0}")]
+    TomlDeserialize(#[from] toml::de::Error),
 
-    #[error("A network error occurred while sending an API request.")]
-    NetworkError(#[from] reqwest::Error),
+    #[error("{0}")]
+    TomlSerialize(#[from] toml::ser::Error),
+}
 
-    #[error("The path at {0} is actually a file.")]
-    ProjectDirIsFile(PathBuf),
+#[derive(Debug, thiserror::Error)]
+pub enum IoError {
+    #[error("A file IO error occured: {0}.")]
+    Native(std::io::Error, Option<PathBuf>),
 
-    #[error("A project configuration already exists at {0}.")]
-    ProjectAlreadyExists(PathBuf),
+    #[error("File not found: {0}.")]
+    FileNotFound(PathBuf),
 
-    #[error("A generic IO error occurred: {0}")]
-    GenericIoError(#[from] std::io::Error),
+    #[error("Expected directory at '{0}', got file.")]
+    DirectoryIsFile(PathBuf),
 
-    #[error("A file IO error occurred at path {0}: {1}")]
-    FileIoError(PathBuf, std::io::Error),
+    #[error("Directory not found: {0}.")]
+    DirNotFound(PathBuf),
 
-    #[error("Invalid version.")]
-    InvalidVersion(#[from] crate::ts::version::VersionParseError),
+    #[error("{0}")]
+    DirWalker(walkdir::Error),
 
-    #[error("Failed to read project file. {0}")]
-    FailedDeserializeProject(#[from] toml::de::Error),
+    #[error("Failed to find file '{0}' within the directory '{1}.")]
+    FailedFileSearch(String, PathBuf),
 
-    #[error("No project exists at the path {0}.")]
-    NoProjectFile(PathBuf),
+    #[error("Failed to read subkey at '{0}'.")]
+    RegistrySubkeyRead(String),
+
+    #[error("Failed to read value with name '{0}' at key '{1}'.")]
+    RegistryValueRead(String, String),
 
     #[error("Failed modifying zip file: {0}.")]
     ZipError(#[from] zip::result::ZipError),
+}
 
-    #[error("Project is missing required table '{0}'.")]
-    MissingTable(&'static str),
-
-    #[error("Missing repository url.")]
-    MissingRepository,
-
-    #[error("Missing auth token.")]
-    MissingAuthToken,
-
-    #[error("The game identifier '{0}' does not exist within the ecosystem schema.")]
-    InvalidGameId(String),
-
-    #[error("An error occurred while parsing JSON: {0}")]
-    JsonParserError(#[from] serde_json::Error),
-
-    #[error("An error occured while serializing TOML: {0}")]
-    TomlSerializer(#[from] toml::ser::Error),
-
-    #[error("The installer does not contain a valid manifest.")]
-    InstallerNoManifest,
-
-    #[error(
-        "The installer executable for the current OS and architecture combination does not exist."
-    )]
-    InstallerNotExecutable,
-
-    #[error(
-        "
-        The installer '{package_id}' does not support the current tcli installer protocol.
-            Expected: {our_version:#?}
-            Recieved: {given_version:#?}
-    "
-    )]
-    InstallerBadVersion {
-        package_id: String,
-        given_version: Version,
-        our_version: Version,
-    },
-
-    #[error(
-        "The installer '{package_id}' did not respond correctly:
-        \t{message}"
-    )]
-    InstallerBadResponse { package_id: String, message: String },
-
-    #[error("The installer returned an error:\n\t{message}")]
-    InstallerError { message: String },
-
-    #[error(
-        "The provided game id '{0}' does not exist or has not been imported into this profile."
-    )]
-    BadGameId(String),
-
-    #[error("The Steam app with id '{0}' could not be found.")]
-    SteamAppNotFound(u32),
+impl From<std::io::Error> for Error {
+    fn from(value: std::io::Error) -> Self {
+        Self::Io(IoError::Native(value, None))
+    }
+}
+
+impl From<reqwest::Error> for Error {
+    fn from(value: reqwest::Error) -> Self {
+        Self::Api(ApiError::BadRequest { source: value, response_body: None })
+    }
+}
+
+impl From<zip::result::ZipError> for Error {
+    fn from(value: zip::result::ZipError) -> Self {
+        Self::Io(IoError::ZipError(value))
+    }
 }
 
 pub trait IoResultToTcli<R> {
-    fn map_fs_error(self, path: impl AsRef<Path>) -> Result<R, Error>;
+    fn map_fs_error(self, path: impl AsRef<Path>) -> Result<R, IoError>;
 }
 
 impl<R> IoResultToTcli<R> for Result<R, std::io::Error> {
-    fn map_fs_error(self, path: impl AsRef<Path>) -> Result<R, Error> {
-        self.map_err(|e| Error::FileIoError(path.as_ref().into(), e))
+    fn map_fs_error(self, path: impl AsRef<Path>) -> Result<R, IoError> {
+        self.map_err(|e| IoError::Native(e, Some(path.as_ref().into())))
     }
 }
 
 pub trait ReqwestToTcli: Sized {
-    async fn error_for_status_tcli(self) -> Result<Self, Error>;
+    async fn error_for_status_tcli(self) -> Result<Self, ApiError>;
 }
 
 impl ReqwestToTcli for reqwest::Response {
-    async fn error_for_status_tcli(self) -> Result<Self, Error> {
+    async fn error_for_status_tcli(self) -> Result<Self, ApiError> {
         match self.error_for_status_ref() {
             Ok(_) => Ok(self),
-            Err(err) => Err(Error::ApiError {
+            Err(err) => Err(ApiError::BadRequest {
                 source: err,
                 response_body: self.text().await.ok(),
             }),
         }
     }
 }
-
-impl From<walkdir::Error> for Error {
-    fn from(value: walkdir::Error) -> Self {
-        Self::FileIoError(
-            value.path().unwrap_or(Path::new("")).into(),
-            value.into_io_error().unwrap(),
-        )
-    }
-}

src/game/error.rs

@@ -0,0 +1,30 @@
+use std::path::PathBuf;
+
+#[derive(Debug, thiserror::Error)]
+#[repr(u32)]
+pub enum GameError {
+    #[error("The game '{0}' is not supported by platform '{1}'.")]
+    NotSupported(String, String),
+
+    #[error("Could not find game with id '{0}' within the ecosystem schema.")]
+    BadGameId(String),
+
+    #[error("Could not find the game '{0}' installed via platform '{1}'.")]
+    NotFound(String, String),
+
+    #[error("Could not find any of '{possible_names:?}' in base directory: '{base_path}'.")]
+    ExeNotFound { possible_names: Vec<String>, base_path: PathBuf},
+
+    #[error("The Steam library could not be automatically found.")]
+    SteamDirNotFound,
+
+    #[error("The path '{0}' does not refer to a valid Steam directory.")]
+    SteamDirBadPath(PathBuf),
+
+    #[error("The app with id '{0}' could not be found in the Steam instance at '{1}'.")]
+    SteamAppNotFound(u32, PathBuf),
+
+    // This should probably live elsewhere but it's fine here for now.
+    #[error("An error occured while fetching the ecosystem schema.")]
+    EcosystemSchema,
+}

src/game/import/ea.rs

@@ -1,6 +1,8 @@
 use std::path::PathBuf;
 
-use super::{Error, GameImporter};
+use super::GameImporter;
+use crate::error::Error;
+use crate::game::error::GameError;
 use crate::game::import::ImportBase;
 use crate::game::registry::{ActiveDistribution, GameData};
 use crate::ts::v1::models::ecosystem::GameDefPlatform;
@@ -34,8 +36,10 @@ impl GameImporter for EaImporter {
             .clone()
             .or_else(|| super::find_game_exe(&r2mm.exe_names, &game_dir))
             .ok_or_else(|| {
-                super::Error::ExeNotFound(base.game_def.label.clone(), game_dir.clone())
-            })?;
+                GameError::ExeNotFound {
+                    possible_names: r2mm.exe_names.clone(),
+                    base_path: game_dir.clone(),
+            }})?;
         let dist = ActiveDistribution {
             dist: GameDefPlatform::Origin {
                 identifier: self.ident.to_string(),

src/game/import/egs.rs

@@ -3,7 +3,9 @@ use std::path::PathBuf;
 
 use serde::{Deserialize, Serialize};
 
-use super::{Error, GameImporter, ImportBase};
+use super::{GameImporter, ImportBase};
+use crate::error::{IoError, Error};
+use crate::game::error::GameError;
 use crate::game::registry::{ActiveDistribution, GameData};
 use crate::ts::v1::models::ecosystem::GameDefPlatform;
 use crate::util::reg::{self, HKey};
@@ -42,7 +44,7 @@ impl GameImporter for EgsImporter {
         let manifests_dir = PathBuf::from(value).join("Manifests");
 
         if !manifests_dir.exists() {
-            Err(Error::DirNotFound(manifests_dir.clone()))?;
+            Err(IoError::DirNotFound(manifests_dir.clone()))?;
         }
 
         // Manifest files are JSON files with .item extensions.
@@ -68,7 +70,7 @@ impl GameImporter for EgsImporter {
                     None
                 }
             })
-            .ok_or_else(|| super::Error::NotFound(game_label.clone(), "EGS".to_string()))?;
+            .ok_or_else(|| GameError::NotFound(game_label.clone(), "EGS".to_string()))?;
 
         let r2mm = base.game_def.r2modman.as_ref().expect(
             "Expected a valid r2mm field in the ecosystem schema, got nothing. This is a bug.",
@@ -80,8 +82,10 @@ impl GameImporter for EgsImporter {
             .clone()
             .or_else(|| super::find_game_exe(&r2mm.exe_names, &game_dir))
             .ok_or_else(|| {
-                super::Error::ExeNotFound(base.game_def.label.clone(), game_dir.clone())
-            })?;
+                GameError::ExeNotFound {
+                    possible_names: r2mm.exe_names.clone(),
+                    base_path: game_dir.clone(),
+            }})?;
         let dist = ActiveDistribution {
             dist: GameDefPlatform::Other,
             game_dir: game_dir.to_path_buf(),

src/game/import/gamepass.rs

@@ -1,7 +1,8 @@
 use std::path::PathBuf;
 
-use super::Error;
 use super::{GameImporter, ImportBase};
+use crate::error::Error;
+use crate::game::error::GameError;
 use crate::game::registry::{ActiveDistribution, GameData};
 use crate::ts::v1::models::ecosystem::GameDefPlatform;
 use crate::util::reg::{self, HKey};
@@ -26,7 +27,7 @@ impl GameImporter for GamepassImporter {
             .into_iter()
             .find(|x| x.key.starts_with(&self.ident))
             .ok_or_else(|| {
-                super::Error::NotFound(base.game_def.label.clone(), "Gamepass".to_string())
+                GameError::NotFound(base.game_def.label.clone(), "Gamepass".to_string())
             })?
             .val
             .replace('\"', "");
@@ -35,7 +36,7 @@ impl GameImporter for GamepassImporter {
             .into_iter()
             .next()
             .ok_or_else(|| {
-                super::Error::NotFound(base.game_def.label.clone(), "Gamepass".to_string())
+                GameError::NotFound(base.game_def.label.clone(), "Gamepass".to_string())
             })?;
         let game_dir = PathBuf::from(reg::get_value_at(HKey::LocalMachine, &game_root, "Root")?);
 
@@ -49,8 +50,10 @@ impl GameImporter for GamepassImporter {
             .clone()
             .or_else(|| super::find_game_exe(&r2mm.exe_names, &game_dir))
             .ok_or_else(|| {
-                super::Error::ExeNotFound(base.game_def.label.clone(), game_dir.clone())
-            })?;
+                GameError::ExeNotFound {
+                    possible_names: r2mm.exe_names.clone(),
+                    base_path: game_dir.clone(),
+            }})?;
         let dist = ActiveDistribution {
             dist: GameDefPlatform::GamePass {
                 identifier: self.ident.to_string(),