[PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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 <a.bied-charreton@proxmox.com>
---
 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