From 82e52bc8f9a35721dfcdfb613c76b94980a68a0a Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Wed, 16 Apr 2025 11:03:36 +0200 Subject: [PATCH] feat(server): add basic cli error handling to avoid unwraps --- server/src/cli/gpo.rs | 26 +++++++++++----------- server/src/cli/mod.rs | 49 +++++++++++++++++++++++++++++++---------- server/src/cli/serve.rs | 34 +++++++++++++++++++--------- server/src/main.rs | 9 +++++++- 4 files changed, 81 insertions(+), 37 deletions(-) diff --git a/server/src/cli/gpo.rs b/server/src/cli/gpo.rs index c50edfb..1cc2f65 100644 --- a/server/src/cli/gpo.rs +++ b/server/src/cli/gpo.rs @@ -1,5 +1,7 @@ use clap::Subcommand; +use super::CliError; + #[derive(Subcommand)] pub enum Command { /// Add devices of a specific user to the same sync group @@ -12,26 +14,24 @@ pub enum Command { } impl Command { - pub fn run(&self, config: &crate::config::Config) -> u8 { + pub fn run(&self, config: &crate::config::Config) -> Result<(), CliError> { let store = gpodder_sqlite::SqliteRepository::from_path(config.data_dir.join(crate::DB_FILENAME)) - .unwrap(); + .map_err(|err| CliError::from(err).msg("failed to initialize Sqlite repository"))?; let store = gpodder::GpodderRepository::new(store); match self { Self::Sync { username, devices } => { - let user = store.get_user(username).unwrap(); - store - .update_device_sync_status( - &user, - vec![devices.iter().map(|s| s.as_ref()).collect()], - Vec::new(), - ) - .unwrap(); + let user = store.get_user(username)?; + store.update_device_sync_status( + &user, + vec![devices.iter().map(|s| s.as_ref()).collect()], + Vec::new(), + )?; } Self::Devices { username } => { - let user = store.get_user(username).unwrap(); - let devices = store.devices_for_user(&user).unwrap(); + let user = store.get_user(username)?; + let devices = store.devices_for_user(&user)?; for device in devices { println!("{} ({} subscriptions)", device.id, device.subscriptions); @@ -39,6 +39,6 @@ impl Command { } } - 0 + Ok(()) } } diff --git a/server/src/cli/mod.rs b/server/src/cli/mod.rs index 5df94a2..8493451 100644 --- a/server/src/cli/mod.rs +++ b/server/src/cli/mod.rs @@ -85,8 +85,40 @@ pub enum Command { Gpo(gpo::Command), } +pub struct CliError { + msg: Option, + err: Box, +} + +impl From for CliError { + fn from(error: E) -> Self { + Self { + msg: None, + err: Box::new(error), + } + } +} + +impl std::fmt::Display for CliError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(msg) = &self.msg { + write!(f, "{msg}: ")?; + } + + write!(f, "{}", self.err) + } +} + +impl CliError { + pub fn msg(mut self, msg: impl AsRef) -> Self { + self.msg = Some(String::from(msg.as_ref())); + + self + } +} + impl Cli { - pub fn run(&self) -> u8 { + pub fn run(&self) -> Result<(), CliError> { let mut figment = Figment::new().merge(Serialized::defaults(crate::config::Config::default())); @@ -94,18 +126,11 @@ impl Cli { figment = figment.merge(Toml::file(config_path)); } - let config = figment + let config: crate::config::Config = figment .merge(Env::prefixed("OTTER_")) - .merge(Serialized::defaults(self.config.clone())); - - let config = match config.extract::() { - Ok(config) => config, - Err(err) => { - eprintln!("{}", err); - - return 1; - } - }; + .merge(Serialized::defaults(self.config.clone())) + .extract() + .map_err(|err| CliError::from(err).msg("failed to read config"))?; match &self.cmd { Command::Serve => serve::serve(&config), diff --git a/server/src/cli/serve.rs b/server/src/cli/serve.rs index 639bafa..4bb000a 100644 --- a/server/src/cli/serve.rs +++ b/server/src/cli/serve.rs @@ -2,18 +2,22 @@ use std::{sync::Arc, time::Duration}; use crate::{config::NetConfig, server}; -pub fn serve(config: &crate::config::Config) -> u8 { +use super::CliError; + +pub fn serve(config: &crate::config::Config) -> Result<(), CliError> { tracing_subscriber::fmt() .with_max_level(tracing::Level::from(config.log_level)) .init(); tracing::info!("Initializing database and running migrations"); - // TODO remove unwraps let store = gpodder_sqlite::SqliteRepository::from_path(config.data_dir.join(crate::DB_FILENAME)) - .unwrap(); + .map_err(|err| CliError::from(err).msg("failed to initialize Sqlite repository"))?; + + // SAFETY this runs on statically embedded templates, and should therefore never fail let tera = crate::web::initialize_tera().unwrap(); + let store = gpodder::GpodderRepository::new(store); let ctx = server::Context { @@ -25,7 +29,7 @@ pub fn serve(config: &crate::config::Config) -> u8 { let rt = tokio::runtime::Builder::new_multi_thread() .enable_all() .build() - .unwrap(); + .map_err(|err| CliError::from(err).msg("failed to initialize Tokio runtime"))?; // Spawn the session cleanup background task let session_removal_duration = Duration::from_secs(config.session_cleanup_interval); @@ -59,11 +63,16 @@ pub fn serve(config: &crate::config::Config) -> u8 { tracing::info!("Listening on TCP address {address}"); rt.block_on(async { - let listener = tokio::net::TcpListener::bind(address).await.unwrap(); + let listener = tokio::net::TcpListener::bind(address) + .await + .map_err(|err| CliError::from(err).msg("failed to bind TCP listener"))?; + axum::serve(listener, app.into_make_service()) .await - .unwrap() - }); + .map_err(|err| { + CliError::from(err).msg("error occured while running Axum server") + }) + })?; } NetConfig::Unix { path } => { // Try to remove the socket file first if it exists @@ -76,14 +85,17 @@ pub fn serve(config: &crate::config::Config) -> u8 { tracing::info!("Listening on Unix socket {:?}", path); rt.block_on(async { - let listener = tokio::net::UnixListener::bind(path).unwrap(); + let listener = tokio::net::UnixListener::bind(path) + .map_err(|err| CliError::from(err).msg("failed to bind Unix listener"))?; axum::serve(listener, app.into_make_service()) .await - .unwrap() - }); + .map_err(|err| { + CliError::from(err).msg("error occured while running Axum server") + }) + })?; } } - 0 + Ok(()) } diff --git a/server/src/main.rs b/server/src/main.rs index fa680ad..f87b018 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -30,5 +30,12 @@ impl ErrorExt for E {} fn main() -> ExitCode { let args = cli::Cli::parse(); - ExitCode::from(args.run()) + match args.run() { + Ok(()) => ExitCode::SUCCESS, + Err(err) => { + eprintln!("{}", err); + + ExitCode::FAILURE + } + } }