From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 209991FF141 for ; Fri, 13 Feb 2026 17:04:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BE7D098D8; Fri, 13 Feb 2026 17:04:34 +0100 (CET) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types Date: Fri, 13 Feb 2026 17:04:05 +0100 Message-ID: <20260213160415.609868-8-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260213160415.609868-1-a.bied-charreton@proxmox.com> References: <20260213160415.609868-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.084 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: KWE3O5FXY4HHJU7PABW37BV2KYWUDYDF X-Message-ID-Hash: KWE3O5FXY4HHJU7PABW37BV2KYWUDYDF X-MailFrom: abied-charreton@jett.proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 { - 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), + /// The state for an endpoint could not be persisted + StatePersistence(String, Box), + /// The state for an endpoint could not be retrieved + StateRetrieval(String, Box), /// 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