From 2127782b742e40cf4e604d6443c55d5da996a6e1 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Wed, 27 Aug 2025 18:01:27 +0200 Subject: [PATCH] term: Remove dependency on `anyhow` As a library crate, it is bad to return such generic errors. To get there, errors from `inquire` are downcasted. This allowed to clean up the public interface of `radicle-term`, so that it does also not leak the `inquire::InquireError` in its public API. --- Cargo.lock | 1 - Cargo.toml | 1 - crates/radicle-cli/Cargo.toml | 2 +- crates/radicle-cli/src/commands/auth.rs | 25 +-- crates/radicle-cli/src/commands/init.rs | 19 ++- .../radicle-cli/src/commands/node/control.rs | 7 +- crates/radicle-cli/src/terminal/io.rs | 9 +- crates/radicle-term/Cargo.toml | 1 - crates/radicle-term/src/io.rs | 147 +++++++++++++----- crates/radicle/src/identity/project.rs | 6 + 10 files changed, 157 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7993d575..e0859e38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2991,7 +2991,6 @@ name = "radicle-term" version = "0.15.0" dependencies = [ "anstyle-query", - "anyhow", "crossbeam-channel", "crossterm 0.29.0", "git2", diff --git a/Cargo.toml b/Cargo.toml index a8661094..5a379a9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ rust-version = "1.81.0" [workspace.dependencies] amplify = { version = "4.0.0", default-features = false } -anyhow = "1" bstr = "1.3" bytes = "1" chrono = { version = "0.4.26", default-features = false } diff --git a/crates/radicle-cli/Cargo.toml b/crates/radicle-cli/Cargo.toml index be44059e..459dddb2 100644 --- a/crates/radicle-cli/Cargo.toml +++ b/crates/radicle-cli/Cargo.toml @@ -14,7 +14,7 @@ name = "rad" path = "src/main.rs" [dependencies] -anyhow = { workspace = true } +anyhow = "1" chrono = { workspace = true, features = ["clock", "std"] } dunce = { workspace = true } git-ref-format = { version = "0.3.0", features = ["macro"] } diff --git a/crates/radicle-cli/src/commands/auth.rs b/crates/radicle-cli/src/commands/auth.rs index 18de1652..881e88ec 100644 --- a/crates/radicle-cli/src/commands/auth.rs +++ b/crates/radicle-cli/src/commands/auth.rs @@ -1,6 +1,5 @@ #![allow(clippy::or_fun_call)] use std::ffi::OsString; -use std::ops::Not as _; use std::str::FromStr; use anyhow::{anyhow, Context}; @@ -90,26 +89,28 @@ pub fn init(options: Options) -> anyhow::Result<()> { term::blank(); } } else { - anyhow::bail!("a Git installation is required for Radicle to run"); + anyhow::bail!("A Git installation is required for Radicle to run."); } let alias: Alias = if let Some(alias) = options.alias { alias } else { let user = env::var("USER").ok().and_then(|u| Alias::from_str(&u).ok()); - term::input( + let user = term::input( "Enter your alias:", user, Some("This is your node alias. You can always change it later"), - )? + )?; + + user.ok_or_else(|| anyhow::anyhow!("An alias is required for Radicle to run."))? }; let home = profile::home()?; let passphrase = if options.stdin { - term::passphrase_stdin() + Some(term::passphrase_stdin()?) } else { - term::passphrase_confirm("Enter a passphrase:", env::RAD_PASSPHRASE) - }?; - let passphrase = passphrase.trim().is_empty().not().then_some(passphrase); + term::passphrase_confirm("Enter a passphrase:", env::RAD_PASSPHRASE)? + }; + let passphrase = passphrase.filter(|passphrase| !passphrase.trim().is_empty()); let spinner = term::spinner("Creating your Ed25519 keypair..."); let profile = Profile::init(home, alias, passphrase.clone(), env::seed())?; let mut agent = true; @@ -187,8 +188,14 @@ pub fn authenticate(options: Options, profile: &Profile) -> anyhow::Result<()> { phrase } else if options.stdin { term::passphrase_stdin()? - } else { + } else if let Some(passphrase) = term::io::passphrase(term::io::PassphraseValidator::new(profile.keystore.clone()))? + { + passphrase + } else { + anyhow::bail!( + "A passphrase is required to read your Radicle key. Unable to continue." + ) }; register(&mut agent, profile, passphrase)?; diff --git a/crates/radicle-cli/src/commands/init.rs b/crates/radicle-cli/src/commands/init.rs index 1e966e0b..88580dea 100644 --- a/crates/radicle-cli/src/commands/init.rs +++ b/crates/radicle-cli/src/commands/init.rs @@ -243,18 +243,24 @@ pub fn init( let name: ProjectName = match options.name { Some(name) => name, None => { - let default = path.file_name().map(|f| f.to_string_lossy().to_string()); - term::input( + let default = path + .file_name() + .and_then(|f| f.to_str()) + .and_then(|f| ProjectName::try_from(f).ok()); + let name = term::input( "Name", default, Some("The name of your repository, eg. 'acme'"), - )? - .try_into()? + )?; + + name.ok_or_else(|| anyhow::anyhow!("A project name is required."))? } }; let description = match options.description { Some(desc) => desc, - None => term::input("Description", None, Some("You may leave this blank"))?, + None => { + term::input("Description", None, Some("You may leave this blank"))?.unwrap_or_default() + } }; let branch = match options.branch { Some(branch) => branch, @@ -262,7 +268,8 @@ pub fn init( "Default branch", Some(default_branch), Some("Please specify an existing branch"), - )?, + )? + .unwrap_or_default(), None => default_branch, }; let branch = RefString::try_from(branch.clone()) diff --git a/crates/radicle-cli/src/commands/node/control.rs b/crates/radicle-cli/src/commands/node/control.rs index 37ab71e4..fb4628a4 100644 --- a/crates/radicle-cli/src/commands/node/control.rs +++ b/crates/radicle-cli/src/commands/node/control.rs @@ -12,6 +12,7 @@ use localtime::LocalTime; use radicle::node; use radicle::node::{Address, ConnectResult, Handle as _, NodeId}; +use radicle::profile::env::RAD_PASSPHRASE; use radicle::Node; use radicle::{profile, Profile}; @@ -39,10 +40,12 @@ pub fn start( let validator = term::io::PassphraseValidator::new(profile.keystore.clone()); let passphrase = if let Some(phrase) = profile::env::passphrase() { phrase - } else if let Ok(phrase) = term::io::passphrase(validator) { + } else if let Some(phrase) = term::io::passphrase(validator)? { phrase } else { - anyhow::bail!("your radicle passphrase is required to start your node"); + anyhow::bail!( + "A passphrase is required to read your Radicle key in order to start the node. Unable to continue. Consider setting the environment variable `{RAD_PASSPHRASE}`." + ); }; Some((profile::env::RAD_PASSPHRASE, passphrase)) } else { diff --git a/crates/radicle-cli/src/terminal/io.rs b/crates/radicle-cli/src/terminal/io.rs index 40e580d7..c1538837 100644 --- a/crates/radicle-cli/src/terminal/io.rs +++ b/crates/radicle-cli/src/terminal/io.rs @@ -49,14 +49,13 @@ pub fn signer(profile: &Profile) -> anyhow::Result { return Ok(signer); } let validator = PassphraseValidator::new(profile.keystore.clone()); - let passphrase = match passphrase(validator) { - Ok(p) => p, - Err(inquire::InquireError::NotTTY) => { + let passphrase = match passphrase(validator)? { + Some(p) => p, + None => { anyhow::bail!( - "running in non-interactive mode, please set `{RAD_PASSPHRASE}` to unseal your key", + "A passphrase is required to read your Radicle key. Unable to continue. Consider setting the environment variable `{RAD_PASSPHRASE}`.", ) } - Err(e) => return Err(e.into()), }; let spinner = spinner("Unsealing key..."); let signer = MemorySigner::load(&profile.keystore, Some(passphrase))?; diff --git a/crates/radicle-term/Cargo.toml b/crates/radicle-term/Cargo.toml index 770db377..15504225 100644 --- a/crates/radicle-term/Cargo.toml +++ b/crates/radicle-term/Cargo.toml @@ -13,7 +13,6 @@ rust-version.workspace = true default = ["git2"] [dependencies] -anyhow = { workspace = true } anstyle-query = "1.0.0" crossterm = "0.29.0" inquire = { version = "0.7.4", default-features = false, features = ["crossterm", "editor"] } diff --git a/crates/radicle-term/src/io.rs b/crates/radicle-term/src/io.rs index 52fba20a..0fa1fb4b 100644 --- a/crates/radicle-term/src/io.rs +++ b/crates/radicle-term/src/io.rs @@ -8,6 +8,7 @@ use inquire::ui::{ErrorMessageRenderConfig, StyleSheet, Styled}; use inquire::validator; use inquire::InquireError; use inquire::{ui::Color, ui::RenderConfig, Confirm, CustomType, Password}; +use thiserror::Error; use zeroize::Zeroizing; use crate::format; @@ -223,54 +224,130 @@ pub fn abort(prompt: D) -> bool { ask(prompt, false) } -pub fn input(message: &str, default: Option, help: Option<&str>) -> anyhow::Result +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum InputError { + #[error(transparent)] + Io(#[from] io::Error), + + #[error(transparent)] + Custom(Custom), +} + +impl From> for io::Error { + fn from(val: InputError) -> Self { + match val { + InputError::Io(err) => err, + InputError::Custom(_) => unreachable!("infallible cannot be constructed"), + } + } +} + +/// Prompts the user for input. If the user cancels the operation, +/// the operation is interrupted, or no suitable terminal is found, +/// then `Ok(None)` is returned. +pub fn input( + message: &str, + default: Option, + help: Option<&str>, +) -> Result, InputError> where - S: fmt::Display + std::str::FromStr + Clone, - E: fmt::Debug + fmt::Display, + T: fmt::Display + std::str::FromStr + Clone, + E: std::error::Error + Send + Sync + 'static, { - let mut input = CustomType::::new(message).with_render_config(*CONFIG); + let mut input = CustomType::::new(message).with_render_config(*CONFIG); input.default = default; input.help_message = help; - let value = input.prompt()?; + match input.prompt() { + Ok(value) => Ok(Some(value)), + Err(err) => handle_inquire_error(err), + } +} - Ok(value) +/// If the [`InquireError`] value is one of the variants: +/// [`InquireError::OperationCanceled`], [`InquireError::OperationInterrupted`], +/// [`InquireError::NotTTY`], then the returned result is `None` – note that no +/// `Some` value is returned. +/// +/// Otherwise, the error is converted into our own domain error: [`InputError`]. +fn handle_inquire_error(error: InquireError) -> Result, InputError> +where + E: std::error::Error + Send + Sync + 'static, +{ + use InquireError::*; + + let inner = match error { + OperationCanceled | OperationInterrupted | NotTTY => None, + InvalidConfiguration(err) => { + // This case not reachable, as long as the configuration passed + // to `prompt` is valid. + // The configuration is *mostly* taken from `CONFIG`, + // except for the added `CustomType` being prompted for. + // We demand that these must not depend on user input in + // a way that makes the configuration invalid. + // If this is the case, `CONFIG` should be reassessed, or + // the caller must control their input for the `CustomType` + // better. In any case, such errors are not recoverable, + // and certainly the user cannot do anything in that + // situation. Their input should not affect the config, + // that's the whole idea! + panic!("{err}") + } + IO(err) => Some(InputError::Io(err)), + Custom(err) => { + match err.downcast::() { + Ok(err) => Some(InputError::Custom(*err)), + Err(err) => { + // `inquire` guarantees that we do not end up here: + // https://github.com/mikaelmello/inquire/blob/4ac91f3e1fc8b29fc17845f9204ea1d1f9e335aa/README.md?plain=1#L109 + panic!("inquire returned an unexpected error: {err:?}") + } + } + } + }; + + match inner { + Some(err) => Err(err), + None => Ok(None), + } } pub fn passphrase( validate: V, -) -> Result { - Ok(Passphrase::from( - Password::new("Passphrase:") - .with_render_config(*CONFIG) - .with_display_mode(inquire::PasswordDisplayMode::Masked) - .without_confirmation() - .with_validator(validate) - .prompt()?, - )) -} - -pub fn passphrase_confirm>( - prompt: &str, - var: K, -) -> Result { - if let Ok(p) = env::var(var) { - Ok(Passphrase::from(p)) - } else { - Ok(Passphrase::from( - Password::new(prompt) - .with_render_config(*CONFIG) - .with_display_mode(inquire::PasswordDisplayMode::Masked) - .with_custom_confirmation_message("Repeat passphrase:") - .with_custom_confirmation_error_message("The passphrases don't match.") - .with_help_message("Leave this blank to keep your radicle key unencrypted") - .prompt()?, - )) +) -> io::Result> { + match Password::new("Passphrase:") + .with_render_config(*CONFIG) + .with_display_mode(inquire::PasswordDisplayMode::Masked) + .without_confirmation() + .with_validator(validate) + .prompt() + { + Ok(p) => Ok(Some(Passphrase::from(p))), + Err(err) => handle_inquire_error(err).map_err(InputError::into), } } -pub fn passphrase_stdin() -> Result { +pub fn passphrase_confirm>(prompt: &str, var: K) -> io::Result> { + if let Ok(p) = env::var(var) { + return Ok(Some(Passphrase::from(p))); + } + + match Password::new(prompt) + .with_render_config(*CONFIG) + .with_display_mode(inquire::PasswordDisplayMode::Masked) + .with_custom_confirmation_message("Repeat passphrase:") + .with_custom_confirmation_error_message("The passphrases don't match.") + .with_help_message("Leave this blank to keep your radicle key unencrypted") + .prompt() + { + Ok(p) => Ok(Some(Passphrase::from(p))), + Err(err) => handle_inquire_error(err).map_err(InputError::into), + } +} + +pub fn passphrase_stdin() -> io::Result { let mut input = String::new(); std::io::stdin().read_line(&mut input)?; @@ -287,4 +364,4 @@ where .with_render_config(*CONFIG); selection.with_starting_cursor(0).prompt() -} \ No newline at end of file +} diff --git a/crates/radicle/src/identity/project.rs b/crates/radicle/src/identity/project.rs index 3ff9412b..242288ef 100644 --- a/crates/radicle/src/identity/project.rs +++ b/crates/radicle/src/identity/project.rs @@ -29,6 +29,12 @@ pub enum ProjectError { #[serde(try_from = "String", into = "String")] pub struct ProjectName(String); +impl std::fmt::Display for ProjectName { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} + impl From for String { fn from(value: ProjectName) -> Self { value.0