From cb6de69ef6bfca672378b7e2ba1d1668145d5574 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Feb 2022 09:25:11 -0500 Subject: [PATCH] tor-config: Add HasKind support. This required a few new ErrorKinds. --- Cargo.lock | 1 + crates/tor-config/Cargo.toml | 2 ++ crates/tor-config/src/err.rs | 20 ++++++++++++++++++++ crates/tor-config/src/path.rs | 22 ++++++++++++++++++++++ crates/tor-error/src/lib.rs | 35 +++++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 3e385033c..a71614bd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2995,6 +2995,7 @@ dependencies = [ "serde", "shellexpand", "thiserror", + "tor-error", "tracing", "tracing-test", ] diff --git a/crates/tor-config/Cargo.toml b/crates/tor-config/Cargo.toml index fd7dc19bc..b13fdde43 100644 --- a/crates/tor-config/Cargo.toml +++ b/crates/tor-config/Cargo.toml @@ -15,6 +15,8 @@ default = ["expand-paths"] expand-paths = ["shellexpand", "directories"] [dependencies] +tor-error = { path="../tor-error", version = "0.0.1"} + thiserror = "1" derive_builder = "0.10" once_cell = "1" diff --git a/crates/tor-config/src/err.rs b/crates/tor-config/src/err.rs index 65025b2fd..22261b7fd 100644 --- a/crates/tor-config/src/err.rs +++ b/crates/tor-config/src/err.rs @@ -1,7 +1,15 @@ //! Declare error types. +use tor_error::{ErrorKind, HasKind}; + /// An error related to an option passed to Arti via a configuration /// builder. +// +// API NOTE: When possible, we should expose this error type rather than +// wrapping it in `TorError`. It can provide specific information about what +// part of the configuration was invalid. +// +// This is part of the public API. #[derive(Debug, Clone, thiserror::Error)] #[non_exhaustive] pub enum ConfigBuildError { @@ -59,6 +67,12 @@ impl ConfigBuildError { } } +impl HasKind for ConfigBuildError { + fn kind(&self) -> ErrorKind { + ErrorKind::InvalidConfig + } +} + /// An error caused when attempting to reconfigure an existing Arti client, or one of its modules. #[derive(Debug, Clone, thiserror::Error)] #[non_exhaustive] @@ -71,6 +85,12 @@ pub enum ReconfigureError { }, } +impl HasKind for ReconfigureError { + fn kind(&self) -> ErrorKind { + ErrorKind::InvalidConfigTransition + } +} + #[cfg(test)] mod test { #![allow(clippy::unwrap_used)] diff --git a/crates/tor-config/src/path.rs b/crates/tor-config/src/path.rs index 7ab7192c8..187dbd2b5 100644 --- a/crates/tor-config/src/path.rs +++ b/crates/tor-config/src/path.rs @@ -9,6 +9,8 @@ use directories::{BaseDirs, ProjectDirs}; use once_cell::sync::Lazy; use serde::Deserialize; +use tor_error::{ErrorKind, HasKind}; + /// A path in a configuration file: tilde expansion is performed, along /// with expansion of certain variables. /// @@ -63,6 +65,26 @@ pub enum CfgPathError { InvalidString(String), } +impl HasKind for CfgPathError { + fn kind(&self) -> ErrorKind { + use CfgPathError as E; + use ErrorKind as EK; + match self { + E::UnknownVar(_) | E::InvalidString(_) => EK::InvalidConfig, + E::NoProjectDirs | E::NoBaseDirs => EK::NoHomeDirectory, + E::BadUtf8(_) => { + // Arguably, this should be a new "unsupported config" type, + // since it isn't truly "invalid" to have a string with bad UTF8 + // when it's going to be used as a filename. + // + // With luck, however, this error will cease to exist when shellexpand + // improves its character-set handling. + EK::InvalidConfig + } + } + } +} + impl CfgPath { /// Create a new configuration path pub fn new(s: String) -> Self { diff --git a/crates/tor-error/src/lib.rs b/crates/tor-error/src/lib.rs index 1d40d30d7..36e2f7e80 100644 --- a/crates/tor-error/src/lib.rs +++ b/crates/tor-error/src/lib.rs @@ -112,6 +112,41 @@ pub enum ErrorKind { #[display(fmt = "operation timed out at exit")] ExitTimeout, + /// One or more configuration values were invalid or incompatible. + /// + /// This kind of error can happen if the user provides an invalid or badly + /// formatted configuration file, if some of the options in that file are + /// out of their ranges or unparsable, or if the options are not all + /// compatible with one another. It can also happen if configuration options + /// provided via APIs are out of range. + /// + /// If this occurs because of user configuration, it's probably best to tell + /// the user about the error. If it occurs because of API usage, it's + /// probably best to fix the code that causes the error. + #[display(fmt = "invalid configuration")] + InvalidConfig, + + /// Tried to change the configuration of a running Arti service in a way + /// that isn't supported. + /// + /// This kind of error can happen when you call a `reconfigure()` method on + /// a service (or part of a service) and the new configuration is not + /// compatible with the previous configuration. + #[display(fmt = "invalid configuration transition")] + InvalidConfigTransition, + + /// Tried to look up a directory depending on the user's home directory, but + /// the user's home directory isn't set or can't be found. + /// + /// This kind of error can also occur if we're running in an environment + /// where users don't have home directories. + /// + /// To resolve this kind of error, either move to an OS with home + /// directories, or make sure that all paths in the configuration are set + /// explicitly, and do not depend on any path variables. + #[display(fmt = "could not find a home directory")] + NoHomeDirectory, + /// Internal error (bug) in Arti. /// /// A supposedly impossible problem has arisen. This indicates a bug in