Log load/store events in SMTP state management, and add error types to make errors clearer (previously used Config(De)?Serialization errors, which led to confusing error messages for state-related errors)
Also add logs for OAuth2 token exchange events. Signed-off-by: Arthur Bied-Charreton <[email protected]> --- proxmox-notify/src/endpoints/smtp.rs | 12 +++++++++++ proxmox-notify/src/endpoints/smtp/state.rs | 25 +++++++++++++++------- proxmox-notify/src/lib.rs | 12 +++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs index 4364bd11..0ae1ac9f 100644 --- a/proxmox-notify/src/endpoints/smtp.rs +++ b/proxmox-notify/src/endpoints/smtp.rs @@ -12,6 +12,7 @@ use oauth2::{ClientId, ClientSecret, RefreshToken}; use proxmox_schema::api_types::COMMENT_SCHEMA; use proxmox_schema::{api, Updater}; +use tracing::info; use crate::context::context; use crate::endpoints::common::mail; @@ -301,6 +302,12 @@ impl SmtpEndpoint { self.name().into(), Box::new(Error::Generic("no refresh token found".into())), ))?; + + info!( + "requesting OAuth2 access token for endpoint '{}'", + self.config.name + ); + let token_exchange_result = self.get_access_token(&refresh_token, method)?; state @@ -313,6 +320,11 @@ impl SmtpEndpoint { .set_last_refreshed(proxmox_time::epoch_i64()) .store(self.name())?; + info!( + "OAuth2 token exchange successful for endpoint '{}'", + self.config.name + ); + transport_builder .credentials(Credentials::new( self.config.from_address.to_owned(), diff --git a/proxmox-notify/src/endpoints/smtp/state.rs b/proxmox-notify/src/endpoints/smtp/state.rs index 60bef590..45d0db93 100644 --- a/proxmox-notify/src/endpoints/smtp/state.rs +++ b/proxmox-notify/src/endpoints/smtp/state.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use tracing::debug; use crate::{context::context, Error}; @@ -25,13 +26,19 @@ impl State { /// # Errors /// An [`Error`] is returned if deserialization of the state object fails. pub(crate) fn load(name: &str) -> Result<State, Error> { - match proxmox_sys::fs::file_get_optional_contents(context().state_file_path(name)) - .map_err(|e| Error::ConfigDeserialization(e.into()))? + let path = context().state_file_path(name); + + match proxmox_sys::fs::file_get_optional_contents(&path) + .map_err(|e| Error::StateRetrieval(path.to_owned(), e.into()))? { Some(bytes) => { - serde_json::from_slice(&bytes).map_err(|e| Error::ConfigDeserialization(e.into())) + debug!("loaded state file for endpoint '{name}' from {path}"); + serde_json::from_slice(&bytes).map_err(|e| Error::StateRetrieval(path, e.into())) + } + None => { + debug!("no existing state file found for endpoint '{name}' at {path}, creating empty state"); + Ok(State::default()) } - None => Ok(State::default()), } } @@ -43,14 +50,16 @@ impl State { let path = context().state_file_path(name); let parent = std::path::Path::new(&path).parent().unwrap(); + debug!("attempting to persist state for endpoint '{name}' at {path}"); + proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false) - .map_err(|e| Error::ConfigSerialization(e.into()))?; + .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?; let s = serde_json::to_string_pretty(&self) - .map_err(|e| Error::ConfigSerialization(e.into()))?; + .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?; - proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true) - .map_err(|e| Error::ConfigSerialization(e.into())) + proxmox_sys::fs::replace_file(&path, s.as_bytes(), context().secret_create_options(), true) + .map_err(|e| Error::StatePersistence(path, e.into())) } /// Set `last_refreshed`. diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 996393c2..37237fc0 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -41,6 +41,10 @@ pub enum Error { FilterFailed(String), /// The notification's template string could not be rendered RenderError(Box<dyn StdError + Send + Sync>), + /// The state for an endpoint could not be persisted + StatePersistence(String, Box<dyn StdError + Send + Sync>), + /// The state for an endpoint could not be retrieved + StateRetrieval(String, Box<dyn StdError + Send + Sync>), /// Generic error for anything else Generic(String), } @@ -70,6 +74,12 @@ impl Display for Error { Error::FilterFailed(message) => { write!(f, "could not apply filter: {message}") } + Error::StatePersistence(path, err) => { + write!(f, "could not persist state at {path}: {err}") + } + Error::StateRetrieval(path, err) => { + write!(f, "could not retrieve state from {path}: {err}") + } Error::RenderError(err) => write!(f, "could not render notification template: {err}"), Error::Generic(message) => f.write_str(message), } @@ -86,6 +96,8 @@ impl StdError for Error { Error::TargetTestFailed(errs) => Some(&*errs[0]), Error::FilterFailed(_) => None, Error::RenderError(err) => Some(&**err), + Error::StatePersistence(_, err) => Some(&**err), + Error::StateRetrieval(_, err) => Some(&**err), Error::Generic(_) => None, } } -- 2.47.3
