From 420f1ff42ae970fa610fc0200b3f2106320aa4f7 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Wed, 14 Jun 2023 20:22:10 +0200 Subject: [PATCH] implement error::unexpected, use it everywhere This is an improved form of the previous `error::Unexpected` type, now with more capabilities and generally better naming. --- TODO | 2 - src/domain/acme/manager.rs | 100 +++++++++++++++---------- src/domain/acme/store.rs | 147 ++++++++++++++++++++++++------------- src/domain/checker.rs | 18 ++--- src/domain/config.rs | 53 +++++++------ src/domain/manager.rs | 39 +++++----- src/error.rs | 57 +------------- src/error/unexpected.rs | 128 ++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/main.rs | 21 +++--- src/origin.rs | 4 +- src/origin/store.rs | 9 ++- src/origin/store/git.rs | 97 +++++++++++++++--------- src/util.rs | 11 +++ 14 files changed, 437 insertions(+), 250 deletions(-) create mode 100644 src/error/unexpected.rs create mode 100644 src/util.rs diff --git a/TODO b/TODO index afbad36..8eef59c 100644 --- a/TODO +++ b/TODO @@ -1,3 +1 @@ -- expect statements (pretend it's "expected", not "expect") -- map_unexpected annotation string - clean up main a lot diff --git a/src/domain/acme/manager.rs b/src/domain/acme/manager.rs index d0bd4ee..353717c 100644 --- a/src/domain/acme/manager.rs +++ b/src/domain/acme/manager.rs @@ -1,18 +1,17 @@ use std::{future, pin, sync, time}; use crate::domain::{self, acme}; -use crate::error; -use crate::error::{MapUnexpected, ToUnexpected}; +use crate::error::unexpected::{self, Intoable, Mappable}; const LETS_ENCRYPT_URL: &str = "https://acme-v02.api.letsencrypt.org/directory"; pub type GetHttp01ChallengeKeyError = acme::store::GetHttp01ChallengeKeyError; #[mockall::automock( - type SyncDomainFuture=future::Ready>; + type SyncDomainFuture=future::Ready>; )] pub trait Manager { - type SyncDomainFuture<'mgr>: future::Future> + type SyncDomainFuture<'mgr>: future::Future> + Send + Unpin + 'mgr @@ -37,14 +36,14 @@ where pub async fn new( store: Store, contact_email: &str, -) -> Result +) -> Result where Store: acme::store::BoxedStore, { let dir = acme2::DirectoryBuilder::new(LETS_ENCRYPT_URL.to_string()) .build() .await - .map_unexpected()?; + .or_unexpected_while("creating acme2 directory builder")?; let mut contact = String::from("mailto:"); contact.push_str(contact_email); @@ -55,17 +54,29 @@ where match store.get_account_key() { Ok(account_key) => { - builder.private_key((&account_key).try_into().map_unexpected()?); + builder.private_key( + (&account_key) + .try_into() + .or_unexpected_while("parsing private key")?, + ); } Err(acme::store::GetAccountKeyError::NotFound) => (), - Err(acme::store::GetAccountKeyError::Unexpected(err)) => return Err(err.to_unexpected()), + Err(acme::store::GetAccountKeyError::Unexpected(err)) => return Err(err.into_unexpected()), } - let account = builder.build().await.map_unexpected()?; - let account_key: acme::PrivateKey = - account.private_key().as_ref().try_into().map_unexpected()?; + let account = builder + .build() + .await + .or_unexpected_while("building account")?; + let account_key: acme::PrivateKey = account + .private_key() + .as_ref() + .try_into() + .or_unexpected_while("parsing private key back out")?; - store.set_account_key(&account_key).map_unexpected()?; + store + .set_account_key(&account_key) + .or_unexpected_while("storing account key")?; Ok(sync::Arc::new(ManagerImpl { store, account })) } @@ -76,7 +87,7 @@ impl Manager for sync::Arc> where Store: acme::store::BoxedStore, { - type SyncDomainFuture<'mgr> = pin::Pin> + Send + 'mgr>> + type SyncDomainFuture<'mgr> = pin::Pin> + Send + 'mgr>> where Self: 'mgr; fn sync_domain(&self, domain: domain::Name) -> Self::SyncDomainFuture<'_> { @@ -92,10 +103,10 @@ where .into_iter() .map(|cert| openssl::x509::X509::try_from(&cert)) .try_collect::>() - .map_unexpected()? + .or_unexpected_while("parsing x509 certs")? .into_iter() .reduce(|a, b| if a.not_after() < b.not_after() { a } else { b }) - .ok_or(error::Unexpected::from( + .ok_or(unexpected::Error::from( "expected there to be more than one cert", ))?; @@ -107,28 +118,34 @@ where log::info!("Fetching a new certificate for domain {}", domain.as_str()); let mut builder = acme2::OrderBuilder::new(self.account.clone()); builder.add_dns_identifier(domain.as_str().to_string()); - let order = builder.build().await.map_unexpected()?; + let order = builder + .build() + .await + .or_unexpected_while("building order")?; - let authorizations = order.authorizations().await.map_unexpected()?; + let authorizations = order + .authorizations() + .await + .or_unexpected_while("fetching authorizations")?; for auth in authorizations { let challenge = auth .get_challenge("http-01") - .ok_or(error::Unexpected::from("expected http-01 challenge"))?; + .ok_or(unexpected::Error::from("expected http-01 challenge"))?; let challenge_token = challenge .token .as_ref() - .ok_or(error::Unexpected::from("expected challenge to have token"))?; + .ok_or(unexpected::Error::from("expected challenge to have token"))?; let challenge_key = challenge .key_authorization() - .map_unexpected()? - .ok_or(error::Unexpected::from("expected challenge to have key"))?; + .or_unexpected_while("getting challenge key from authorization")? + .ok_or(unexpected::Error::from("expected challenge to have key"))?; self.store .set_http01_challenge_key(challenge_token, &challenge_key) - .map_unexpected()?; + .or_unexpected_while("storing challenge token")?; // At this point the manager is prepared to serve the challenge key via the // `get_http01_challenge_key` method. It is expected that there is some http @@ -141,7 +158,10 @@ where "Waiting for ACME challenge to be validated for domain {}", domain.as_str(), ); - let challenge = challenge.validate().await.map_unexpected()?; + let challenge = challenge + .validate() + .await + .or_unexpected_while("initiating challenge validation")?; // Poll the challenge every 5 seconds until it is in either the // `valid` or `invalid` state. @@ -150,12 +170,12 @@ where // no matter what the result is, clean up the challenge key self.store .del_http01_challenge_key(challenge_token) - .map_unexpected()?; + .or_unexpected_while("deleting challenge token")?; - let challenge = challenge_res.map_unexpected()?; + let challenge = challenge_res.or_unexpected_while("getting challenge status")?; if challenge.status != acme2::ChallengeStatus::Valid { - return Err(error::Unexpected::from( + return Err(unexpected::Error::from( format!( "expected challenge status to be valid, instead it was {:?}", challenge.status @@ -174,10 +194,10 @@ where let authorization = auth .wait_done(time::Duration::from_secs(5), 3) .await - .map_unexpected()?; + .or_unexpected_while("waiting for authorization status")?; if authorization.status != acme2::AuthorizationStatus::Valid { - return Err(error::Unexpected::from( + return Err(unexpected::Error::from( format!( "expected authorization status to be valid, instead it was {:?}", authorization.status, @@ -197,10 +217,10 @@ where let order = order .wait_ready(time::Duration::from_secs(5), 3) .await - .map_unexpected()?; + .or_unexpected_while("waiting for order to be ready")?; if order.status != acme2::OrderStatus::Ready { - return Err(error::Unexpected::from( + return Err(unexpected::Error::from( format!( "expected order status to be ready, instead it was {:?}", order.status, @@ -212,14 +232,16 @@ where // Generate an RSA private key for the certificate. let pkey = acme::PrivateKey::new(); - let acme2_pkey = (&pkey).try_into().map_unexpected()?; + let acme2_pkey = (&pkey) + .try_into() + .or_unexpected_while("parsing new private key")?; // Create a certificate signing request for the order, and request // the certificate. let order = order .finalize(acme2::Csr::Automatic(acme2_pkey)) .await - .map_unexpected()?; + .or_unexpected_while("finalizing order")?; // Poll the order every 5 seconds until it is in either the // `valid` or `invalid` state. Valid means that the certificate @@ -231,10 +253,10 @@ where let order = order .wait_done(time::Duration::from_secs(5), 3) .await - .map_unexpected()?; + .or_unexpected_while("waiting for order to be validated")?; if order.status != acme2::OrderStatus::Valid { - return Err(error::Unexpected::from( + return Err(unexpected::Error::from( format!( "expected order status to be valid, instead it was {:?}", order.status, @@ -248,17 +270,17 @@ where let cert = order .certificate() .await - .map_unexpected()? - .ok_or(error::Unexpected::from( + .or_unexpected_while("fetching certificate")? + .ok_or(unexpected::Error::from( "expected the order to return a certificate", ))? .into_iter() .map(|cert| acme::Certificate::try_from(cert.as_ref())) .try_collect::>() - .map_unexpected()?; + .or_unexpected_while("parsing certificate")?; if cert.len() <= 1 { - return Err(error::Unexpected::from( + return Err(unexpected::Error::from( format!( "expected more than one certificate to be returned, instead got {}", cert.len(), @@ -270,7 +292,7 @@ where log::info!("Certificate for {} successfully retrieved", domain.as_str()); self.store .set_certificate(domain.as_str(), pkey, cert) - .map_unexpected()?; + .or_unexpected_while("storing new cert")?; Ok(()) }) diff --git a/src/domain/acme/store.rs b/src/domain/acme/store.rs index d712ecd..1e0c7e3 100644 --- a/src/domain/acme/store.rs +++ b/src/domain/acme/store.rs @@ -1,10 +1,10 @@ use std::io::{Read, Write}; use std::str::FromStr; -use std::{fs, io, path, sync}; +use std::{fs, path, sync}; use crate::domain::acme::{Certificate, PrivateKey}; -use crate::error; -use crate::error::{MapUnexpected, ToUnexpected}; +use crate::error::unexpected::{self, Mappable}; +use crate::util; use hex::ToHex; use serde::{Deserialize, Serialize}; @@ -16,7 +16,7 @@ pub enum GetAccountKeyError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] @@ -25,7 +25,7 @@ pub enum GetHttp01ChallengeKeyError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] @@ -34,24 +34,24 @@ pub enum GetCertificateError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[mockall::automock] pub trait Store { - fn set_account_key(&self, k: &PrivateKey) -> Result<(), error::Unexpected>; + fn set_account_key(&self, k: &PrivateKey) -> Result<(), unexpected::Error>; fn get_account_key(&self) -> Result; - fn set_http01_challenge_key(&self, token: &str, key: &str) -> Result<(), error::Unexpected>; + fn set_http01_challenge_key(&self, token: &str, key: &str) -> Result<(), unexpected::Error>; fn get_http01_challenge_key(&self, token: &str) -> Result; - fn del_http01_challenge_key(&self, token: &str) -> Result<(), error::Unexpected>; + fn del_http01_challenge_key(&self, token: &str) -> Result<(), unexpected::Error>; fn set_certificate( &self, domain: &str, key: PrivateKey, cert: Vec, - ) -> Result<(), error::Unexpected>; + ) -> Result<(), unexpected::Error>; /// Returned vec is guaranteed to have len > 0 fn get_certificate( @@ -77,10 +77,17 @@ struct BoxedFSStore(sync::Arc); pub fn new( dir_path: &path::Path, -) -> Result { - fs::create_dir_all(dir_path).map_unexpected()?; - fs::create_dir_all(dir_path.join("http01_challenge_keys")).map_unexpected()?; - fs::create_dir_all(dir_path.join("certificates")).map_unexpected()?; +) -> Result { + vec![ + dir_path, + dir_path.join("http01_challenge_keys").as_ref(), + dir_path.join("certificates").as_ref(), + ] + .iter() + .map(|dir| { + fs::create_dir_all(dir).map_unexpected_while(|| format!("creating dir {}", dir.display())) + }) + .try_collect()?; Ok(BoxedFSStore(sync::Arc::new(FSStore { dir_path: dir_path.into(), @@ -113,47 +120,72 @@ impl BoxedFSStore { impl BoxedStore for BoxedFSStore {} impl Store for BoxedFSStore { - fn set_account_key(&self, k: &PrivateKey) -> Result<(), error::Unexpected> { - let mut file = fs::File::create(self.account_key_path()).map_unexpected()?; - file.write_all(k.to_string().as_bytes()).map_unexpected()?; + fn set_account_key(&self, k: &PrivateKey) -> Result<(), unexpected::Error> { + let path = self.account_key_path(); + { + let mut file = fs::File::create(&path).or_unexpected_while("creating file")?; + file.write_all(k.to_string().as_bytes()) + .or_unexpected_while("writing file") + } + .map_unexpected_while(|| format!("path is {}", path.display()))?; Ok(()) } fn get_account_key(&self) -> Result { - let mut file = fs::File::open(self.account_key_path()).map_err(|e| match e.kind() { - io::ErrorKind::NotFound => GetAccountKeyError::NotFound, - _ => e.to_unexpected().into(), - })?; + let path = self.account_key_path(); + { + let mut file = + match util::open_file(path.as_path()).or_unexpected_while("opening_file")? { + Some(file) => file, + None => return Err(GetAccountKeyError::NotFound), + }; - let mut key = String::new(); - file.read_to_string(&mut key).map_unexpected()?; + let mut key = String::new(); + file.read_to_string(&mut key) + .or_unexpected_while("reading file")?; - let key = PrivateKey::from_str(&key).map_unexpected()?; - Ok(key) + let key = PrivateKey::from_str(&key).or_unexpected_while("parsing private key")?; + + Ok::(key) + } + .map_unexpected_while(|| format!("path is {}", path.display())) + .map_err(|err| err.into()) } - fn set_http01_challenge_key(&self, token: &str, key: &str) -> Result<(), error::Unexpected> { - let mut file = fs::File::create(self.http01_challenge_key_path(token)).map_unexpected()?; - file.write_all(key.as_bytes()).map_unexpected()?; + fn set_http01_challenge_key(&self, token: &str, key: &str) -> Result<(), unexpected::Error> { + let path = self.http01_challenge_key_path(token); + { + let mut file = fs::File::create(path.as_path()).or_unexpected_while("creating file")?; + file.write_all(key.as_bytes()) + .or_unexpected_while("writing file") + } + .map_unexpected_while(|| format!("path is {}", path.display()))?; Ok(()) } fn get_http01_challenge_key(&self, token: &str) -> Result { - let mut file = - fs::File::open(self.http01_challenge_key_path(token)).map_err(|e| match e.kind() { - io::ErrorKind::NotFound => GetHttp01ChallengeKeyError::NotFound, - _ => e.to_unexpected().into(), - })?; + let path = self.http01_challenge_key_path(token); + { + let mut file = + match util::open_file(path.as_path()).or_unexpected_while("opening_file")? { + Some(file) => file, + None => return Err(GetHttp01ChallengeKeyError::NotFound), + }; - let mut key = String::new(); - file.read_to_string(&mut key).map_unexpected()?; + let mut key = String::new(); + file.read_to_string(&mut key) + .or_unexpected_while("reading file")?; - Ok(key) + Ok::(key) + } + .map_unexpected_while(|| format!("path is {}", path.display())) + .map_err(|err| err.into()) } - fn del_http01_challenge_key(&self, token: &str) -> Result<(), error::Unexpected> { - fs::remove_file(self.http01_challenge_key_path(token)).map_unexpected()?; - Ok(()) + fn del_http01_challenge_key(&self, token: &str) -> Result<(), unexpected::Error> { + let path = self.http01_challenge_key_path(token); + fs::remove_file(path.as_path()) + .map_unexpected_while(|| format!("path is {}", path.display())) } fn set_certificate( @@ -161,31 +193,42 @@ impl Store for BoxedFSStore { domain: &str, key: PrivateKey, cert: Vec, - ) -> Result<(), error::Unexpected> { + ) -> Result<(), unexpected::Error> { let to_store = StoredPKeyCert { private_key: key, cert, }; - let cert_file = fs::File::create(self.certificate_path(domain)).map_unexpected()?; - - serde_json::to_writer(cert_file, &to_store).map_unexpected()?; - - Ok(()) + let path = self.certificate_path(domain); + { + let cert_file = + fs::File::create(path.as_path()).or_unexpected_while("creating file")?; + serde_json::to_writer(cert_file, &to_store).or_unexpected_while("writing cert to file") + } + .map_unexpected_while(|| format!("path is {}", path.display())) } fn get_certificate( &self, domain: &str, ) -> Result<(PrivateKey, Vec), GetCertificateError> { - let file = fs::File::open(self.certificate_path(domain)).map_err(|e| match e.kind() { - io::ErrorKind::NotFound => GetCertificateError::NotFound, - _ => e.to_unexpected().into(), - })?; + let path = self.certificate_path(domain); + { + let file = match util::open_file(path.as_path()).or_unexpected_while("opening_file")? { + Some(file) => file, + None => return Err(GetCertificateError::NotFound), + }; - let stored: StoredPKeyCert = serde_json::from_reader(file).map_unexpected()?; + let stored: StoredPKeyCert = + serde_json::from_reader(file).or_unexpected_while("parsing json")?; - Ok((stored.private_key, stored.cert)) + Ok::<(PrivateKey, Vec), unexpected::Error>(( + stored.private_key, + stored.cert, + )) + } + .map_unexpected_while(|| format!("path is {}", path.display())) + .map_err(|err| err.into()) } } @@ -203,7 +246,7 @@ impl rustls::server::ResolvesServerCert for BoxedFSStore { } Err(GetCertificateError::Unexpected(err)) => Err(err), Ok((key, cert)) => { - match rustls::sign::any_supported_type(&key.into()).map_unexpected() { + match rustls::sign::any_supported_type(&key.into()).or_unexpected() { Err(err) => Err(err), Ok(key) => Ok(Some(sync::Arc::new(rustls::sign::CertifiedKey { cert: cert.into_iter().map(|cert| cert.into()).collect(), diff --git a/src/domain/checker.rs b/src/domain/checker.rs index 9fdd6c2..962457c 100644 --- a/src/domain/checker.rs +++ b/src/domain/checker.rs @@ -1,8 +1,8 @@ use std::net; use std::str::FromStr; -use crate::error::MapUnexpected; -use crate::{domain, error}; +use crate::domain; +use crate::error::unexpected::{self, Mappable}; use trust_dns_client::client::{AsyncClient, ClientHandle}; use trust_dns_client::rr::{DNSClass, Name, RData, RecordType}; @@ -14,7 +14,7 @@ pub enum NewDNSCheckerError { InvalidResolverAddress, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] @@ -26,7 +26,7 @@ pub enum CheckDomainError { ChallengeTokenNotSet, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } pub struct DNSChecker { @@ -46,7 +46,7 @@ pub async fn new( let stream = udp::UdpClientStream::::new(resolver_addr); - let (client, bg) = AsyncClient::connect(stream).await.map_unexpected()?; + let (client, bg) = AsyncClient::connect(stream).await.or_unexpected()?; tokio::spawn(bg); @@ -72,7 +72,7 @@ impl DNSChecker { .await .query(domain.clone(), DNSClass::IN, RecordType::A) .await - .map_unexpected()?; + .or_unexpected_while("querying A record")?; let records = response.answers(); @@ -91,9 +91,9 @@ impl DNSChecker { // check that the TXT record with the challenge token is correctly installed on the domain { let domain = Name::from_str("_domiply_challenge") - .map_unexpected()? + .or_unexpected_while("parsing TXT name")? .append_domain(domain) - .map_unexpected()?; + .or_unexpected_while("appending domain to TXT")?; let response = self .client @@ -101,7 +101,7 @@ impl DNSChecker { .await .query(domain, DNSClass::IN, RecordType::TXT) .await - .map_unexpected()?; + .or_unexpected_while("querying TXT record")?; let records = response.answers(); diff --git a/src/domain/config.rs b/src/domain/config.rs index 42069f3..ceaa33a 100644 --- a/src/domain/config.rs +++ b/src/domain/config.rs @@ -2,8 +2,8 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::{fs, io, sync}; -use crate::error::{MapUnexpected, ToUnexpected}; -use crate::{domain, error, origin}; +use crate::error::unexpected::{self, Intoable, Mappable}; +use crate::{domain, origin}; use hex::ToHex; use serde::{Deserialize, Serialize}; @@ -16,9 +16,9 @@ pub struct Config { } impl Config { - pub fn hash(&self) -> Result { + pub fn hash(&self) -> Result { let mut h = Sha256::new(); - serde_json::to_writer(&mut h, self).map_unexpected()?; + serde_json::to_writer(&mut h, self).or_unexpected()?; Ok(h.finalize().encode_hex::()) } } @@ -29,17 +29,17 @@ pub enum GetError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] pub enum SetError { #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } /// Used in the return from all_domains from Store. -pub type AllDomainsResult = Result; +pub type AllDomainsResult = Result; #[mockall::automock] pub trait Store { @@ -75,37 +75,46 @@ impl BoxedStore for sync::Arc {} impl Store for sync::Arc { fn get(&self, domain: &domain::Name) -> Result { - let config_file = - fs::File::open(self.config_file_path(domain)).map_err(|e| match e.kind() { - io::ErrorKind::NotFound => GetError::NotFound, - _ => e.to_unexpected().into(), - })?; + let path = self.config_file_path(domain); + let config_file = fs::File::open(path.as_path()).map_err(|e| match e.kind() { + io::ErrorKind::NotFound => GetError::NotFound, + _ => e + .into_unexpected_while(format!("opening {}", path.display())) + .into(), + })?; - let config = serde_json::from_reader(config_file).map_unexpected()?; + let config = serde_json::from_reader(config_file) + .map_unexpected_while(|| format!("json parsing {}", path.display()))?; Ok(config) } fn set(&self, domain: &domain::Name, config: &Config) -> Result<(), SetError> { - fs::create_dir_all(self.config_dir_path(domain)).map_unexpected()?; + let dir_path = self.config_dir_path(domain); + fs::create_dir_all(dir_path.as_path()) + .map_unexpected_while(|| format!("creating dir {}", dir_path.display()))?; - let config_file = fs::File::create(self.config_file_path(domain)).map_unexpected()?; + let file_path = self.config_file_path(domain); + let config_file = fs::File::create(file_path.as_path()) + .map_unexpected_while(|| format!("creating file {}", file_path.display()))?; - serde_json::to_writer(config_file, config).map_unexpected()?; + serde_json::to_writer(config_file, config) + .map_unexpected_while(|| format!("writing config to {}", file_path.display()))?; Ok(()) } fn all_domains(&self) -> AllDomainsResult>> { Ok(fs::read_dir(&self.dir_path) - .map_unexpected()? + .or_unexpected()? .map( |dir_entry_res: io::Result| -> AllDomainsResult { - let domain = dir_entry_res.map_unexpected()?.file_name(); - let domain = domain.to_str().ok_or_else(|| { - error::Unexpected::from("couldn't convert os string to &str") - })?; + let domain = dir_entry_res.or_unexpected()?.file_name(); + let domain = domain.to_str().ok_or(unexpected::Error::from( + "couldn't convert os string to &str", + ))?; - domain::Name::from_str(domain).map_unexpected() + domain::Name::from_str(domain) + .map_unexpected_while(|| format!("parsing {domain} as domain name")) }, ) .collect()) diff --git a/src/domain/manager.rs b/src/domain/manager.rs index b926148..37b37df 100644 --- a/src/domain/manager.rs +++ b/src/domain/manager.rs @@ -1,6 +1,6 @@ use crate::domain::{self, acme, checker, config}; -use crate::error::{MapUnexpected, ToUnexpected}; -use crate::{error, origin}; +use crate::error::unexpected::{self, Intoable, Mappable}; +use crate::origin; use std::{future, pin, sync}; @@ -10,7 +10,7 @@ pub enum GetConfigError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } impl From for GetConfigError { @@ -28,7 +28,7 @@ pub enum GetOriginError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } impl From for GetOriginError { @@ -49,7 +49,7 @@ pub enum SyncError { AlreadyInProgress, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } impl From for SyncError { @@ -79,7 +79,7 @@ pub enum SyncWithConfigError { ChallengeTokenNotSet, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } impl From for SyncWithConfigError { @@ -120,7 +120,7 @@ pub type AllDomainsResult = config::AllDomainsResult; #[mockall::automock( type Origin=origin::MockOrigin; type SyncWithConfigFuture=future::Ready>; - type SyncAllOriginsErrorsIter=Vec<(Option,error::Unexpected)>; + type SyncAllOriginsErrorsIter=Vec; )] pub trait Manager { type Origin<'mgr>: origin::Origin + 'mgr @@ -134,8 +134,7 @@ pub trait Manager { where Self: 'mgr; - type SyncAllOriginsErrorsIter<'mgr>: IntoIterator, error::Unexpected)> - + 'mgr + type SyncAllOriginsErrorsIter<'mgr>: IntoIterator + 'mgr where Self: 'mgr; @@ -149,7 +148,7 @@ pub trait Manager { config: config::Config, ) -> Self::SyncWithConfigFuture<'_>; - fn sync_all_origins(&self) -> Result, error::Unexpected>; + fn sync_all_origins(&self) -> Result, unexpected::Error>; fn get_acme_http01_challenge_key( &self, @@ -214,7 +213,7 @@ where type SyncWithConfigFuture<'mgr> = pin::Pin> + Send + 'mgr>> where Self: 'mgr; - type SyncAllOriginsErrorsIter<'mgr> = Box, error::Unexpected)> + 'mgr> + type SyncAllOriginsErrorsIter<'mgr> = Box + 'mgr> where Self: 'mgr; fn get_config(&self, domain: &domain::Name) -> Result { @@ -227,7 +226,7 @@ where .origin_store .get(config.origin_descr) // if there's a config there should be an origin, any error here is unexpected - .map_unexpected()?; + .or_unexpected()?; Ok(origin) } @@ -237,7 +236,9 @@ where config: config::Config, ) -> Self::SyncWithConfigFuture<'_> { Box::pin(async move { - let config_hash = config.hash().map_unexpected()?; + let config_hash = config + .hash() + .or_unexpected_while("calculating config hash")?; self.domain_checker .check_domain(&domain, &config_hash) @@ -256,12 +257,16 @@ where }) } - fn sync_all_origins(&self) -> Result, error::Unexpected> { - let iter = self.origin_store.all_descrs().map_unexpected()?.into_iter(); + fn sync_all_origins(&self) -> Result, unexpected::Error> { + let iter = self + .origin_store + .all_descrs() + .or_unexpected_while("fetching all origin descrs")? + .into_iter(); Ok(Box::from(iter.filter_map(|descr| { if let Err(err) = descr { - return Some((None, err.to_unexpected())); + return Some(err.into_unexpected()); } let descr = descr.unwrap(); @@ -270,7 +275,7 @@ where .origin_store .sync(descr.clone(), origin::store::Limits {}) { - return Some((Some(descr), err.to_unexpected())); + return Some(err.into_unexpected_while(format!("syncing store {:?}", descr))); } None diff --git a/src/error.rs b/src/error.rs index 57e9132..5dfb9bb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,56 +1 @@ -use std::error; -use std::fmt; -use std::fmt::Write; - -#[derive(Debug, Clone)] -/// Unexpected is a String which implements the Error trait. It is intended to be used in -/// situations where the caller is being given an error they can't really handle, except to pass it -/// along or log it. -/// -/// The error is intended to also implement Send + Sync + Clone, such that it is easy to use in -/// async situations. -pub struct Unexpected(String); - -impl fmt::Display for Unexpected { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Unexpected error occurred: {}", self.0) - } -} - -impl error::Error for Unexpected {} - -impl From<&str> for Unexpected { - fn from(s: &str) -> Self { - Unexpected(s.to_string()) - } -} - -pub trait ToUnexpected { - fn to_unexpected(&self) -> Unexpected; -} - -impl ToUnexpected for E { - fn to_unexpected(&self) -> Unexpected { - let mut w = String::new(); - write!(w, "{}", self.to_string()).expect("underlying error formatted correctly"); - - if let Some(src) = self.source() { - write!(w, " [{src}]").expect("underyling source formatted correctly"); - } - - Unexpected(w.to_string()) - } -} - -pub trait MapUnexpected { - fn map_unexpected(self) -> Result; -} - -impl MapUnexpected for Result { - fn map_unexpected(self) -> Result { - match self { - Ok(t) => Ok(t), - Err(err) => Err(err.to_unexpected()), - } - } -} +pub mod unexpected; diff --git a/src/error/unexpected.rs b/src/error/unexpected.rs new file mode 100644 index 0000000..ca31ab9 --- /dev/null +++ b/src/error/unexpected.rs @@ -0,0 +1,128 @@ +use std::fmt::Write; +use std::{error, fmt}; + +#[derive(Debug, Clone)] +/// Error is a String which implements the Error trait. It is intended to be used in +/// situations where the caller is being given an error they can't really handle, except to pass it +/// along or log it. +/// +/// The error is intended to also implement Send + Sync + Clone, such that it is easy to use in +/// async situations. +pub struct Error(String); + +impl Error { + fn from_displays(prefix: Option, body: &D2, source: Option) -> Error + where + D1: fmt::Display, + D2: fmt::Display + ?Sized, + D3: fmt::Display, + { + let mut w = String::new(); + + if let Some(prefix) = prefix { + write!(w, "{prefix}").expect("error writing prefix"); + } + + write!(w, "{body}").expect("error formatting body"); + + if let Some(source) = source { + write!(w, " [{source}]").expect("error formatting source"); + } + + Error(w.to_string()) + } + + fn from_err(prefix: Option, e: E) -> Error + where + E: error::Error, + D: fmt::Display, + { + return Error::from_displays(prefix, &e, e.source()); + } + + pub fn from(s: &str) -> Error { + Error::from_displays(None::, s, None::) + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Unexpected error occurred: {}", self.0) + } +} + +impl error::Error for Error {} + +pub trait Mappable { + /// or_unexpected returns an Err(Error) wrapping self's Err, or the Ok value of self. + fn or_unexpected(self) -> Result; + + /// or_unexpected_while is like or_unexpected, but will prefix the error message. The prefix + /// should be worded as if it started with the word "while", e.g.: `opening file {path}`. + fn or_unexpected_while(self, prefix: D) -> Result; + + /// map_unexpected_while is like or_unexpected_while, but uses a closure to produce the error + /// prefix. + fn map_unexpected_while(self, f: F) -> Result + where + F: FnOnce() -> D, + D: fmt::Display; +} + +fn map_unexpected_maybe_while( + res: Result, + prefix_fn: Option, +) -> Result +where + E: error::Error, + F: FnOnce() -> D, + D: std::fmt::Display, +{ + match res { + Ok(res) => Ok(res), + Err(err) => Err(Error::from_err(prefix_fn.map(|prefix_fn| prefix_fn()), err)), + } +} + +impl Mappable for Result { + fn or_unexpected(self) -> Result { + let no_fn = None:: Box>>; // lol, good job rust + map_unexpected_maybe_while(self, no_fn) + } + + fn or_unexpected_while(self, prefix: D) -> Result { + map_unexpected_maybe_while(self, Some(|| prefix)) + } + + fn map_unexpected_while(self, f: F) -> Result + where + F: FnOnce() -> D, + D: fmt::Display, + { + map_unexpected_maybe_while(self, Some(f)) + } +} + +pub trait Intoable { + fn into_unexpected(self) -> Error; + + /// into_unexpected_while is like to_unexpected, but will prefix the Error error. The + /// prefix should be worded as if it started with the word "while", e.g.: `opening file + /// {path}`. + fn into_unexpected_while(self, prefix: D) -> Error + where + D: fmt::Display; +} + +impl Intoable for E { + fn into_unexpected(self) -> Error { + Error::from_err(None::, self) + } + + fn into_unexpected_while(self, prefix: D) -> Error + where + D: fmt::Display, + { + Error::from_err(Some(prefix), self) + } +} diff --git a/src/lib.rs b/src/lib.rs index 7cf0674..884cf00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,3 +4,4 @@ pub mod domain; pub mod error; pub mod origin; pub mod service; +pub mod util; diff --git a/src/main.rs b/src/main.rs index 82d616d..a216ef4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -98,8 +98,8 @@ async fn main() { let canceller = canceller.clone(); tokio::spawn(async move { - let mut signals = - Signals::new(signal_hook::consts::TERM_SIGNALS).expect("initialized signals"); + let mut signals = Signals::new(signal_hook::consts::TERM_SIGNALS) + .expect("initializing signals failed"); if (signals.next().await).is_some() { log::info!("Gracefully shutting down..."); @@ -114,23 +114,23 @@ async fn main() { } let origin_store = domiply::origin::store::git::new(config.origin_store_git_dir_path) - .expect("git origin store initialized"); + .expect("git origin store initialization failed"); let domain_checker = domiply::domain::checker::new( config.domain_checker_target_a, &config.domain_checker_resolver_addr, ) .await - .expect("domain checker initialized"); + .expect("domain checker initialization failed"); let domain_config_store = domiply::domain::config::new(&config.domain_config_store_dir_path) - .expect("domain config store initialized"); + .expect("domain config store initialization failed"); let https_params = if let Some(https_listen_addr) = config.https_listen_addr { let domain_acme_store_dir_path = config.domain_acme_store_dir_path.unwrap(); let domain_acme_store = domiply::domain::acme::store::new(&domain_acme_store_dir_path) - .expect("domain acme store initialized"); + .expect("domain acme store initialization failed"); // if https_listen_addr is set then domain_acme_contact_email is required, see the Cli/clap // settings. @@ -141,7 +141,7 @@ async fn main() { &domain_acme_contact_email, ) .await - .expect("domain acme manager initialized"); + .expect("domain acme manager initialization failed"); Some(HTTPSParams { https_listen_addr, @@ -182,10 +182,7 @@ async fn main() { errors_iter .unwrap() .into_iter() - .for_each(|(descr, err)| match descr { - None => log::error!("Error while syncing unknown descr: {err}"), - Some(descr) => log::error!("Failed to sync {descr:?}: {err}"), - }); + .for_each(|err| log::error!("syncing failed: {err}")); } }) }); @@ -323,7 +320,7 @@ async fn main() { let addr = https_params.https_listen_addr; let addr_incoming = hyper::server::conn::AddrIncoming::bind(&addr) - .expect("https listen socket created"); + .expect("https listen socket creation failed"); let incoming = tls_listener::TlsListener::new(server_config, addr_incoming).filter(|conn| { diff --git a/src/origin.rs b/src/origin.rs index 5a61708..b0b448e 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -1,4 +1,4 @@ -use crate::error; +use crate::error::unexpected; mod descr; pub use self::descr::Descr; @@ -10,7 +10,7 @@ pub enum ReadFileIntoError { FileNotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[mockall::automock] diff --git a/src/origin/store.rs b/src/origin/store.rs index 8a5c3bb..66e8f7e 100644 --- a/src/origin/store.rs +++ b/src/origin/store.rs @@ -1,4 +1,5 @@ -use crate::{error, origin}; +use crate::error::unexpected; +use crate::origin; pub mod git; @@ -19,7 +20,7 @@ pub enum SyncError { AlreadyInProgress, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] @@ -28,13 +29,13 @@ pub enum GetError { NotFound, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } #[derive(thiserror::Error, Debug)] pub enum AllDescrsError { #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } /// Used in the return from all_descrs from Store. diff --git a/src/origin/store/git.rs b/src/origin/store/git.rs index 342d737..52367c1 100644 --- a/src/origin/store/git.rs +++ b/src/origin/store/git.rs @@ -1,6 +1,5 @@ -use crate::error::{MapUnexpected, ToUnexpected}; -use crate::origin::store; -use crate::{error, origin}; +use crate::error::unexpected::{self, Intoable, Mappable}; +use crate::origin::{self, store}; use std::path::{Path, PathBuf}; use std::{collections, fs, io, sync}; @@ -28,16 +27,23 @@ impl origin::Origin for sync::Arc { let file_object = repo .find_object(self.tree_object_id) - .map_unexpected()? + .map_unexpected_while(|| format!("finding tree object {}", self.tree_object_id))? .peel_to_tree() - .map_unexpected()? + .map_unexpected_while(|| format!("peeling tree object {}", self.tree_object_id))? .lookup_entry_by_path(clean_path) - .map_unexpected()? + .map_unexpected_while(|| { + format!( + "looking up {} in tree object {}", + clean_path.display(), + self.tree_object_id + ) + })? .ok_or(origin::ReadFileIntoError::FileNotFound)? .object() - .map_unexpected()?; + .or_unexpected()?; - into.write_all(file_object.data.as_ref()).map_unexpected()?; + into.write_all(file_object.data.as_ref()) + .or_unexpected_while("copying out file")?; Ok(()) } @@ -49,7 +55,7 @@ enum GetOriginError { InvalidBranchName, #[error(transparent)] - Unexpected(#[from] error::Unexpected), + Unexpected(#[from] unexpected::Error), } /// git::Store implements the Store trait for any Descr::Git based Origins. If any non-git @@ -95,19 +101,21 @@ impl Store { ref branch_name, .. } = descr; + let branch_ref = self.branch_ref(branch_name); + let commit_object_id = repo - .try_find_reference(&self.branch_ref(branch_name)) - .map_unexpected()? + .try_find_reference(&branch_ref) + .map_unexpected_while(|| format!("finding branch ref {branch_ref}"))? .ok_or(GetOriginError::InvalidBranchName)? .peel_to_id_in_place() - .map_unexpected()? + .or_unexpected_while("peeling id in place")? .detach(); let tree_object_id = repo .find_object(commit_object_id) - .map_unexpected()? + .map_unexpected_while(|| format!("finding commit object {commit_object_id}"))? .try_to_commit_ref() - .map_unexpected()? + .map_unexpected_while(|| format!("parsing {commit_object_id} as commit"))? .tree(); Ok(sync::Arc::from(Origin { @@ -131,7 +139,8 @@ impl Store { // if the path doesn't exist then use the gix clone feature to clone it into the // directory. if fs::read_dir(repo_path).is_err() { - fs::create_dir_all(repo_path).map_unexpected()?; + fs::create_dir_all(repo_path) + .map_unexpected_while(|| format!("creating {}", repo_path.display()))?; let origin::Descr::Git { ref url, @@ -146,43 +155,53 @@ impl Store { gixCloneErr::UrlParse(_) | gixCloneErr::CanonicalizeUrl { .. } => { store::SyncError::InvalidURL } - _ => e.to_unexpected().into(), + _ => e + .into_unexpected_while(format!( + "cloning {} into {}", + url, + repo_path.display() + )) + .into(), })? .fetch_only(Discard, should_interrupt) .map_err(|_| store::SyncError::InvalidURL)?; // Check to make sure the branch name exists // TODO if this fails we should delete repo_path - repo.try_find_reference(&self.branch_ref(branch_name)) - .map_unexpected()? + let branch_ref = self.branch_ref(branch_name); + repo.try_find_reference(&branch_ref) + .map_unexpected_while(|| format!("finding branch ref {branch_ref}"))? .ok_or(store::SyncError::InvalidBranchName)?; // Add the descr to the repo directory, so we can know the actual descr later // TODO if this fails we should delete repo_path - let descr_file = - fs::File::create(self.descr_file_path(descr.id().as_ref())).map_unexpected()?; + let file_path = self.descr_file_path(descr.id().as_ref()); + let descr_file = fs::File::create(&file_path) + .map_unexpected_while(|| format!("creating {}", file_path.display()))?; - serde_json::to_writer(descr_file, &descr).map_unexpected()?; + serde_json::to_writer(descr_file, &descr) + .map_unexpected_while(|| format!("writing descr to {}", file_path.display()))?; return Ok(repo); } let direction = gix::remote::Direction::Fetch; - let repo = gix::open(repo_path).map_unexpected()?; + let repo = gix::open(repo_path) + .map_unexpected_while(|| format!("opening repo at {}", repo_path.display()))?; let remote = repo .find_default_remote(direction) - .ok_or_else(|| error::Unexpected::from("no default configured"))? - .map_unexpected()?; + .ok_or_else(|| unexpected::Error::from("no default configured"))? + .or_unexpected_while("finding default remote for fetching")?; remote .connect(direction) - .map_unexpected()? + .or_unexpected_while("connecting to remote")? .prepare_fetch(Discard, Default::default()) - .map_unexpected()? + .or_unexpected_while("preparing fetch")? .receive(Discard, should_interrupt) - .map_unexpected()?; + .or_unexpected_while("fetching from remote")?; Ok(repo) } @@ -253,15 +272,18 @@ impl super::Store for sync::Arc { fs::read_dir(&repo_path).map_err(|e| match e.kind() { io::ErrorKind::NotFound => store::GetError::NotFound, - _ => e.to_unexpected().into(), + _ => e + .into_unexpected_while(format!("checking if {} exists", repo_path.display())) + .into(), })?; - let repo = gix::open(&repo_path).map_unexpected()?; + let repo = gix::open(&repo_path) + .map_unexpected_while(|| format!("opening {} as git repo", repo_path.display()))?; let origin = self.get_origin(repo, descr.clone()).map_err(|e| match e { // it's not expected that the branch name is invalid at this point, it must have // existed for sync to have been successful. - GetOriginError::InvalidBranchName => e.to_unexpected().into(), + GetOriginError::InvalidBranchName => e.into_unexpected().into(), GetOriginError::Unexpected(e) => store::GetError::Unexpected(e), })?; @@ -273,14 +295,14 @@ impl super::Store for sync::Arc { fn all_descrs(&self) -> store::AllDescrsResult> { Ok(Box::from( - fs::read_dir(&self.dir_path).map_unexpected()?.map( + fs::read_dir(&self.dir_path).or_unexpected()?.map( |dir_entry_res: io::Result| -> store::AllDescrsResult { let descr_id: String = dir_entry_res - .map_unexpected()? + .or_unexpected()? .file_name() .to_str() .ok_or_else(|| { - error::Unexpected::from("couldn't convert os string to &str") + unexpected::Error::from("couldn't convert os string to &str") })? .into(); @@ -289,9 +311,14 @@ impl super::Store for sync::Arc { // TODO it's possible that opening the file will fail if syncing is // still ongoing, as writing the descr file is the last step after // initial sync has succeeded. - let descr_file = fs::File::open(descr_file_path).map_unexpected()?; + let descr_file = fs::File::open(descr_file_path.as_path()) + .map_unexpected_while(|| { + format!("opening descr file {}", descr_file_path.display()) + })?; - let descr = serde_json::from_reader(descr_file).map_unexpected()?; + let descr = serde_json::from_reader(descr_file).map_unexpected_while(|| { + format!("reading descr file {}", descr_file_path.display()) + })?; Ok(descr) }, diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 0000000..d0eef25 --- /dev/null +++ b/src/util.rs @@ -0,0 +1,11 @@ +use std::{fs, io, path}; + +pub fn open_file(path: &path::Path) -> io::Result> { + match fs::File::open(path) { + Ok(file) => Ok(Some(file)), + Err(err) => match err.kind() { + io::ErrorKind::NotFound => Ok(None), + _ => Err(err), + }, + } +}