diff --git a/data-pipeline-ffi/src/error.rs b/data-pipeline-ffi/src/error.rs index c008de4df..17d5e76e2 100644 --- a/data-pipeline-ffi/src/error.rs +++ b/data-pipeline-ffi/src/error.rs @@ -1,12 +1,20 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use data_pipeline::trace_exporter::error::{ - AgentErrorKind, BuilderErrorKind, InternalErrorKind, NetworkErrorKind, TraceExporterError, -}; +use data_pipeline::trace_exporter::error::TraceExporterError; use std::ffi::{c_char, CString}; use std::fmt::Display; -use std::io::ErrorKind as IoErrorKind; + +/// Context field for structured error data. +/// Contains a key-value pair that can be safely used for logging or debugging. +#[repr(C)] +#[derive(Debug)] +pub struct ContextField { + /// Key name for this context field + pub key: *const c_char, + /// Value for this context field + pub value: *const c_char, +} /// Represent error codes that `Error` struct can hold #[repr(C)] @@ -73,17 +81,78 @@ impl Display for ExporterErrorCode { /// Structure that contains error information that `TraceExporter` API can return. #[repr(C)] -#[derive(Debug, PartialEq)] +#[derive(Debug)] pub struct ExporterError { pub code: ExporterErrorCode, - pub msg: *mut c_char, + /// Static error message template + pub msg_template: *const c_char, + /// Array of context fields + pub context_fields: *const ContextField, + /// Number of context fields + pub context_count: usize, } impl ExporterError { - pub fn new(code: ExporterErrorCode, msg: &str) -> Self { + /// Creates a new ExporterError with a static template and no context fields. + /// + /// # Arguments + /// + /// * `code` - The error code representing the type of error + /// * `template` - A static string template for the error message + /// + /// The returned error owns the template string and will free it when dropped. + /// The template string is converted to a null-terminated C string. + pub fn new(code: ExporterErrorCode, template: &'static str) -> Self { + // Convert to CString to ensure null termination + let template_cstring = CString::new(template).unwrap_or_default(); + let template_ptr = template_cstring.into_raw(); + + Self { + code, + msg_template: template_ptr, + context_fields: std::ptr::null(), + context_count: 0, + } + } + + /// Creates a new ExporterError with a static template and context fields. + /// + /// This method is designed for template-based error messaging where static error + /// templates are separated from dynamic context data. + /// + /// # Arguments + /// + /// * `code` - The error code representing the type of error + /// * `template` - A static string template for the error message + /// * `context_fields` - Vector of context fields containing structured error data + /// + /// The returned error owns all the strings and will free them when dropped. + /// Both the template and all context field keys/values are converted to + /// null-terminated C strings. The context fields array is heap-allocated + /// and will be properly freed. + pub fn with_template_and_context( + code: ExporterErrorCode, + template: &'static str, + context_fields: Vec, + ) -> Self { + let (fields_ptr, count) = if context_fields.is_empty() { + (std::ptr::null(), 0) + } else { + let boxed_fields = context_fields.into_boxed_slice(); + let len = boxed_fields.len(); + let ptr = Box::into_raw(boxed_fields) as *const ContextField; + (ptr, len) + }; + + // Convert to CString to ensure null termination + let template_cstring = CString::new(template).unwrap_or_default(); + let template_ptr = template_cstring.into_raw(); + Self { code, - msg: CString::new(msg).unwrap_or_default().into_raw(), + msg_template: template_ptr, + context_fields: fields_ptr, + context_count: count, } } } @@ -91,65 +160,118 @@ impl ExporterError { impl From for ExporterError { fn from(value: TraceExporterError) -> Self { let code = match &value { - TraceExporterError::Agent(e) => match e { - AgentErrorKind::EmptyResponse => ExporterErrorCode::HttpEmptyBody, - }, - TraceExporterError::Builder(e) => match e { - BuilderErrorKind::InvalidUri(_) => ExporterErrorCode::InvalidUrl, - BuilderErrorKind::InvalidTelemetryConfig(_) => ExporterErrorCode::InvalidArgument, - BuilderErrorKind::InvalidConfiguration(_) => ExporterErrorCode::InvalidArgument, - }, - TraceExporterError::Internal(e) => match e { - InternalErrorKind::InvalidWorkerState(_) => ExporterErrorCode::Internal, - }, - TraceExporterError::Deserialization(_) => ExporterErrorCode::Serde, - TraceExporterError::Io(e) => match e.kind() { - IoErrorKind::InvalidData => ExporterErrorCode::InvalidData, - IoErrorKind::InvalidInput => ExporterErrorCode::InvalidInput, - IoErrorKind::ConnectionReset => ExporterErrorCode::ConnectionReset, - IoErrorKind::ConnectionAborted => ExporterErrorCode::ConnectionAborted, - IoErrorKind::ConnectionRefused => ExporterErrorCode::ConnectionRefused, - IoErrorKind::TimedOut => ExporterErrorCode::TimedOut, - IoErrorKind::AddrInUse => ExporterErrorCode::AddressInUse, - _ => ExporterErrorCode::IoError, + TraceExporterError::Agent(_) => ExporterErrorCode::HttpEmptyBody, + TraceExporterError::Builder(builder_error) => match builder_error { + data_pipeline::trace_exporter::error::BuilderErrorKind::InvalidUri(_) => { + ExporterErrorCode::InvalidUrl + } + _ => ExporterErrorCode::InvalidArgument, }, - TraceExporterError::Network(e) => match e.kind() { - NetworkErrorKind::Body => ExporterErrorCode::HttpBodyFormat, - NetworkErrorKind::Canceled => ExporterErrorCode::ConnectionAborted, - NetworkErrorKind::ConnectionClosed => ExporterErrorCode::ConnectionReset, - NetworkErrorKind::MessageTooLarge => ExporterErrorCode::HttpBodyTooLong, - NetworkErrorKind::Parse => ExporterErrorCode::HttpParse, - NetworkErrorKind::TimedOut => ExporterErrorCode::TimedOut, - NetworkErrorKind::Unknown => ExporterErrorCode::NetworkUnknown, - NetworkErrorKind::WrongStatus => ExporterErrorCode::HttpWrongStatus, + TraceExporterError::Internal(_) => ExporterErrorCode::Internal, + TraceExporterError::Network(network_error) => match network_error.kind() { + data_pipeline::trace_exporter::error::NetworkErrorKind::Body => { + ExporterErrorCode::HttpBodyFormat + } + data_pipeline::trace_exporter::error::NetworkErrorKind::Parse => { + ExporterErrorCode::HttpParse + } + data_pipeline::trace_exporter::error::NetworkErrorKind::TimedOut => { + ExporterErrorCode::TimedOut + } + data_pipeline::trace_exporter::error::NetworkErrorKind::WrongStatus => { + ExporterErrorCode::HttpWrongStatus + } + data_pipeline::trace_exporter::error::NetworkErrorKind::ConnectionClosed => { + ExporterErrorCode::ConnectionReset + } + data_pipeline::trace_exporter::error::NetworkErrorKind::MessageTooLarge => { + ExporterErrorCode::HttpBodyTooLong + } + data_pipeline::trace_exporter::error::NetworkErrorKind::Canceled => { + ExporterErrorCode::HttpClient + } + data_pipeline::trace_exporter::error::NetworkErrorKind::Unknown => { + ExporterErrorCode::NetworkUnknown + } }, - TraceExporterError::Request(e) => { - let status: u16 = e.status().into(); - if (400..499).contains(&status) { + TraceExporterError::Request(request_error) => { + if request_error.status().is_client_error() { ExporterErrorCode::HttpClient - } else if status >= 500 { + } else if request_error.status().is_server_error() { ExporterErrorCode::HttpServer } else { ExporterErrorCode::HttpUnknown } } TraceExporterError::Shutdown(_) => ExporterErrorCode::Shutdown, + TraceExporterError::Deserialization(_) => ExporterErrorCode::InvalidData, + TraceExporterError::Io(io_error) => match io_error.kind() { + std::io::ErrorKind::ConnectionAborted => ExporterErrorCode::ConnectionAborted, + std::io::ErrorKind::ConnectionRefused => ExporterErrorCode::ConnectionRefused, + std::io::ErrorKind::ConnectionReset => ExporterErrorCode::ConnectionReset, + std::io::ErrorKind::TimedOut => ExporterErrorCode::TimedOut, + std::io::ErrorKind::AddrInUse => ExporterErrorCode::AddressInUse, + _ => ExporterErrorCode::IoError, + }, TraceExporterError::Telemetry(_) => ExporterErrorCode::Telemetry, TraceExporterError::Serialization(_) => ExporterErrorCode::Serde, }; - ExporterError::new(code, &value.to_string()) + + let template = value.template(); + let context = value.context(); + + let context_fields: Vec = context + .fields() + .iter() + .map(|(key, value)| { + let key_cstring = CString::new(key.as_str()).unwrap_or_default(); + let value_cstring = CString::new(value.as_str()).unwrap_or_default(); + + ContextField { + key: key_cstring.into_raw(), + value: value_cstring.into_raw(), + } + }) + .collect(); + + ExporterError::with_template_and_context(code, template, context_fields) } } impl Drop for ExporterError { fn drop(&mut self) { - if !self.msg.is_null() { - // SAFETY: `the caller must ensure that `ExporterError` has been created through its - // `new` method which ensures that `msg` property is originated from - // `Cstring::into_raw` call. Any other possibility could lead to UB. - unsafe { - drop(CString::from_raw(self.msg)); - self.msg = std::ptr::null_mut(); + unsafe { + // Free the msg_template + if !self.msg_template.is_null() { + // SAFETY: msg_template is originated from CString::into_raw in new() and + // with_template_and_context() methods + drop(CString::from_raw(self.msg_template as *mut c_char)); + self.msg_template = std::ptr::null(); + } + + // Free the context fields + if !self.context_fields.is_null() && self.context_count > 0 { + // SAFETY: `context_fields` and individual key/value pointers are originated from + // `CString::into_raw` calls in the `From` conversion and + // `with_template_and_context` method. The array is created via `Box::into_raw` + // from a boxed slice. Any other creation path could lead to UB. + for i in 0..self.context_count { + let field = &*self.context_fields.add(i); + if !field.key.is_null() { + drop(CString::from_raw(field.key as *mut c_char)); + } + if !field.value.is_null() { + drop(CString::from_raw(field.value as *mut c_char)); + } + } + + // Free the context fields array + drop(Box::from_raw(std::slice::from_raw_parts_mut( + self.context_fields as *mut ContextField, + self.context_count, + ))); + self.context_fields = std::ptr::null(); + self.context_count = 0; } } } @@ -172,22 +294,143 @@ mod tests { #[test] fn constructor_test() { let code = ExporterErrorCode::InvalidArgument; - let error = Box::new(ExporterError::new(code, &code.to_string())); + let template = "Invalid argument provided"; + let error = Box::new(ExporterError::new(code, template)); assert_eq!(error.code, ExporterErrorCode::InvalidArgument); - let msg = unsafe { CStr::from_ptr(error.msg).to_string_lossy() }; - assert_eq!(msg, ExporterErrorCode::InvalidArgument.to_string()); + assert!(!error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Invalid argument provided"); + assert!(error.context_fields.is_null()); + assert_eq!(error.context_count, 0); } #[test] fn destructor_test() { let code = ExporterErrorCode::InvalidArgument; - let error = Box::new(ExporterError::new(code, &code.to_string())); + let template = "Test template"; + let error = Box::new(ExporterError::new(code, template)); assert_eq!(error.code, ExporterErrorCode::InvalidArgument); - let msg = unsafe { CStr::from_ptr(error.msg).to_string_lossy() }; - assert_eq!(msg, ExporterErrorCode::InvalidArgument.to_string()); + assert!(!error.msg_template.is_null()); + + unsafe { ddog_trace_exporter_error_free(Some(error)) }; + } + + #[test] + fn template_and_context_test() { + let code = ExporterErrorCode::InvalidUrl; + let template = "Invalid URI provided"; + let context_fields = vec![ContextField { + key: CString::new("details").unwrap().into_raw(), + value: CString::new("invalid://url").unwrap().into_raw(), + }]; + + let error = Box::new(ExporterError::with_template_and_context( + code, + template, + context_fields, + )); + + assert_eq!(error.code, ExporterErrorCode::InvalidUrl); + assert!(!error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Invalid URI provided"); + assert!(!error.context_fields.is_null()); + assert_eq!(error.context_count, 1); unsafe { ddog_trace_exporter_error_free(Some(error)) }; } + + #[test] + fn from_trace_exporter_error_builder_test() { + use data_pipeline::trace_exporter::error::{BuilderErrorKind, TraceExporterError}; + + let builder_error = + TraceExporterError::Builder(BuilderErrorKind::InvalidUri("bad://url".to_string())); + let ffi_error = ExporterError::from(builder_error); + + assert_eq!(ffi_error.code, ExporterErrorCode::InvalidUrl); + assert!(!ffi_error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(ffi_error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Invalid URI provided: {details}"); + assert!(!ffi_error.context_fields.is_null()); + assert_eq!(ffi_error.context_count, 1); + + // Check context field content + let context_field = unsafe { &*ffi_error.context_fields }; + let key_str = unsafe { CStr::from_ptr(context_field.key).to_string_lossy() }; + let value_str = unsafe { CStr::from_ptr(context_field.value).to_string_lossy() }; + assert_eq!(key_str, "details"); + assert_eq!(value_str, "bad://url"); + } + + #[test] + fn from_trace_exporter_error_network_test() { + use data_pipeline::trace_exporter::error::TraceExporterError; + use std::io::{Error as IoError, ErrorKind}; + + // Create a network error by wrapping an IO error + let io_error = IoError::new(ErrorKind::ConnectionAborted, "Connection closed"); + let network_error = TraceExporterError::Io(io_error); + let ffi_error = ExporterError::from(network_error); + + assert_eq!(ffi_error.code, ExporterErrorCode::ConnectionAborted); + assert!(!ffi_error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(ffi_error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Connection aborted"); + assert!(!ffi_error.context_fields.is_null()); + assert!(ffi_error.context_count > 0); + } + + #[test] + fn from_trace_exporter_error_agent_test() { + use data_pipeline::trace_exporter::error::{AgentErrorKind, TraceExporterError}; + + let agent_error = TraceExporterError::Agent(AgentErrorKind::EmptyResponse); + let ffi_error = ExporterError::from(agent_error); + + assert_eq!(ffi_error.code, ExporterErrorCode::HttpEmptyBody); + assert!(!ffi_error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(ffi_error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Agent returned empty response"); + assert!(ffi_error.context_fields.is_null()); // AgentErrorKind has no context + assert_eq!(ffi_error.context_count, 0); + } + + #[test] + fn from_trace_exporter_error_without_template_test() { + use data_pipeline::trace_exporter::error::TraceExporterError; + use std::io::{Error as IoError, ErrorKind}; + + let io_error = + TraceExporterError::Io(IoError::new(ErrorKind::PermissionDenied, "Access denied")); + let ffi_error = ExporterError::from(io_error); + + assert_eq!(ffi_error.code, ExporterErrorCode::IoError); + assert!(!ffi_error.msg_template.is_null()); + let template_str = unsafe { CStr::from_ptr(ffi_error.msg_template).to_string_lossy() }; + assert_eq!(template_str, "Permission denied"); + assert!(!ffi_error.context_fields.is_null()); + assert!(ffi_error.context_count > 0); + } + + #[test] + fn from_trace_exporter_error_memory_safety_test() { + use data_pipeline::trace_exporter::error::{BuilderErrorKind, TraceExporterError}; + + // Create error with context + let builder_error = TraceExporterError::Builder(BuilderErrorKind::InvalidConfiguration( + "Missing service name".to_string(), + )); + let ffi_error = Box::new(ExporterError::from(builder_error)); + + // Verify context is properly allocated + assert_eq!(ffi_error.context_count, 1); + assert!(!ffi_error.context_fields.is_null()); + + // Memory should be properly freed when dropped + unsafe { ddog_trace_exporter_error_free(Some(ffi_error)) }; + // If this doesn't crash/leak, memory management is working correctly + } } diff --git a/data-pipeline-ffi/src/trace_exporter.rs b/data-pipeline-ffi/src/trace_exporter.rs index 1f8faa38d..baa85b635 100644 --- a/data-pipeline-ffi/src/trace_exporter.rs +++ b/data-pipeline-ffi/src/trace_exporter.rs @@ -16,9 +16,22 @@ use tracing::error; #[cfg(all(feature = "catch_panic", panic = "unwind"))] use std::panic::{catch_unwind, AssertUnwindSafe}; +const INVALID_ARGUMENT_TEMPLATE: &str = "Invalid argument provided"; +const PANIC_TEMPLATE: &str = "Operation panicked"; +const INVALID_INPUT_TEMPLATE: &str = "Invalid input"; +const FFI_ERROR_TEMPLATE: &str = "FFI error"; + macro_rules! gen_error { ($l:expr) => { - Some(Box::new(ExporterError::new($l, &$l.to_string()))) + Some(Box::new(ExporterError::new( + $l, + match $l { + ErrorCode::InvalidArgument => INVALID_ARGUMENT_TEMPLATE, + #[cfg(feature = "catch_panic")] + ErrorCode::Panic => PANIC_TEMPLATE, + _ => FFI_ERROR_TEMPLATE, + }, + ))) }; } @@ -54,7 +67,7 @@ fn sanitize_string(str: CharSlice) -> Result> { Ok(s) => Ok(s.to_string()), Err(_) => Err(Box::new(ExporterError::new( ErrorCode::InvalidInput, - &ErrorCode::InvalidInput.to_string(), + INVALID_INPUT_TEMPLATE, ))), } } @@ -593,7 +606,7 @@ mod tests { CharSlice::from("http://localhost"), ); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.url.as_ref().unwrap(), "http://localhost"); @@ -613,7 +626,7 @@ mod tests { config.as_mut(), CharSlice::from("1.0"), ); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.tracer_version.as_ref().unwrap(), "1.0"); @@ -632,7 +645,7 @@ mod tests { let error = ddog_trace_exporter_config_set_language(config.as_mut(), CharSlice::from("lang")); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.language.as_ref().unwrap(), "lang"); @@ -653,7 +666,7 @@ mod tests { CharSlice::from("0.1"), ); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.language_version.as_ref().unwrap(), "0.1"); @@ -675,7 +688,7 @@ mod tests { CharSlice::from("foo"), ); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.language_interpreter.as_ref().unwrap(), "foo"); @@ -696,7 +709,7 @@ mod tests { CharSlice::from("hostname"), ); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.hostname.as_ref().unwrap(), "hostname"); @@ -715,7 +728,7 @@ mod tests { let error = ddog_trace_exporter_config_set_env(config.as_mut(), CharSlice::from("env-test")); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.env.as_ref().unwrap(), "env-test"); @@ -734,7 +747,7 @@ mod tests { let error = ddog_trace_exporter_config_set_version(config.as_mut(), CharSlice::from("1.2")); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.version.as_ref().unwrap(), "1.2"); @@ -753,7 +766,7 @@ mod tests { let error = ddog_trace_exporter_config_set_service(config.as_mut(), CharSlice::from("service")); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert_eq!(cfg.service.as_ref().unwrap(), "service"); @@ -771,7 +784,7 @@ mod tests { let mut config = Some(TraceExporterConfig::default()); let error = ddog_trace_exporter_config_set_client_computed_stats(config.as_mut(), true); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert!(cfg.client_computed_stats); @@ -846,7 +859,7 @@ mod tests { Some(cfg.as_mut()), CharSlice::from("http://localhost"), ); - assert_eq!(error, None); + assert!(error.is_none()); let mut ptr: MaybeUninit> = MaybeUninit::uninit(); @@ -856,7 +869,7 @@ mod tests { ); let exporter = ptr.assume_init(); - assert_eq!(ret, None); + assert!(ret.is_none()); ddog_trace_exporter_free(exporter); ddog_trace_exporter_config_free(cfg); @@ -875,7 +888,7 @@ mod tests { Some(cfg.as_mut()), CharSlice::from("service"), ); - assert_eq!(error, None); + assert!(error.is_none()); ddog_trace_exporter_error_free(error); @@ -967,7 +980,7 @@ mod tests { let exporter = ptr.assume_init(); - assert_eq!(ret, None); + assert!(ret.is_none()); let data = rmp_serde::to_vec_named::>>(&vec![vec![]]).unwrap(); let traces = ByteSlice::new(&data); @@ -977,7 +990,7 @@ mod tests { 0, Some(NonNull::new_unchecked(&mut response).cast()), ); - assert_eq!(ret, None); + assert!(ret.is_none()); assert_eq!( String::from_utf8_lossy(&response.assume_init().body.unwrap()), r#"{ @@ -1034,7 +1047,7 @@ mod tests { let exporter = ptr.assume_init(); - assert_eq!(ret, None); + assert!(ret.is_none()); let data = vec![0x90]; let traces = ByteSlice::new(&data); @@ -1047,7 +1060,7 @@ mod tests { Some(NonNull::new_unchecked(&mut response).cast()), ); mock_traces.assert(); - assert_eq!(ret, None); + assert!(ret.is_none()); assert_eq!( String::from_utf8_lossy(&response.assume_init().body.unwrap()), response_body @@ -1113,7 +1126,7 @@ mod tests { let exporter = ptr.assume_init(); - assert_eq!(ret, None); + assert!(ret.is_none()); let data = vec![0x90]; let traces = ByteSlice::new(&data); @@ -1126,7 +1139,7 @@ mod tests { Some(NonNull::new_unchecked(&mut response).cast()), ); mock_traces.assert(); - assert_eq!(ret, None); + assert!(ret.is_none()); assert_eq!( String::from_utf8_lossy(&response.assume_init().body.unwrap()), response_body @@ -1159,7 +1172,7 @@ mod tests { assert!(!config.as_ref().unwrap().health_metrics_enabled); let error = ddog_trace_exporter_config_enable_health_metrics(config.as_mut(), true); - assert_eq!(error, None); + assert!(error.is_none()); let cfg = config.unwrap(); assert!(cfg.health_metrics_enabled); diff --git a/data-pipeline/src/trace_exporter/error.rs b/data-pipeline/src/trace_exporter/error.rs index 82b7db039..98af4abbc 100644 --- a/data-pipeline/src/trace_exporter/error.rs +++ b/data-pipeline/src/trace_exporter/error.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::telemetry::error::TelemetryError; -use crate::trace_exporter::msgpack_decoder::decode::error::DecodeError; +use datadog_trace_utils::msgpack_decoder::decode::error::DecodeError; use ddcommon::hyper_migration; use hyper::http::StatusCode; use hyper::Error as HyperError; @@ -10,6 +10,49 @@ use rmp_serde::encode::Error as EncodeError; use std::error::Error; use std::fmt::{Debug, Display}; +/// Context data for structured error information. +/// Contains key-value pairs that can be safely used for logging or debugging. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ErrorContext { + fields: Vec<(String, String)>, +} + +impl ErrorContext { + /// Creates a new empty context. + pub fn new() -> Self { + Self { fields: Vec::new() } + } + + /// Adds a key-value pair to the context. + pub fn with_field(mut self, key: impl Into, value: impl Into) -> Self { + self.fields.push((key.into(), value.into())); + self + } + + /// Returns all context fields as key-value pairs. + pub fn fields(&self) -> &[(String, String)] { + &self.fields + } + + /// Checks if the context is empty. + pub fn is_empty(&self) -> bool { + self.fields.is_empty() + } +} + +/// Trait for errors that can provide template-based error messages. +pub trait ErrorTemplate { + /// Returns a static error message template. + /// May contain placeholders like {field_name} for structured data. + fn template(&self) -> &'static str; + + /// Returns structured context data that can be used to populate templates. + /// Default implementation returns empty context. + fn context(&self) -> ErrorContext { + ErrorContext::new() + } +} + /// Represents different kinds of errors that can occur when interacting with the agent. #[derive(Debug, PartialEq)] pub enum AgentErrorKind { @@ -17,6 +60,10 @@ pub enum AgentErrorKind { EmptyResponse, } +impl AgentErrorKind { + const EMPTY_RESPONSE_TEMPLATE: &'static str = "Agent returned empty response"; +} + impl Display for AgentErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -25,6 +72,12 @@ impl Display for AgentErrorKind { } } +impl ErrorTemplate for AgentErrorKind { + fn template(&self) -> &'static str { + Self::EMPTY_RESPONSE_TEMPLATE + } +} + /// Represents different kinds of errors that can occur during the builder process. #[derive(Debug, PartialEq)] pub enum BuilderErrorKind { @@ -37,6 +90,13 @@ pub enum BuilderErrorKind { InvalidConfiguration(String), } +impl BuilderErrorKind { + const INVALID_URI_TEMPLATE: &'static str = "Invalid URI provided: {details}"; + const INVALID_TELEMETRY_CONFIG_TEMPLATE: &'static str = + "Invalid telemetry configuration: {details}"; + const INVALID_CONFIGURATION_TEMPLATE: &'static str = "Invalid configuration: {details}"; +} + impl Display for BuilderErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -51,6 +111,30 @@ impl Display for BuilderErrorKind { } } +impl ErrorTemplate for BuilderErrorKind { + fn template(&self) -> &'static str { + match self { + BuilderErrorKind::InvalidUri(_) => Self::INVALID_URI_TEMPLATE, + BuilderErrorKind::InvalidTelemetryConfig(_) => Self::INVALID_TELEMETRY_CONFIG_TEMPLATE, + BuilderErrorKind::InvalidConfiguration(_) => Self::INVALID_CONFIGURATION_TEMPLATE, + } + } + + fn context(&self) -> ErrorContext { + match self { + BuilderErrorKind::InvalidUri(details) => { + ErrorContext::new().with_field("details", details) + } + BuilderErrorKind::InvalidTelemetryConfig(details) => { + ErrorContext::new().with_field("details", details) + } + BuilderErrorKind::InvalidConfiguration(details) => { + ErrorContext::new().with_field("details", details) + } + } + } +} + /// Represents different kinds of internal errors. #[derive(Debug, PartialEq)] pub enum InternalErrorKind { @@ -59,6 +143,11 @@ pub enum InternalErrorKind { InvalidWorkerState(String), } +impl InternalErrorKind { + const INVALID_WORKER_STATE_TEMPLATE: &'static str = + "Background worker in invalid state: {details}"; +} + impl Display for InternalErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -69,6 +158,20 @@ impl Display for InternalErrorKind { } } +impl ErrorTemplate for InternalErrorKind { + fn template(&self) -> &'static str { + Self::INVALID_WORKER_STATE_TEMPLATE + } + + fn context(&self) -> ErrorContext { + match self { + InternalErrorKind::InvalidWorkerState(details) => { + ErrorContext::new().with_field("details", details) + } + } + } +} + /// Represents different kinds of network errors. #[derive(Copy, Clone, Debug)] pub enum NetworkErrorKind { @@ -90,6 +193,32 @@ pub enum NetworkErrorKind { WrongStatus, } +impl NetworkErrorKind { + const BODY_TEMPLATE: &'static str = "Error processing request/response body"; + const CANCELED_TEMPLATE: &'static str = "Request was canceled"; + const CONNECTION_CLOSED_TEMPLATE: &'static str = "Connection was closed"; + const MESSAGE_TOO_LARGE_TEMPLATE: &'static str = "Message size exceeds limit"; + const PARSE_TEMPLATE: &'static str = "Error parsing network response"; + const TIMED_OUT_TEMPLATE: &'static str = "Request timed out"; + const UNKNOWN_TEMPLATE: &'static str = "Unknown network error"; + const WRONG_STATUS_TEMPLATE: &'static str = "Unexpected status code received"; +} + +impl ErrorTemplate for NetworkErrorKind { + fn template(&self) -> &'static str { + match self { + NetworkErrorKind::Body => Self::BODY_TEMPLATE, + NetworkErrorKind::Canceled => Self::CANCELED_TEMPLATE, + NetworkErrorKind::ConnectionClosed => Self::CONNECTION_CLOSED_TEMPLATE, + NetworkErrorKind::MessageTooLarge => Self::MESSAGE_TOO_LARGE_TEMPLATE, + NetworkErrorKind::Parse => Self::PARSE_TEMPLATE, + NetworkErrorKind::TimedOut => Self::TIMED_OUT_TEMPLATE, + NetworkErrorKind::Unknown => Self::UNKNOWN_TEMPLATE, + NetworkErrorKind::WrongStatus => Self::WRONG_STATUS_TEMPLATE, + } + } +} + /// Represents a network error, containing the kind of error and the source error. #[derive(Debug)] pub struct NetworkError { @@ -130,12 +259,27 @@ impl Display for NetworkError { } } +impl ErrorTemplate for NetworkError { + fn template(&self) -> &'static str { + self.kind.template() + } + + fn context(&self) -> ErrorContext { + self.kind.context() + } +} + #[derive(Debug, PartialEq)] pub struct RequestError { code: StatusCode, msg: String, } +impl RequestError { + const REQUEST_ERROR_TEMPLATE: &'static str = + "Agent responded with error status {status_code}: {response}"; +} + impl Display for RequestError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( @@ -163,11 +307,28 @@ impl RequestError { } } +impl ErrorTemplate for RequestError { + fn template(&self) -> &'static str { + Self::REQUEST_ERROR_TEMPLATE + } + + fn context(&self) -> ErrorContext { + ErrorContext::new() + .with_field("status_code", self.code.as_u16().to_string()) + .with_field("response", &self.msg) + } +} + #[derive(Debug)] pub enum ShutdownError { TimedOut(std::time::Duration), } +impl ShutdownError { + const TIMED_OUT_TEMPLATE: &'static str = + "Shutdown operation timed out after {timeout_seconds} seconds"; +} + impl Display for ShutdownError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -178,6 +339,46 @@ impl Display for ShutdownError { } } +impl ErrorTemplate for ShutdownError { + fn template(&self) -> &'static str { + Self::TIMED_OUT_TEMPLATE + } + + fn context(&self) -> ErrorContext { + match self { + ShutdownError::TimedOut(duration) => ErrorContext::new() + .with_field("timeout_seconds", duration.as_secs_f32().to_string()), + } + } +} + +/// Local ErrorTemplate implementation for DecodeError to avoid dependency on trace-utils. +impl ErrorTemplate for DecodeError { + fn template(&self) -> &'static str { + match self { + DecodeError::InvalidConversion(_) => "Failed to convert decoded value: {details}", + DecodeError::InvalidType(_) => "Invalid type in trace payload: {details}", + DecodeError::InvalidFormat(_) => "Invalid msgpack format: {details}", + DecodeError::IOError => "Failed to read from buffer", + DecodeError::Utf8Error(_) => "Failed to decode UTF-8 string: {details}", + } + } + + fn context(&self) -> ErrorContext { + match self { + DecodeError::InvalidConversion(details) => { + ErrorContext::new().with_field("details", details) + } + DecodeError::InvalidType(details) => ErrorContext::new().with_field("details", details), + DecodeError::InvalidFormat(details) => { + ErrorContext::new().with_field("details", details) + } + DecodeError::IOError => ErrorContext::new(), + DecodeError::Utf8Error(details) => ErrorContext::new().with_field("details", details), + } + } +} + /// TraceExporterError holds different types of errors that occur when handling traces. #[derive(Debug)] pub enum TraceExporterError { @@ -205,22 +406,26 @@ pub enum TraceExporterError { impl Display for TraceExporterError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - TraceExporterError::Agent(e) => write!(f, "Agent response processing: {e}"), - TraceExporterError::Builder(e) => write!(f, "Invalid builder input: {e}"), - TraceExporterError::Internal(e) => write!(f, "Internal: {e}"), - TraceExporterError::Deserialization(e) => { - write!(f, "Deserialization of incoming payload: {e}") - } - TraceExporterError::Io(e) => write!(f, "IO: {e}"), - TraceExporterError::Shutdown(e) => write!(f, "Shutdown: {e}"), - TraceExporterError::Telemetry(e) => write!(f, "Telemetry: {e}"), - TraceExporterError::Network(e) => write!(f, "Network: {e}"), - TraceExporterError::Request(e) => write!(f, "Agent responded with an error code: {e}"), - TraceExporterError::Serialization(e) => { - write!(f, "Serialization of trace payload payload: {e}") + // Use template + context pattern for consistent formatting across all variants + let template = self.template(); + let context = self.context(); + + // Start with the template + write!(f, "{}", template)?; + + // Add context data if available + if !context.fields().is_empty() { + write!(f, " (")?; + for (i, (key, value)) in context.fields().iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{}: {}", key, value)?; } + write!(f, ")")?; } + + Ok(()) } } @@ -330,6 +535,86 @@ impl From for TraceExporterError { } } +impl TraceExporterError { + fn io_error_template(io_error: &std::io::Error) -> &'static str { + match io_error.kind() { + std::io::ErrorKind::NotFound => "File or resource not found", + std::io::ErrorKind::PermissionDenied => "Permission denied", + std::io::ErrorKind::ConnectionRefused => "Connection refused", + std::io::ErrorKind::ConnectionReset => "Connection reset by peer", + std::io::ErrorKind::ConnectionAborted => "Connection aborted", + std::io::ErrorKind::TimedOut => "Operation timed out", + std::io::ErrorKind::AddrInUse => "Address already in use", + _ => "Input/Output error occurred", + } + } + + fn io_error_context(io_error: &std::io::Error) -> ErrorContext { + let mut context = ErrorContext::new(); + + // Add the raw OS error code if available + if let Some(raw_os_error) = io_error.raw_os_error() { + context = context.with_field("os_error_code", raw_os_error.to_string()); + } + + // Add the inner error message if it provides additional details + let error_msg = io_error.to_string(); + if !error_msg.is_empty() && error_msg != io_error.kind().to_string() { + context = context.with_field("details", &error_msg); + } + + context + } +} + +impl ErrorTemplate for TraceExporterError { + fn template(&self) -> &'static str { + match self { + TraceExporterError::Agent(e) => e.template(), + TraceExporterError::Builder(e) => e.template(), + TraceExporterError::Internal(e) => e.template(), + TraceExporterError::Network(e) => e.template(), + TraceExporterError::Request(e) => e.template(), + TraceExporterError::Shutdown(e) => e.template(), + TraceExporterError::Deserialization(e) => e.template(), + TraceExporterError::Io(io_error) => Self::io_error_template(io_error), + TraceExporterError::Telemetry(_) => "Telemetry operation failed: {details}", + TraceExporterError::Serialization(_) => "Failed to serialize data: {details}", + } + } + + fn context(&self) -> ErrorContext { + match self { + TraceExporterError::Agent(e) => e.context(), + TraceExporterError::Builder(e) => e.context(), + TraceExporterError::Internal(e) => e.context(), + TraceExporterError::Network(e) => e.context(), + TraceExporterError::Request(e) => e.context(), + TraceExporterError::Shutdown(e) => e.context(), + TraceExporterError::Deserialization(e) => e.context(), + TraceExporterError::Io(io_error) => Self::io_error_context(io_error), + TraceExporterError::Telemetry(msg) => { + ErrorContext::new().with_field("details", msg.as_str()) + } + TraceExporterError::Serialization(encode_error) => { + ErrorContext::new().with_field("details", encode_error.to_string()) + } + } + } +} + +impl TraceExporterError { + /// Returns the static error message template. + pub fn template(&self) -> &'static str { + ErrorTemplate::template(self) + } + + /// Returns structured context data for the error. + pub fn context(&self) -> ErrorContext { + ErrorTemplate::context(self) + } +} + impl Error for TraceExporterError {} #[cfg(test)] @@ -342,4 +627,207 @@ mod tests { assert_eq!(error.status(), StatusCode::NOT_FOUND); assert_eq!(error.msg(), "Not found") } + + #[test] + fn test_error_context() { + let context = ErrorContext::new(); + assert!(context.is_empty()); + assert_eq!(context.fields().len(), 0); + + let context = ErrorContext::new() + .with_field("key1", "value1") + .with_field("key2", "value2"); + + assert!(!context.is_empty()); + assert_eq!(context.fields().len(), 2); + assert_eq!( + context.fields()[0], + ("key1".to_string(), "value1".to_string()) + ); + assert_eq!( + context.fields()[1], + ("key2".to_string(), "value2".to_string()) + ); + } + + #[test] + fn test_agent_error_template() { + let error = AgentErrorKind::EmptyResponse; + assert_eq!(error.template(), "Agent returned empty response"); + assert!(error.context().is_empty()); + } + + #[test] + fn test_builder_error_template() { + let error = BuilderErrorKind::InvalidUri("invalid://url".to_string()); + assert_eq!(error.template(), "Invalid URI provided: {details}"); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("details".to_string(), "invalid://url".to_string()) + ); + + let error = BuilderErrorKind::InvalidTelemetryConfig("missing field".to_string()); + assert_eq!( + error.template(), + "Invalid telemetry configuration: {details}" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("details".to_string(), "missing field".to_string()) + ); + + let error = BuilderErrorKind::InvalidConfiguration("bad setting".to_string()); + assert_eq!(error.template(), "Invalid configuration: {details}"); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("details".to_string(), "bad setting".to_string()) + ); + } + + #[test] + fn test_internal_error_template() { + let error = InternalErrorKind::InvalidWorkerState("worker crashed".to_string()); + assert_eq!( + error.template(), + "Background worker in invalid state: {details}" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("details".to_string(), "worker crashed".to_string()) + ); + } + + #[test] + fn test_network_error_kind_templates() { + assert_eq!( + NetworkErrorKind::Body.template(), + "Error processing request/response body" + ); + assert_eq!( + NetworkErrorKind::Canceled.template(), + "Request was canceled" + ); + assert_eq!( + NetworkErrorKind::ConnectionClosed.template(), + "Connection was closed" + ); + assert_eq!( + NetworkErrorKind::MessageTooLarge.template(), + "Message size exceeds limit" + ); + assert_eq!( + NetworkErrorKind::Parse.template(), + "Error parsing network response" + ); + assert_eq!(NetworkErrorKind::TimedOut.template(), "Request timed out"); + assert_eq!( + NetworkErrorKind::Unknown.template(), + "Unknown network error" + ); + assert_eq!( + NetworkErrorKind::WrongStatus.template(), + "Unexpected status code received" + ); + + // All network error kinds should have empty context by default + assert!(NetworkErrorKind::Body.context().is_empty()); + assert!(NetworkErrorKind::Canceled.context().is_empty()); + assert!(NetworkErrorKind::ConnectionClosed.context().is_empty()); + assert!(NetworkErrorKind::MessageTooLarge.context().is_empty()); + assert!(NetworkErrorKind::Parse.context().is_empty()); + assert!(NetworkErrorKind::TimedOut.context().is_empty()); + assert!(NetworkErrorKind::Unknown.context().is_empty()); + assert!(NetworkErrorKind::WrongStatus.context().is_empty()); + } + + #[test] + fn test_error_context_chaining() { + let context = ErrorContext::new() + .with_field("host", "example.com") + .with_field("port", "443") + .with_field("timeout", "5000"); + + assert_eq!(context.fields().len(), 3); + assert_eq!( + context.fields()[0], + ("host".to_string(), "example.com".to_string()) + ); + assert_eq!(context.fields()[1], ("port".to_string(), "443".to_string())); + assert_eq!( + context.fields()[2], + ("timeout".to_string(), "5000".to_string()) + ); + } + + #[test] + fn test_request_error_template() { + let error = RequestError::new(StatusCode::NOT_FOUND, "Resource not found"); + assert_eq!( + error.template(), + "Agent responded with error status {status_code}: {response}" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 2); + assert_eq!( + context.fields()[0], + ("status_code".to_string(), "404".to_string()) + ); + assert_eq!( + context.fields()[1], + ("response".to_string(), "Resource not found".to_string()) + ); + + let error = RequestError::new(StatusCode::INTERNAL_SERVER_ERROR, "Server error"); + assert_eq!( + error.template(), + "Agent responded with error status {status_code}: {response}" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 2); + assert_eq!( + context.fields()[0], + ("status_code".to_string(), "500".to_string()) + ); + assert_eq!( + context.fields()[1], + ("response".to_string(), "Server error".to_string()) + ); + } + + #[test] + fn test_shutdown_error_template() { + use std::time::Duration; + + let error = ShutdownError::TimedOut(Duration::from_secs(5)); + assert_eq!( + error.template(), + "Shutdown operation timed out after {timeout_seconds} seconds" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("timeout_seconds".to_string(), "5".to_string()) + ); + + let error = ShutdownError::TimedOut(Duration::from_millis(2500)); + assert_eq!( + error.template(), + "Shutdown operation timed out after {timeout_seconds} seconds" + ); + let context = error.context(); + assert_eq!(context.fields().len(), 1); + assert_eq!( + context.fields()[0], + ("timeout_seconds".to_string(), "2.5".to_string()) + ); + } } diff --git a/examples/ffi/trace_exporter.c b/examples/ffi/trace_exporter.c index 90500cf82..04070c086 100644 --- a/examples/ffi/trace_exporter.c +++ b/examples/ffi/trace_exporter.c @@ -15,7 +15,35 @@ enum { }; void handle_error(ddog_TraceExporterError *err) { - fprintf(stderr, "Operation failed with error: %d, reason: %s\n", err->code, err->msg); + fprintf(stderr, "Operation failed with error: %d\n", err->code); + + // Example: Use the template for external logging (safe, no sensitive data) + if (err->msg_template) { + fprintf(stderr, "Safe template message: %s\n", err->msg_template); + } + + // Example: Use context fields for internal debugging (may contain sensitive data) + if (err->context_fields && err->context_count > 0) { + fprintf(stderr, "Debug context (%zu fields):\n", err->context_count); + for (size_t i = 0; i < err->context_count; i++) { + fprintf(stderr, " %s: %s\n", err->context_fields[i].key, err->context_fields[i].value); + } + } + + // Example: Consumer can combine template + context for full message if needed + if (err->msg_template) { + fprintf(stderr, "Combined message: %s", err->msg_template); + if (err->context_fields && err->context_count > 0) { + fprintf(stderr, " ("); + for (size_t i = 0; i < err->context_count; i++) { + if (i > 0) fprintf(stderr, ", "); + fprintf(stderr, "%s: %s", err->context_fields[i].key, err->context_fields[i].value); + } + fprintf(stderr, ")"); + } + fprintf(stderr, "\n"); + } + ddog_trace_exporter_error_free(err); } @@ -102,7 +130,7 @@ int main(int argc, char** argv) ret = ddog_trace_exporter_send(trace_exporter, buffer, 0, &response); - assert(ret->code == DDOG_TRACE_EXPORTER_ERROR_CODE_SERDE); + assert(ret->code == DDOG_TRACE_EXPORTER_ERROR_CODE_INVALID_DATA); if (ret) { error = ERROR_SEND; handle_error(ret);