feat(logging): Emit a setup error when a restricted keys are used for logging default keys (#5185)

Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>

Cargo.lock

@@ -2002,6 +2002,7 @@ dependencies = [
  "serde",
  "serde_json",
  "strum 0.26.2",
+ "thiserror",
  "utoipa",
 ]
 
@@ -6219,6 +6220,7 @@ name = "router_env"
 version = "0.1.0"
 dependencies = [
  "cargo_metadata 0.18.1",
+ "common_enums",
  "config",
  "error-stack",
  "gethostname",
@@ -7238,6 +7240,7 @@ dependencies = [
  "async-trait",
  "bb8",
  "bytes 1.6.0",
+ "common_enums",
  "common_utils",
  "config",
  "crc32fast",
@@ -8439,9 +8442,9 @@ checksum = "110352d4e9076c67839003c7788d8604e24dcded13e0b375af3efaa8cf468517"
 
 [[package]]
 name = "utf8parse"
-version = "0.2.1"
+version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a"
+checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821"
 
 [[package]]
 name = "utoipa"

crates/common_enums/Cargo.toml

@@ -13,6 +13,7 @@ openapi = []
 payouts = []
 
 [dependencies]
+thiserror = "1.0.58"
 diesel = { version = "2.1.5", features = ["postgres"] }
 serde = { version = "1.0.197", features = ["derive"] }
 serde_json = "1.0.115"

crates/common_enums/src/enums.rs

@@ -20,6 +20,80 @@ pub mod diesel_exports {
     };
 }
 
+pub type ApplicationResult<T> = Result<T, ApplicationError>;
+
+#[derive(Debug, thiserror::Error)]
+pub enum ApplicationError {
+    #[error("Application configuration error")]
+    ConfigurationError,
+
+    #[error("Invalid configuration value provided: {0}")]
+    InvalidConfigurationValueError(String),
+
+    #[error("Metrics error")]
+    MetricsError,
+
+    #[error("I/O: {0}")]
+    IoError(std::io::Error),
+
+    #[error("Error while constructing api client: {0}")]
+    ApiClientError(ApiClientError),
+}
+
+#[derive(Debug, thiserror::Error, PartialEq, Clone)]
+pub enum ApiClientError {
+    #[error("Header map construction failed")]
+    HeaderMapConstructionFailed,
+    #[error("Invalid proxy configuration")]
+    InvalidProxyConfiguration,
+    #[error("Client construction failed")]
+    ClientConstructionFailed,
+    #[error("Certificate decode failed")]
+    CertificateDecodeFailed,
+    #[error("Request body serialization failed")]
+    BodySerializationFailed,
+    #[error("Unexpected state reached/Invariants conflicted")]
+    UnexpectedState,
+
+    #[error("URL encoding of request payload failed")]
+    UrlEncodingFailed,
+    #[error("Failed to send request to connector {0}")]
+    RequestNotSent(String),
+    #[error("Failed to decode response")]
+    ResponseDecodingFailed,
+
+    #[error("Server responded with Request Timeout")]
+    RequestTimeoutReceived,
+
+    #[error("connection closed before a message could complete")]
+    ConnectionClosedIncompleteMessage,
+
+    #[error("Server responded with Internal Server Error")]
+    InternalServerErrorReceived,
+    #[error("Server responded with Bad Gateway")]
+    BadGatewayReceived,
+    #[error("Server responded with Service Unavailable")]
+    ServiceUnavailableReceived,
+    #[error("Server responded with Gateway Timeout")]
+    GatewayTimeoutReceived,
+    #[error("Server responded with unexpected response")]
+    UnexpectedServerResponse,
+}
+impl ApiClientError {
+    pub fn is_upstream_timeout(&self) -> bool {
+        self == &Self::RequestTimeoutReceived
+    }
+    pub fn is_connection_closed_before_message_could_complete(&self) -> bool {
+        self == &Self::ConnectionClosedIncompleteMessage
+    }
+}
+
+impl From<std::io::Error> for ApplicationError {
+    fn from(err: std::io::Error) -> Self {
+        Self::IoError(err)
+    }
+}
+
 /// The status of the attempt
 #[derive(
     Clone,

crates/hyperswitch_interfaces/src/configs.rs

@@ -1,8 +1,8 @@
 //! Configs interface
+use common_enums::ApplicationError;
 use masking::Secret;
 use router_derive;
 use serde::Deserialize;
-use storage_impl::errors::ApplicationError;
 
 // struct Connectors
 #[allow(missing_docs, missing_debug_implementations)]

crates/router/src/bin/router.rs

@@ -1,3 +1,4 @@
+use error_stack::ResultExt;
 use router::{
     configs::settings::{CmdLineConf, Settings},
     core::errors::{ApplicationError, ApplicationResult},
@@ -24,7 +25,8 @@ async fn main() -> ApplicationResult<()> {
         &conf.log,
         router_env::service_name!(),
         [router_env::service_name!(), "actix_server"],
-    );
+    )
+    .change_context(ApplicationError::ConfigurationError)?;
 
     logger::info!("Application started [{:?}] [{:?}]", conf.server, conf.log);
 
@@ -39,8 +41,7 @@ async fn main() -> ApplicationResult<()> {
         .expect("Failed to create the server");
     let _ = server.await;
 
-    Err(ApplicationError::from(std::io::Error::new(
-        std::io::ErrorKind::Other,
-        "Server shut down",
+    Err(error_stack::Report::from(ApplicationError::from(
+        std::io::Error::new(std::io::ErrorKind::Other, "Server shut down"),
     )))
 }

crates/router/src/bin/scheduler.rs

@@ -116,7 +116,8 @@ pub async fn start_web_server(
     let web_server = actix_web::HttpServer::new(move || {
         actix_web::App::new().service(Health::server(state.clone(), service.clone()))
     })
-    .bind((server.host.as_str(), server.port))?
+    .bind((server.host.as_str(), server.port))
+    .change_context(ApplicationError::ConfigurationError)?
     .workers(server.workers)
     .run();
     let _ = web_server.handle();

crates/router/src/configs/settings.rs

@@ -8,6 +8,7 @@ use analytics::{opensearch::OpenSearchConfig, ReportConfig};
 use api_models::{enums, payment_methods::RequiredFieldInfo};
 use common_utils::ext_traits::ConfigExt;
 use config::{Environment, File};
+use error_stack::ResultExt;
 #[cfg(feature = "email")]
 use external_services::email::EmailSettings;
 use external_services::{
@@ -33,7 +34,7 @@ use storage_impl::config::QueueStrategy;
 use crate::analytics::AnalyticsConfig;
 use crate::{
     core::errors::{ApplicationError, ApplicationResult},
-    env::{self, logger, Env},
+    env::{self, Env},
     events::EventsConfig,
 };
 
@@ -722,7 +723,8 @@ impl Settings<SecuredSecret> {
         let environment = env::which();
         let config_path = router_env::Config::config_path(&environment.to_string(), config_path);
 
-        let config = router_env::Config::builder(&environment.to_string())?
+        let config = router_env::Config::builder(&environment.to_string())
+            .change_context(ApplicationError::ConfigurationError)?
             .add_source(File::from(config_path).required(false))
             .add_source(
                 Environment::with_prefix("ROUTER")
@@ -736,13 +738,12 @@ impl Settings<SecuredSecret> {
                     .with_list_parse_key("connector_request_reference_id_config.merchant_ids_send_payment_id_as_connector_request_id"),
 
             )
-            .build()?;
+            .build()
+            .change_context(ApplicationError::ConfigurationError)?;
 
-        serde_path_to_error::deserialize(config).map_err(|error| {
-            logger::error!(%error, "Unable to deserialize application configuration");
-            eprintln!("Unable to deserialize application configuration: {error}");
-            ApplicationError::from(error.into_inner())
-        })
+        serde_path_to_error::deserialize(config)
+            .attach_printable("Unable to deserialize application configuration")
+            .change_context(ApplicationError::ConfigurationError)
     }
 
     pub fn validate(&self) -> ApplicationResult<()> {
@@ -756,14 +757,18 @@ impl Settings<SecuredSecret> {
         })?;
         if self.log.file.enabled {
             if self.log.file.file_name.is_default_or_empty() {
-                return Err(ApplicationError::InvalidConfigurationValueError(
+                return Err(error_stack::Report::from(
+                    ApplicationError::InvalidConfigurationValueError(
                         "log file name must not be empty".into(),
+                    ),
                 ));
             }
 
             if self.log.file.path.is_default_or_empty() {
-                return Err(ApplicationError::InvalidConfigurationValueError(
+                return Err(error_stack::Report::from(
+                    ApplicationError::InvalidConfigurationValueError(
                         "log directory path must not be empty".into(),
+                    ),
                 ));
             }
         }

crates/router/src/core/errors.rs

@@ -32,7 +32,7 @@ use crate::services;
 pub type RouterResult<T> = CustomResult<T, ApiErrorResponse>;
 pub type RouterResponse<T> = CustomResult<services::ApplicationResponse<T>, ApiErrorResponse>;
 
-pub type ApplicationResult<T> = Result<T, ApplicationError>;
+pub type ApplicationResult<T> = error_stack::Result<T, ApplicationError>;
 pub type ApplicationResponse<T> = ApplicationResult<services::ApplicationResponse<T>>;
 
 pub type CustomerResponse<T> =

crates/router_env/Cargo.toml

@@ -14,6 +14,7 @@ error-stack = "0.4.1"
 gethostname = "0.4.3"
 once_cell = "1.19.0"
 opentelemetry = { version = "0.19.0", features = ["rt-tokio-current-thread", "metrics"] }
+common_enums = { path = "../common_enums" }
 opentelemetry-otlp = { version = "0.12.0", features = ["metrics"] }
 rustc-hash = "1.1"
 serde = { version = "1.0.197", features = ["derive"] }

crates/router_env/src/logger/formatter.rs

@@ -8,6 +8,7 @@ use std::{
     io::Write,
 };
 
+use config::ConfigError;
 use once_cell::sync::Lazy;
 use serde::ser::{SerializeMap, Serializer};
 use serde_json::{ser::Formatter, Value};
@@ -155,7 +156,11 @@ where
     /// let formatting_layer = router_env::FormattingLayer::new("my_service", std::io::stdout, serde_json::ser::CompactFormatter);
     /// ```
     ///
-    pub fn new(service: &str, dst_writer: W, formatter: F) -> Self {
+    pub fn new(
+        service: &str,
+        dst_writer: W,
+        formatter: F,
+    ) -> error_stack::Result<Self, ConfigError> {
         Self::new_with_implicit_entries(service, dst_writer, HashMap::new(), formatter)
     }
 
@@ -163,9 +168,9 @@ where
     pub fn new_with_implicit_entries(
         service: &str,
         dst_writer: W,
-        mut default_fields: HashMap<String, Value>,
+        default_fields: HashMap<String, Value>,
         formatter: F,
-    ) -> Self {
+    ) -> error_stack::Result<Self, ConfigError> {
         let pid = std::process::id();
         let hostname = gethostname::gethostname().to_string_lossy().into_owned();
         let service = service.to_string();
@@ -174,20 +179,16 @@ where
         #[cfg(feature = "vergen")]
         let build = crate::build!().to_string();
         let env = crate::env::which().to_string();
-        default_fields.retain(|key, value| {
-            if !IMPLICIT_KEYS.contains(key.as_str()) {
-                true
-            } else {
-                tracing::warn!(
-                    ?key,
-                    ?value,
-                    "Attempting to log a reserved entry. It won't be added to the logs"
-                );
-                false
+        for key in default_fields.keys() {
+            if IMPLICIT_KEYS.contains(key.as_str()) {
+                return Err(ConfigError::Message(format!(
+                    "A reserved key `{key}` was included in `default_fields` in the log formatting layer"
+                ))
+                .into());
+            }
         }
-        });
 
-        Self {
+        Ok(Self {
             dst_writer,
             pid,
             hostname,
@@ -199,7 +200,7 @@ where
             build,
             default_fields,
             formatter,
-        }
+        })
     }
 
     /// Serialize common for both span and event entries.

crates/router_env/src/logger/setup.rs

@@ -2,6 +2,7 @@
 
 use std::time::Duration;
 
+use ::config::ConfigError;
 use opentelemetry::{
     global, runtime,
     sdk::{
@@ -36,7 +37,7 @@ pub fn setup(
     config: &config::Log,
     service_name: &str,
     crates_to_filter: impl AsRef<[&'static str]>,
-) -> TelemetryGuard {
+) -> error_stack::Result<TelemetryGuard, ConfigError> {
     let mut guards = Vec::new();
 
     // Setup OpenTelemetry traces and metrics
@@ -69,11 +70,9 @@ pub fn setup(
             &crates_to_filter,
         );
         println!("Using file logging filter: {file_filter}");
-
-        Some(
-            FormattingLayer::new(service_name, file_writer, CompactFormatter)
-                .with_filter(file_filter),
-        )
+        let layer = FormattingLayer::new(service_name, file_writer, CompactFormatter)?
+            .with_filter(file_filter);
+        Some(layer)
     } else {
         None
     };
@@ -107,17 +106,21 @@ pub fn setup(
             }
             config::LogFormat::Json => {
                 error_stack::Report::set_color_mode(error_stack::fmt::ColorMode::None);
-                let logging_layer =
-                    FormattingLayer::new(service_name, console_writer, CompactFormatter)
-                        .with_filter(console_filter);
-                subscriber.with(logging_layer).init();
+                subscriber
+                    .with(
+                        FormattingLayer::new(service_name, console_writer, CompactFormatter)?
+                            .with_filter(console_filter),
+                    )
+                    .init();
             }
             config::LogFormat::PrettyJson => {
                 error_stack::Report::set_color_mode(error_stack::fmt::ColorMode::None);
-                let logging_layer =
-                    FormattingLayer::new(service_name, console_writer, PrettyFormatter::new())
-                        .with_filter(console_filter);
-                subscriber.with(logging_layer).init();
+                subscriber
+                    .with(
+                        FormattingLayer::new(service_name, console_writer, PrettyFormatter::new())?
+                            .with_filter(console_filter),
+                    )
+                    .init();
             }
         }
     } else {
@@ -126,10 +129,10 @@ pub fn setup(
 
     // Returning the TelemetryGuard for logs to be printed and metrics to be collected until it is
     // dropped
-    TelemetryGuard {
+    Ok(TelemetryGuard {
         _log_guards: guards,
         _metrics_controller,
-    }
+    })
 }
 
 fn get_opentelemetry_exporter(config: &config::LogTelemetry) -> TonicExporterBuilder {

crates/router_env/tests/logger.rs

@@ -1,25 +1,25 @@
 #![allow(clippy::unwrap_used)]
 
 mod test_module;
-
+use ::config::ConfigError;
 use router_env::TelemetryGuard;
 
 use self::test_module::some_module::*;
 
-fn logger() -> &'static TelemetryGuard {
+fn logger() -> error_stack::Result<&'static TelemetryGuard, ConfigError> {
     use once_cell::sync::OnceCell;
 
     static INSTANCE: OnceCell<TelemetryGuard> = OnceCell::new();
-    INSTANCE.get_or_init(|| {
+    Ok(INSTANCE.get_or_init(|| {
         let config = router_env::Config::new().unwrap();
 
-        router_env::setup(&config.log, "router_env_test", [])
-    })
+        router_env::setup(&config.log, "router_env_test", []).unwrap()
+    }))
 }
 
 #[tokio::test]
 async fn basic() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-    logger();
+    logger()?;
 
     fn_with_colon(13).await;
 

crates/storage_impl/Cargo.toml

@@ -18,6 +18,7 @@ payouts = ["hyperswitch_domain_models/payouts"]
 # First Party dependencies
 api_models = { version = "0.1.0", path = "../api_models" }
 common_utils = { version = "0.1.0", path = "../common_utils" }
+common_enums = { version = "0.1.0", path = "../common_enums" }
 hyperswitch_domain_models = { version = "0.1.0", path = "../hyperswitch_domain_models", default-features = false }
 diesel_models = { version = "0.1.0", path = "../diesel_models" }
 masking = { version = "0.1.0", path = "../masking" }

crates/storage_impl/src/errors.rs

@@ -1,17 +1,10 @@
-use std::fmt::Display;
-
-use actix_web::ResponseError;
+pub use common_enums::{ApiClientError, ApplicationError, ApplicationResult};
 use common_utils::errors::ErrorSwitch;
-use config::ConfigError;
-use http::StatusCode;
 use hyperswitch_domain_models::errors::StorageError as DataStorageError;
 pub use redis_interface::errors::RedisError;
-use router_env::opentelemetry::metrics::MetricsError;
 
 use crate::store::errors::DatabaseError;
 
-pub type ApplicationResult<T> = Result<T, ApplicationError>;
-
 #[derive(Debug, thiserror::Error)]
 pub enum StorageError {
     #[error("DatabaseError: {0:?}")]
@@ -154,115 +147,6 @@ impl RedisErrorExt for error_stack::Report<RedisError> {
     }
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum ApplicationError {
-    // Display's impl can be overridden by the attribute error marco.
-    // Don't use Debug here, Debug gives error stack in response.
-    #[error("Application configuration error: {0}")]
-    ConfigurationError(ConfigError),
-
-    #[error("Invalid configuration value provided: {0}")]
-    InvalidConfigurationValueError(String),
-
-    #[error("Metrics error: {0}")]
-    MetricsError(MetricsError),
-
-    #[error("I/O: {0}")]
-    IoError(std::io::Error),
-
-    #[error("Error while constructing api client: {0}")]
-    ApiClientError(ApiClientError),
-}
-
-impl From<MetricsError> for ApplicationError {
-    fn from(err: MetricsError) -> Self {
-        Self::MetricsError(err)
-    }
-}
-
-impl From<std::io::Error> for ApplicationError {
-    fn from(err: std::io::Error) -> Self {
-        Self::IoError(err)
-    }
-}
-
-impl From<ConfigError> for ApplicationError {
-    fn from(err: ConfigError) -> Self {
-        Self::ConfigurationError(err)
-    }
-}
-
-fn error_response<T: Display>(err: &T) -> actix_web::HttpResponse {
-    actix_web::HttpResponse::BadRequest()
-        .content_type(mime::APPLICATION_JSON)
-        .body(format!(r#"{{ "error": {{ "message": "{err}" }} }}"#))
-}
-
-impl ResponseError for ApplicationError {
-    fn status_code(&self) -> StatusCode {
-        match self {
-            Self::MetricsError(_)
-            | Self::IoError(_)
-            | Self::ConfigurationError(_)
-            | Self::InvalidConfigurationValueError(_)
-            | Self::ApiClientError(_) => StatusCode::INTERNAL_SERVER_ERROR,
-        }
-    }
-
-    fn error_response(&self) -> actix_web::HttpResponse {
-        error_response(self)
-    }
-}
-
-#[derive(Debug, thiserror::Error, PartialEq, Clone)]
-pub enum ApiClientError {
-    #[error("Header map construction failed")]
-    HeaderMapConstructionFailed,
-    #[error("Invalid proxy configuration")]
-    InvalidProxyConfiguration,
-    #[error("Client construction failed")]
-    ClientConstructionFailed,
-    #[error("Certificate decode failed")]
-    CertificateDecodeFailed,
-    #[error("Request body serialization failed")]
-    BodySerializationFailed,
-    #[error("Unexpected state reached/Invariants conflicted")]
-    UnexpectedState,
-
-    #[error("URL encoding of request payload failed")]
-    UrlEncodingFailed,
-    #[error("Failed to send request to connector {0}")]
-    RequestNotSent(String),
-    #[error("Failed to decode response")]
-    ResponseDecodingFailed,
-
-    #[error("Server responded with Request Timeout")]
-    RequestTimeoutReceived,
-
-    #[error("connection closed before a message could complete")]
-    ConnectionClosedIncompleteMessage,
-
-    #[error("Server responded with Internal Server Error")]
-    InternalServerErrorReceived,
-    #[error("Server responded with Bad Gateway")]
-    BadGatewayReceived,
-    #[error("Server responded with Service Unavailable")]
-    ServiceUnavailableReceived,
-    #[error("Server responded with Gateway Timeout")]
-    GatewayTimeoutReceived,
-    #[error("Server responded with unexpected response")]
-    UnexpectedServerResponse,
-}
-
-impl ApiClientError {
-    pub fn is_upstream_timeout(&self) -> bool {
-        self == &Self::RequestTimeoutReceived
-    }
-    pub fn is_connection_closed_before_message_could_complete(&self) -> bool {
-        self == &Self::ConnectionClosedIncompleteMessage
-    }
-}
-
 #[derive(Debug, thiserror::Error, PartialEq)]
 pub enum ConnectorError {
     #[error("Error while obtaining URL for the integration")]