From 1b99707159f6d4a5a0be1fde0f096ebe24351809 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Fri, 21 Nov 2025 14:39:07 +0100 Subject: [PATCH 01/16] Retrieve panic message when crashing --- libdd-crashtracker/src/collector/api.rs | 10 ++- .../src/collector/collector_manager.rs | 5 ++ .../src/collector/crash_handler.rs | 79 +++++++++++++++++++ libdd-crashtracker/src/collector/emitters.rs | 14 ++++ libdd-crashtracker/src/crash_info/builder.rs | 4 + .../src/receiver/receive_report.rs | 28 ++++--- libdd-crashtracker/src/shared/constants.rs | 2 + 7 files changed, 130 insertions(+), 12 deletions(-) diff --git a/libdd-crashtracker/src/collector/api.rs b/libdd-crashtracker/src/collector/api.rs index a934c05d41..91e1e78a10 100644 --- a/libdd-crashtracker/src/collector/api.rs +++ b/libdd-crashtracker/src/collector/api.rs @@ -4,9 +4,10 @@ use super::{crash_handler::enable, receiver_manager::Receiver}; use crate::{ - clear_spans, clear_traces, collector::signal_handler_manager::register_crash_handlers, - crash_info::Metadata, reset_counters, shared::configuration::CrashtrackerReceiverConfig, - update_config, update_metadata, CrashtrackerConfiguration, + clear_spans, clear_traces, collector::crash_handler::register_panic_hook, + collector::signal_handler_manager::register_crash_handlers, crash_info::Metadata, + reset_counters, shared::configuration::CrashtrackerReceiverConfig, update_config, + update_metadata, CrashtrackerConfiguration, }; pub static DEFAULT_SYMBOLS: [libc::c_int; 4] = @@ -43,6 +44,8 @@ pub fn on_fork( // The altstack (if any) is similarly unaffected by fork: // https://man7.org/linux/man-pages/man2/sigaltstack.2.html + // panic hook is unaffected by fork. + update_metadata(metadata)?; update_config(config)?; Receiver::update_stored_config(receiver_config)?; @@ -68,6 +71,7 @@ pub fn init( update_config(config.clone())?; Receiver::update_stored_config(receiver_config)?; register_crash_handlers(&config)?; + register_panic_hook()?; enable(); Ok(()) } diff --git a/libdd-crashtracker/src/collector/collector_manager.rs b/libdd-crashtracker/src/collector/collector_manager.rs index 3c818b0154..a3e3fb1e65 100644 --- a/libdd-crashtracker/src/collector/collector_manager.rs +++ b/libdd-crashtracker/src/collector/collector_manager.rs @@ -30,6 +30,7 @@ impl Collector { config: &CrashtrackerConfiguration, config_str: &str, metadata_str: &str, + message_ptr: *mut String, sig_info: *const siginfo_t, ucontext: *const ucontext_t, ) -> Result { @@ -45,6 +46,7 @@ impl Collector { config, config_str, metadata_str, + message_ptr, sig_info, ucontext, receiver.handle.uds_fd, @@ -66,10 +68,12 @@ impl Collector { } } +#[allow(clippy::too_many_arguments)] pub(crate) fn run_collector_child( config: &CrashtrackerConfiguration, config_str: &str, metadata_str: &str, + message_ptr: *mut String, sig_info: *const siginfo_t, ucontext: *const ucontext_t, uds_fd: RawFd, @@ -96,6 +100,7 @@ pub(crate) fn run_collector_child( config, config_str, metadata_str, + message_ptr, sig_info, ucontext, ppid, diff --git a/libdd-crashtracker/src/collector/crash_handler.rs b/libdd-crashtracker/src/collector/crash_handler.rs index eaaf188abb..7a9048b5f6 100644 --- a/libdd-crashtracker/src/collector/crash_handler.rs +++ b/libdd-crashtracker/src/collector/crash_handler.rs @@ -10,6 +10,8 @@ use crate::crash_info::Metadata; use crate::shared::configuration::CrashtrackerConfiguration; use libc::{c_void, siginfo_t, ucontext_t}; use libdd_common::timeout::TimeoutManager; +use std::panic; +use std::panic::PanicHookInfo; use std::ptr; use std::sync::atomic::Ordering::SeqCst; use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64}; @@ -35,6 +37,10 @@ use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64}; // `Box::from_raw` to recreate the box, then dropping it. static METADATA: AtomicPtr<(Metadata, String)> = AtomicPtr::new(ptr::null_mut()); static CONFIG: AtomicPtr<(CrashtrackerConfiguration, String)> = AtomicPtr::new(ptr::null_mut()); +static PANIC_MESSAGE: AtomicPtr = AtomicPtr::new(ptr::null_mut()); + +type PanicHook = Box) + Send + Sync>; +static PREVIOUS_PANIC_HOOK: AtomicPtr = AtomicPtr::new(ptr::null_mut()); #[derive(Debug, thiserror::Error)] pub enum CrashHandlerError { @@ -72,6 +78,73 @@ pub fn update_metadata(metadata: Metadata) -> anyhow::Result<()> { Ok(()) } +/// Register the panic hook. +/// +/// This function is used to register the panic hook and store the previous hook. +/// PRECONDITIONS: +/// None +/// SAFETY: +/// Crash-tracking functions are not guaranteed to be reentrant. +/// No other crash-handler functions should be called concurrently. +/// ATOMICITY: +/// This function uses a swap on an atomic pointer. +pub fn register_panic_hook() -> anyhow::Result<()> { + // register only once, if it is already registered, do nothing + if !PREVIOUS_PANIC_HOOK.load(SeqCst).is_null() { + return Ok(()); + } + + let old_hook = panic::take_hook(); + let old_hook_ptr = Box::into_raw(Box::new(old_hook)); + PREVIOUS_PANIC_HOOK.swap(old_hook_ptr, SeqCst); + panic::set_hook(Box::new(panic_hook)); + Ok(()) +} + +/// The panic hook function. +/// +/// This function is used to update the metadata with the panic message +/// and call the previous hook. +/// PRECONDITIONS: +/// None +/// SAFETY: +/// Crash-tracking functions are not guaranteed to be reentrant. +/// No other crash-handler functions should be called concurrently. +/// ATOMICITY: +/// This function uses a load on an atomic pointer. +fn panic_hook(panic_info: &PanicHookInfo<'_>) { + if let Some(s) = panic_info.payload().downcast_ref::<&str>() { + let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst); + // message_ptr should be null, but just in case. + if !message_ptr.is_null() { + unsafe { + std::mem::drop(Box::from_raw(message_ptr)); + } + } + } + call_previous_panic_hook(panic_info); +} + +/// Call the previous panic hook. +/// +/// This function is used to call the previous panic hook. +/// PRECONDITIONS: +/// None +/// SAFETY: +/// Crash-tracking functions are not guaranteed to be reentrant. +/// No other crash-handler functions should be called concurrently. +fn call_previous_panic_hook(panic_info: &PanicHookInfo<'_>) { + let old_hook_ptr = PREVIOUS_PANIC_HOOK.load(SeqCst); + if !old_hook_ptr.is_null() { + // Safety: This pointer can only come from Box::into_raw above in register_panic_hook. + // We borrow it here without taking ownership so it remains valid for future calls. + unsafe { + let old_hook = &*old_hook_ptr; + old_hook(panic_info); + } + } +} + /// Updates the crashtracker config for this process /// Config is stored in a global variable and sent to the crashtracking /// receiver when a crash occurs. @@ -172,6 +245,11 @@ fn handle_posix_signal_impl( } let (_metadata, metadata_string) = unsafe { &*metadata_ptr }; + // Get the panic message pointer but don't dereference or deallocate in signal handler. + // The collector child process will handle converting this to a String after forking. + // Leak of the message pointer is ok here. + let message_ptr = PANIC_MESSAGE.swap(ptr::null_mut(), SeqCst); + let timeout_manager = TimeoutManager::new(config.timeout()); // Optionally, create the receiver. This all hinges on whether or not the configuration has a @@ -190,6 +268,7 @@ fn handle_posix_signal_impl( config, config_str, metadata_string, + message_ptr, sig_info, ucontext, )?; diff --git a/libdd-crashtracker/src/collector/emitters.rs b/libdd-crashtracker/src/collector/emitters.rs index f609c9f9a1..45a4b16362 100644 --- a/libdd-crashtracker/src/collector/emitters.rs +++ b/libdd-crashtracker/src/collector/emitters.rs @@ -132,15 +132,18 @@ unsafe fn emit_backtrace_by_frames( Ok(()) } +#[allow(clippy::too_many_arguments)] pub(crate) fn emit_crashreport( pipe: &mut impl Write, config: &CrashtrackerConfiguration, config_str: &str, metadata_string: &str, + message_ptr: *mut String, sig_info: *const siginfo_t, ucontext: *const ucontext_t, ppid: i32, ) -> Result<(), EmitterError> { + emit_message(pipe, message_ptr)?; emit_metadata(pipe, metadata_string)?; emit_config(pipe, config_str)?; emit_siginfo(pipe, sig_info)?; @@ -191,6 +194,17 @@ fn emit_metadata(w: &mut impl Write, metadata_str: &str) -> Result<(), EmitterEr Ok(()) } +fn emit_message(w: &mut impl Write, message_ptr: *mut String) -> Result<(), EmitterError> { + if !message_ptr.is_null() { + let message = unsafe { &*message_ptr }; + writeln!(w, "{DD_CRASHTRACK_BEGIN_MESSAGE}")?; + writeln!(w, "{message}")?; + writeln!(w, "{DD_CRASHTRACK_END_MESSAGE}")?; + w.flush()?; + } + Ok(()) +} + fn emit_procinfo(w: &mut impl Write, pid: i32) -> Result<(), EmitterError> { writeln!(w, "{DD_CRASHTRACK_BEGIN_PROCINFO}")?; writeln!(w, "{{\"pid\": {pid} }}")?; diff --git a/libdd-crashtracker/src/crash_info/builder.rs b/libdd-crashtracker/src/crash_info/builder.rs index fdff37f213..a7901b4803 100644 --- a/libdd-crashtracker/src/crash_info/builder.rs +++ b/libdd-crashtracker/src/crash_info/builder.rs @@ -405,6 +405,10 @@ impl CrashInfoBuilder { self.metadata.is_some() } } + + pub fn has_message(&self) -> bool { + self.error.message.is_some() + } } #[cfg(test)] diff --git a/libdd-crashtracker/src/receiver/receive_report.rs b/libdd-crashtracker/src/receiver/receive_report.rs index 5f44cb0e59..38428b01fe 100644 --- a/libdd-crashtracker/src/receiver/receive_report.rs +++ b/libdd-crashtracker/src/receiver/receive_report.rs @@ -116,6 +116,7 @@ pub(crate) enum StdinState { // may have lines that we need to accumulate depending on runtime (e.g. Python) RuntimeStackFrame(Vec), RuntimeStackString(Vec), + Message, } /// A state machine that processes data from the crash-tracker collector line by @@ -227,19 +228,27 @@ fn process_line( StdinState::SigInfo if line.starts_with(DD_CRASHTRACK_END_SIGINFO) => StdinState::Waiting, StdinState::SigInfo => { let sig_info: SigInfo = serde_json::from_str(line)?; - // By convention, siginfo is the first thing sent. - let message = format!( - "Process terminated with {:?} ({:?})", - sig_info.si_code_human_readable, sig_info.si_signo_human_readable - ); + if !builder.has_message() { + let message = format!( + "Process terminated with {:?} ({:?})", + sig_info.si_code_human_readable, sig_info.si_signo_human_readable + ); + builder.with_message(message)?; + } - builder.with_timestamp_now()?; - builder.with_sig_info(sig_info)?; - builder.with_incomplete(true)?; - builder.with_message(message)?; + builder + .with_timestamp_now()? + .with_sig_info(sig_info)? + .with_incomplete(true)?; StdinState::SigInfo } + StdinState::Message if line.starts_with(DD_CRASHTRACK_END_MESSAGE) => StdinState::Waiting, + StdinState::Message => { + builder.with_message(line.to_string())?; + StdinState::Message + } + StdinState::SpanIds if line.starts_with(DD_CRASHTRACK_END_SPAN_IDS) => StdinState::Waiting, StdinState::SpanIds => { let span_ids: Vec = serde_json::from_str(line)?; @@ -289,6 +298,7 @@ fn process_line( StdinState::ProcInfo } StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_SIGINFO) => StdinState::SigInfo, + StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_MESSAGE) => StdinState::Message, StdinState::Waiting if line.starts_with(DD_CRASHTRACK_BEGIN_SPAN_IDS) => { StdinState::SpanIds } diff --git a/libdd-crashtracker/src/shared/constants.rs b/libdd-crashtracker/src/shared/constants.rs index 43b56a95e3..85d6029ec1 100644 --- a/libdd-crashtracker/src/shared/constants.rs +++ b/libdd-crashtracker/src/shared/constants.rs @@ -17,6 +17,7 @@ pub const DD_CRASHTRACK_BEGIN_SPAN_IDS: &str = "DD_CRASHTRACK_BEGIN_SPAN_IDS"; pub const DD_CRASHTRACK_BEGIN_STACKTRACE: &str = "DD_CRASHTRACK_BEGIN_STACKTRACE"; pub const DD_CRASHTRACK_BEGIN_TRACE_IDS: &str = "DD_CRASHTRACK_BEGIN_TRACE_IDS"; pub const DD_CRASHTRACK_BEGIN_UCONTEXT: &str = "DD_CRASHTRACK_BEGIN_UCONTEXT"; +pub const DD_CRASHTRACK_BEGIN_MESSAGE: &str = "DD_CRASHTRACK_BEGIN_MESSAGE"; pub const DD_CRASHTRACK_DONE: &str = "DD_CRASHTRACK_DONE"; pub const DD_CRASHTRACK_END_ADDITIONAL_TAGS: &str = "DD_CRASHTRACK_END_ADDITIONAL_TAGS"; pub const DD_CRASHTRACK_END_CONFIG: &str = "DD_CRASHTRACK_END_CONFIG"; @@ -31,5 +32,6 @@ pub const DD_CRASHTRACK_END_SPAN_IDS: &str = "DD_CRASHTRACK_END_SPAN_IDS"; pub const DD_CRASHTRACK_END_STACKTRACE: &str = "DD_CRASHTRACK_END_STACKTRACE"; pub const DD_CRASHTRACK_END_TRACE_IDS: &str = "DD_CRASHTRACK_END_TRACE_IDS"; pub const DD_CRASHTRACK_END_UCONTEXT: &str = "DD_CRASHTRACK_END_UCONTEXT"; +pub const DD_CRASHTRACK_END_MESSAGE: &str = "DD_CRASHTRACK_END_MESSAGE"; pub const DD_CRASHTRACK_DEFAULT_TIMEOUT: Duration = Duration::from_millis(5_000); From 0b000f6169406e8d330cc0c610467153c2809169 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Mon, 24 Nov 2025 14:40:38 +0100 Subject: [PATCH 02/16] Improve and add tests --- bin_tests/src/bin/crashing_test_app.rs | 43 +++++++++-- bin_tests/src/lib.rs | 8 ++ bin_tests/tests/crashtracker_bin_test.rs | 73 +++++++++++++++++++ bin_tests/tests/test_the_tests.rs | 1 + .../src/collector/crash_handler.rs | 35 +++------ libdd-crashtracker/src/collector/emitters.rs | 30 +++++++- libdd-crashtracker/src/crash_info/builder.rs | 4 + .../src/crash_info/telemetry.rs | 66 ++++++++++++++--- .../src/receiver/receive_report.rs | 7 +- 9 files changed, 220 insertions(+), 47 deletions(-) diff --git a/bin_tests/src/bin/crashing_test_app.rs b/bin_tests/src/bin/crashing_test_app.rs index 42802a16c4..b5b0354dc2 100644 --- a/bin_tests/src/bin/crashing_test_app.rs +++ b/bin_tests/src/bin/crashing_test_app.rs @@ -23,8 +23,24 @@ mod unix { const TEST_COLLECTOR_TIMEOUT: Duration = Duration::from_secs(10); - #[inline(never)] - unsafe fn fn3() { + enum CrashType { + Segfault, + Panic, + } + + impl std::str::FromStr for CrashType { + type Err = anyhow::Error; + fn from_str(s: &str) -> Result { + match s { + "segfault" => Ok(CrashType::Segfault), + "panic" => Ok(CrashType::Panic), + _ => anyhow::bail!("Invalid crash type: {s}"), + } + } + } + + #[inline(always)] + unsafe fn cause_segfault() { #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] { std::arch::asm!("mov eax, [0]", options(nostack)); @@ -37,13 +53,25 @@ mod unix { } #[inline(never)] - fn fn2() { - unsafe { fn3() } + fn fn3(crash_type: CrashType) { + match crash_type { + CrashType::Segfault => { + unsafe { cause_segfault() }; + } + CrashType::Panic => { + panic!("program panicked"); + } + } + } + + #[inline(never)] + fn fn2(crash_type: CrashType) { + fn3(crash_type); } #[inline(never)] - fn fn1() { - fn2() + fn fn1(crash_type: CrashType) { + fn2(crash_type); } #[inline(never)] @@ -53,6 +81,7 @@ mod unix { let output_url = args.next().context("Unexpected number of arguments 1")?; let receiver_binary = args.next().context("Unexpected number of arguments 2")?; let output_dir = args.next().context("Unexpected number of arguments 3")?; + let crash_type = args.next().context("Unexpected number of arguments 4")?; anyhow::ensure!(args.next().is_none(), "unexpected extra arguments"); let stderr_filename = format!("{output_dir}/out.stderr"); @@ -100,7 +129,7 @@ mod unix { metadata, )?; - fn1(); + fn1(crash_type.parse().context("Invalid crash type")?); Ok(()) } } diff --git a/bin_tests/src/lib.rs b/bin_tests/src/lib.rs index 4aec1f6ef0..91b95dc7bb 100644 --- a/bin_tests/src/lib.rs +++ b/bin_tests/src/lib.rs @@ -46,6 +46,7 @@ pub struct ArtifactsBuild { pub artifact_type: ArtifactType, pub build_profile: BuildProfile, pub triple_target: Option, + pub panic_abort: Option, } fn inner_build_artifact(c: &ArtifactsBuild) -> anyhow::Result { @@ -58,6 +59,13 @@ fn inner_build_artifact(c: &ArtifactsBuild) -> anyhow::Result { ArtifactType::ExecutablePackage | ArtifactType::CDylib => build_cmd.arg("-p"), ArtifactType::Bin => build_cmd.arg("--bin"), }; + + if let Some(panic_abort) = c.panic_abort { + if panic_abort { + build_cmd.env("RUSTFLAGS", "-C panic=abort"); + } + } + build_cmd.arg(&c.name); let output = build_cmd.output().unwrap(); diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 5c70bdb5a9..1373b43a35 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -291,6 +291,79 @@ fn test_crash_tracking_errors_intake_uds_socket() { ); } +#[test] +#[cfg_attr(miri, ignore)] +fn test_crash_tracking_bin_panic() { + test_crash_tracking_app("panic"); +} + +#[test] +#[cfg_attr(miri, ignore)] +fn test_crash_tracking_bin_segfault() { + test_crash_tracking_app("segfault"); +} + +fn test_crash_tracking_app(crash_type: &str) { + use bin_tests::test_runner::run_custom_crash_test; + + // Set up custom artifacts: receiver + crashing_test_app with panic_abort + let crashtracker_receiver = ArtifactsBuild { + name: "test_crashtracker_receiver".to_owned(), + build_profile: BuildProfile::Release, + artifact_type: ArtifactType::Bin, + triple_target: None, + ..Default::default() + }; + + let crashing_app = ArtifactsBuild { + name: "crashing_test_app".to_owned(), + build_profile: BuildProfile::Debug, + artifact_type: ArtifactType::Bin, + triple_target: None, + panic_abort: Some(true), + ..Default::default() + }; + + let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashing_app]).unwrap(); + + // Create validator based on crash type + let crash_type_owned = crash_type.to_owned(); + let validator: ValidatorFn = Box::new(move |payload, _fixtures| { + let sig_info = &payload["sig_info"]; + let error = &payload["error"]; + + match crash_type_owned.as_str() { + "panic" => { + let expected_message = "program panicked"; + assert_eq!( + error["message"].as_str().unwrap(), + expected_message, + "Expected panic message for panic crash type" + ); + } + "segfault" => { + assert_error_message(&error["message"], sig_info); + } + _ => unreachable!("Invalid crash type: {}", crash_type_owned), + } + + Ok(()) + }); + + run_custom_crash_test( + &artifacts_map[&crashing_app], + |cmd, fixtures| { + cmd.arg(format!("file://{}", fixtures.crash_profile_path.display())) + .arg(&artifacts_map[&crashtracker_receiver]) + .arg(&fixtures.output_dir) + .arg(crash_type); + }, + false, // expect crash (not success) + validator, + ) + .unwrap(); +} + // ==================================================================================== // CALLSTACK VALIDATION TESTS - MIGRATED TO CUSTOM TEST RUNNER // ==================================================================================== diff --git a/bin_tests/tests/test_the_tests.rs b/bin_tests/tests/test_the_tests.rs index 9495387f52..afb3933acc 100644 --- a/bin_tests/tests/test_the_tests.rs +++ b/bin_tests/tests/test_the_tests.rs @@ -34,6 +34,7 @@ fn test_the_tests_inner(profile: BuildProfile) { build_profile: profile, artifact_type: ArtifactType::CDylib, triple_target: None, + panic_abort: None, }, &test_the_tests, ]; diff --git a/libdd-crashtracker/src/collector/crash_handler.rs b/libdd-crashtracker/src/collector/crash_handler.rs index 7a9048b5f6..fb31ba4fac 100644 --- a/libdd-crashtracker/src/collector/crash_handler.rs +++ b/libdd-crashtracker/src/collector/crash_handler.rs @@ -97,32 +97,19 @@ pub fn register_panic_hook() -> anyhow::Result<()> { let old_hook = panic::take_hook(); let old_hook_ptr = Box::into_raw(Box::new(old_hook)); PREVIOUS_PANIC_HOOK.swap(old_hook_ptr, SeqCst); - panic::set_hook(Box::new(panic_hook)); - Ok(()) -} - -/// The panic hook function. -/// -/// This function is used to update the metadata with the panic message -/// and call the previous hook. -/// PRECONDITIONS: -/// None -/// SAFETY: -/// Crash-tracking functions are not guaranteed to be reentrant. -/// No other crash-handler functions should be called concurrently. -/// ATOMICITY: -/// This function uses a load on an atomic pointer. -fn panic_hook(panic_info: &PanicHookInfo<'_>) { - if let Some(s) = panic_info.payload().downcast_ref::<&str>() { - let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst); - // message_ptr should be null, but just in case. - if !message_ptr.is_null() { - unsafe { - std::mem::drop(Box::from_raw(message_ptr)); + panic::set_hook(Box::new(|panic_info| { + if let Some(s) = panic_info.payload().downcast_ref::<&str>() { + let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst); + // message_ptr should be null, but just in case. + if !message_ptr.is_null() { + unsafe { + std::mem::drop(Box::from_raw(message_ptr)); + } } } - } - call_previous_panic_hook(panic_info); + call_previous_panic_hook(panic_info); + })); + Ok(()) } /// Call the previous panic hook. diff --git a/libdd-crashtracker/src/collector/emitters.rs b/libdd-crashtracker/src/collector/emitters.rs index 45a4b16362..f8c40f39fd 100644 --- a/libdd-crashtracker/src/collector/emitters.rs +++ b/libdd-crashtracker/src/collector/emitters.rs @@ -143,10 +143,13 @@ pub(crate) fn emit_crashreport( ucontext: *const ucontext_t, ppid: i32, ) -> Result<(), EmitterError> { - emit_message(pipe, message_ptr)?; - emit_metadata(pipe, metadata_string)?; emit_config(pipe, config_str)?; + emit_message(pipe, message_ptr)?; emit_siginfo(pipe, sig_info)?; + // send metadata after the config (needed to send the ping/report) + // after the message + // after the siginfo (if the message is not set, we use the siginfo to generate the message) + emit_metadata(pipe, metadata_string)?; emit_ucontext(pipe, ucontext)?; emit_procinfo(pipe, ppid)?; emit_counters(pipe)?; @@ -469,4 +472,27 @@ mod tests { "crashtracker itself must be filtered away, found 'backtrace::backtrace'" ); } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_nullptr() { + let mut buf = Vec::new(); + emit_message(&mut buf, std::ptr::null_mut()).expect("to work ;-)"); + assert!(buf.is_empty()); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message() { + let message = "test message"; + let message_ptr = Box::into_raw(Box::new(message.to_string())); + let mut buf = Vec::new(); + emit_message(&mut buf, message_ptr).expect("to work ;-)"); + let out = str::from_utf8(&buf).expect("to be valid UTF8"); + assert!(out.contains("BEGIN_MESSAGE")); + assert!(out.contains("END_MESSAGE")); + assert!(out.contains(message)); + // Clean up the allocated String + unsafe { drop(Box::from_raw(message_ptr)) }; + } } diff --git a/libdd-crashtracker/src/crash_info/builder.rs b/libdd-crashtracker/src/crash_info/builder.rs index a7901b4803..3a087d181a 100644 --- a/libdd-crashtracker/src/crash_info/builder.rs +++ b/libdd-crashtracker/src/crash_info/builder.rs @@ -383,6 +383,7 @@ impl CrashInfoBuilder { /// This method requires that the builder has a UUID and metadata set. /// Siginfo is optional for platforms that don't support it (like Windows) pub fn build_crash_ping(&self) -> anyhow::Result { + let message = self.error.message.clone(); let sig_info = self.sig_info.clone(); let metadata = self.metadata.clone().context("metadata is required")?; @@ -390,6 +391,9 @@ impl CrashInfoBuilder { if let Some(sig_info) = sig_info { builder = builder.with_sig_info(sig_info); } + if let Some(message) = message { + builder = builder.with_custom_message(message); + } builder.build() } diff --git a/libdd-crashtracker/src/crash_info/telemetry.rs b/libdd-crashtracker/src/crash_info/telemetry.rs index d54d8a8925..1b9dda6608 100644 --- a/libdd-crashtracker/src/crash_info/telemetry.rs +++ b/libdd-crashtracker/src/crash_info/telemetry.rs @@ -63,16 +63,16 @@ impl CrashPingBuilder { let sig_info = self.sig_info; let metadata = self.metadata.context("metadata is required")?; - let message = self.custom_message.unwrap_or_else(|| { - if let Some(ref sig_info) = sig_info { - format!( - "Crashtracker crash ping: crash processing started - Process terminated with {:?} ({:?})", - sig_info.si_code_human_readable, sig_info.si_signo_human_readable - ) - } else { - "Crashtracker crash ping: crash processing started - Process terminated".to_string() - } - }); + let message = if let Some(custom_message) = self.custom_message { + format!("Crashtracker crash ping: crash processing started - {custom_message}") + } else if let Some(ref sig_info) = sig_info { + format!( + "Crashtracker crash ping: crash processing started - Process terminated with {:?} ({:?})", + sig_info.si_code_human_readable, sig_info.si_signo_human_readable + ) + } else { + "Crashtracker crash ping: crash processing started - Process terminated".to_string() + }; Ok(CrashPing { crash_uuid: crash_uuid.to_string(), @@ -824,6 +824,52 @@ mod tests { assert_eq!(crash_ping.siginfo(), Some(&sig_info)); } + #[test] + #[cfg_attr(miri, ignore)] + fn test_crash_ping_with_message_generated_from_sig_info() { + let sig_info = crate::SigInfo::test_instance(99); + let metadata = Metadata::test_instance(2); + + // Build crash ping through CrashInfoBuilder + let mut crash_info_builder = CrashInfoBuilder::new(); + crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); + crash_info_builder.with_metadata(metadata.clone()).unwrap(); + let crash_ping = crash_info_builder.build_crash_ping().unwrap(); + + assert!(!crash_ping.crash_uuid().is_empty()); + assert!(Uuid::parse_str(crash_ping.crash_uuid()).is_ok()); + assert_eq!(crash_ping.message(), format!( + "Crashtracker crash ping: crash processing started - Process terminated with {:?} ({:?})", + sig_info.si_code_human_readable, sig_info.si_signo_human_readable + )); + assert_eq!(crash_ping.metadata(), &metadata); + assert_eq!(crash_ping.siginfo(), Some(&sig_info)); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_crash_ping_with_custom_message() { + let sig_info = crate::SigInfo::test_instance(99); + let metadata = Metadata::test_instance(2); + + // Build crash ping through CrashInfoBuilder + let mut crash_info_builder = CrashInfoBuilder::new(); + crash_info_builder.with_sig_info(sig_info.clone()).unwrap(); + crash_info_builder.with_metadata(metadata.clone()).unwrap(); + crash_info_builder + .with_message("my process panicked".to_string()) + .unwrap(); + let crash_ping = crash_info_builder.build_crash_ping().unwrap(); + + assert!(!crash_ping.crash_uuid().is_empty()); + assert!(Uuid::parse_str(crash_ping.crash_uuid()).is_ok()); + assert!(crash_ping + .message() + .contains("crash processing started - my process panicked")); + assert_eq!(crash_ping.metadata(), &metadata); + assert_eq!(crash_ping.siginfo(), Some(&sig_info)); + } + #[tokio::test] #[cfg_attr(miri, ignore)] async fn test_crash_ping_telemetry_upload_all_fields() -> anyhow::Result<()> { diff --git a/libdd-crashtracker/src/receiver/receive_report.rs b/libdd-crashtracker/src/receiver/receive_report.rs index 38428b01fe..f40b7339f9 100644 --- a/libdd-crashtracker/src/receiver/receive_report.rs +++ b/libdd-crashtracker/src/receiver/receive_report.rs @@ -236,10 +236,9 @@ fn process_line( builder.with_message(message)?; } - builder - .with_timestamp_now()? - .with_sig_info(sig_info)? - .with_incomplete(true)?; + builder.with_timestamp_now()?; + builder.with_sig_info(sig_info)?; + builder.with_incomplete(true)?; StdinState::SigInfo } From 65f63ffe0d755f5c64062fb9667dac6cc6d702df Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 09:09:09 +0100 Subject: [PATCH 03/16] Disable tests on macos, empty stack returned --- bin_tests/tests/crashtracker_bin_test.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 1373b43a35..1b343b4c76 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -291,13 +291,19 @@ fn test_crash_tracking_errors_intake_uds_socket() { ); } +/// For some reason, the next two tests fail on MacOS, because the callstack cannot be collected. +/// We get this error: +/// thread 'test_crash_tracking_bin_segfault' (88268) panicked at bin_tests/tests/crashtracker_bin_test.rs:250:5: +/// got Ok("Unable to process line: DD_CRASHTRACK_END_STACKTRACE. Error: Can't set non-existant stack complete\n") #[test] #[cfg_attr(miri, ignore)] +#[cfg(not(target_os = "macos"))] fn test_crash_tracking_bin_panic() { test_crash_tracking_app("panic"); } #[test] +#[cfg(not(target_os = "macos"))] #[cfg_attr(miri, ignore)] fn test_crash_tracking_bin_segfault() { test_crash_tracking_app("segfault"); From 1841443afcd776f9a9e9dd29f084526b67bab0e7 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 09:12:55 +0100 Subject: [PATCH 04/16] Do not replace RUSTFLAGS value --- bin_tests/src/lib.rs | 8 +++++++- bin_tests/tests/crashtracker_bin_test.rs | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bin_tests/src/lib.rs b/bin_tests/src/lib.rs index 91b95dc7bb..5c525782c0 100644 --- a/bin_tests/src/lib.rs +++ b/bin_tests/src/lib.rs @@ -62,7 +62,13 @@ fn inner_build_artifact(c: &ArtifactsBuild) -> anyhow::Result { if let Some(panic_abort) = c.panic_abort { if panic_abort { - build_cmd.env("RUSTFLAGS", "-C panic=abort"); + let existing_rustflags = std::env::var("RUSTFLAGS").unwrap_or_default(); + let new_rustflags = if existing_rustflags.is_empty() { + "-C panic=abort".to_string() + } else { + format!("{} -C panic=abort", existing_rustflags) + }; + build_cmd.env("RUSTFLAGS", new_rustflags); } } diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 1b343b4c76..3504a88201 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -293,8 +293,9 @@ fn test_crash_tracking_errors_intake_uds_socket() { /// For some reason, the next two tests fail on MacOS, because the callstack cannot be collected. /// We get this error: -/// thread 'test_crash_tracking_bin_segfault' (88268) panicked at bin_tests/tests/crashtracker_bin_test.rs:250:5: -/// got Ok("Unable to process line: DD_CRASHTRACK_END_STACKTRACE. Error: Can't set non-existant stack complete\n") +/// thread 'test_crash_tracking_bin_segfault' (88268) panicked at +/// bin_tests/tests/crashtracker_bin_test.rs:250:5: got Ok("Unable to process line: +/// DD_CRASHTRACK_END_STACKTRACE. Error: Can't set non-existant stack complete\n") #[test] #[cfg_attr(miri, ignore)] #[cfg(not(target_os = "macos"))] From 6cd5b7f16fcfd1ec6363091abda8322761901e25 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 09:44:08 +0100 Subject: [PATCH 05/16] Check previous hook being called --- bin_tests/src/bin/crashing_test_app.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/bin_tests/src/bin/crashing_test_app.rs b/bin_tests/src/bin/crashing_test_app.rs index b5b0354dc2..4630082635 100644 --- a/bin_tests/src/bin/crashing_test_app.rs +++ b/bin_tests/src/bin/crashing_test_app.rs @@ -14,6 +14,8 @@ mod unix { use anyhow::ensure; use anyhow::Context; use std::env; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Arc; use std::time::Duration; use libdd_common::{tag, Endpoint}; @@ -23,6 +25,7 @@ mod unix { const TEST_COLLECTOR_TIMEOUT: Duration = Duration::from_secs(10); + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CrashType { Segfault, Panic, @@ -117,6 +120,17 @@ mod unix { .collect(), }; + let crash_type = crash_type.parse().context("Invalid crash type")?; + let is_panic_mode = matches!(crash_type, CrashType::Panic); + + let called_panic_hook = Arc::new(AtomicBool::new(false)); + if is_panic_mode { + let called_panic_hook_clone = Arc::clone(&called_panic_hook); + std::panic::set_hook(Box::new(move |_| { + called_panic_hook_clone.store(true, Ordering::SeqCst); + })); + } + crashtracker::init( config, CrashtrackerReceiverConfig::new( @@ -129,7 +143,13 @@ mod unix { metadata, )?; - fn1(crash_type.parse().context("Invalid crash type")?); + fn1(crash_type); + + // If the panic hook was chained, it should have been called. + anyhow::ensure!( + !is_panic_mode || called_panic_hook.load(Ordering::SeqCst), + "panic hook was not called" + ); Ok(()) } } From 1d60a13d6cb8f02498d9f06ce7da38ba8e4b6bd0 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 15:31:26 +0100 Subject: [PATCH 06/16] Add comment about information emission order --- libdd-crashtracker/src/collector/emitters.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libdd-crashtracker/src/collector/emitters.rs b/libdd-crashtracker/src/collector/emitters.rs index f8c40f39fd..37b0eb6e6a 100644 --- a/libdd-crashtracker/src/collector/emitters.rs +++ b/libdd-crashtracker/src/collector/emitters.rs @@ -143,13 +143,16 @@ pub(crate) fn emit_crashreport( ucontext: *const ucontext_t, ppid: i32, ) -> Result<(), EmitterError> { + // The following order is important in order to emit the crash ping: + // - receiver expects the config + // - then message if any + // - then siginfo (if the message is not set, we use the siginfo to generate the message) + // - then metadata emit_config(pipe, config_str)?; emit_message(pipe, message_ptr)?; emit_siginfo(pipe, sig_info)?; - // send metadata after the config (needed to send the ping/report) - // after the message - // after the siginfo (if the message is not set, we use the siginfo to generate the message) emit_metadata(pipe, metadata_string)?; + // after the metadata the ping should have been sent emit_ucontext(pipe, ucontext)?; emit_procinfo(pipe, ppid)?; emit_counters(pipe)?; From ddead97b9ada47e1e18b228d07dfba9881f2f355 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 16:16:33 +0100 Subject: [PATCH 07/16] Add more unit tests --- .../src/collector/crash_handler.rs | 95 +++++++++++++++ libdd-crashtracker/src/collector/emitters.rs | 107 ++++++++++++++++- libdd-crashtracker/src/crash_info/builder.rs | 92 ++++++++++++++ .../src/receiver/receive_report.rs | 112 ++++++++++++++++++ 4 files changed, 402 insertions(+), 4 deletions(-) diff --git a/libdd-crashtracker/src/collector/crash_handler.rs b/libdd-crashtracker/src/collector/crash_handler.rs index fb31ba4fac..8254187957 100644 --- a/libdd-crashtracker/src/collector/crash_handler.rs +++ b/libdd-crashtracker/src/collector/crash_handler.rs @@ -266,3 +266,98 @@ fn handle_posix_signal_impl( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_register_panic_hook() { + assert!(PREVIOUS_PANIC_HOOK.load(SeqCst).is_null()); + + let result = register_panic_hook(); + assert!(result.is_ok()); + + assert!(!PREVIOUS_PANIC_HOOK.load(SeqCst).is_null()); + } + + #[test] + fn test_panic_message_storage_and_retrieval() { + // Test that panic messages can be stored and retrieved via atomic pointer + let test_message = "test panic message".to_string(); + let message_ptr = Box::into_raw(Box::new(test_message.clone())); + + // Store the message + let old_ptr = PANIC_MESSAGE.swap(message_ptr, SeqCst); + assert!(old_ptr.is_null()); // Should be null initially + + // Retrieve and verify + let retrieved_ptr = PANIC_MESSAGE.swap(ptr::null_mut(), SeqCst); + assert!(!retrieved_ptr.is_null()); + + unsafe { + let retrieved_message = *Box::from_raw(retrieved_ptr); + assert_eq!(retrieved_message, test_message); + } + } + + #[test] + fn test_panic_message_null_handling() { + // Test that null message pointers are handled correctly + PANIC_MESSAGE.store(ptr::null_mut(), SeqCst); + + let message_ptr = PANIC_MESSAGE.load(SeqCst); + assert!(message_ptr.is_null()); + + // Swapping null with null should be safe + let old_ptr = PANIC_MESSAGE.swap(ptr::null_mut(), SeqCst); + assert!(old_ptr.is_null()); + } + + #[test] + fn test_panic_message_replacement() { + // Test that replacing an existing message cleans up the old one + let message1 = "first message".to_string(); + let message2 = "second message".to_string(); + + let ptr1 = Box::into_raw(Box::new(message1)); + let ptr2 = Box::into_raw(Box::new(message2.clone())); + + PANIC_MESSAGE.store(ptr1, SeqCst); + let old_ptr = PANIC_MESSAGE.swap(ptr2, SeqCst); + + // Old pointer should be the first one + assert_eq!(old_ptr, ptr1); + + // Clean up both + unsafe { + drop(Box::from_raw(old_ptr)); + let final_ptr = PANIC_MESSAGE.swap(ptr::null_mut(), SeqCst); + let final_message = *Box::from_raw(final_ptr); + assert_eq!(final_message, message2); + } + } + + #[test] + fn test_metadata_update_atomic() { + // Test that metadata updates are atomic + let metadata = Metadata { + library_name: "test".to_string(), + library_version: "1.0.0".to_string(), + family: "test_family".to_string(), + tags: vec![], + }; + + let result = update_metadata(metadata.clone()); + assert!(result.is_ok()); + + // Verify metadata was stored + let metadata_ptr = METADATA.load(SeqCst); + assert!(!metadata_ptr.is_null()); + + unsafe { + let (stored_metadata, _) = &*metadata_ptr; + assert_eq!(stored_metadata.library_name, "test"); + } + } +} diff --git a/libdd-crashtracker/src/collector/emitters.rs b/libdd-crashtracker/src/collector/emitters.rs index 37b0eb6e6a..35e061d4e6 100644 --- a/libdd-crashtracker/src/collector/emitters.rs +++ b/libdd-crashtracker/src/collector/emitters.rs @@ -203,10 +203,12 @@ fn emit_metadata(w: &mut impl Write, metadata_str: &str) -> Result<(), EmitterEr fn emit_message(w: &mut impl Write, message_ptr: *mut String) -> Result<(), EmitterError> { if !message_ptr.is_null() { let message = unsafe { &*message_ptr }; - writeln!(w, "{DD_CRASHTRACK_BEGIN_MESSAGE}")?; - writeln!(w, "{message}")?; - writeln!(w, "{DD_CRASHTRACK_END_MESSAGE}")?; - w.flush()?; + if !message.trim().is_empty() { + writeln!(w, "{DD_CRASHTRACK_BEGIN_MESSAGE}")?; + writeln!(w, "{message}")?; + writeln!(w, "{DD_CRASHTRACK_END_MESSAGE}")?; + w.flush()?; + } } Ok(()) } @@ -498,4 +500,101 @@ mod tests { // Clean up the allocated String unsafe { drop(Box::from_raw(message_ptr)) }; } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_empty_string() { + let empty_message = String::new(); + let message_ptr = Box::into_raw(Box::new(empty_message)); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + + // Empty messages should not emit anything + assert!(buf.is_empty()); + + unsafe { drop(Box::from_raw(message_ptr)) }; + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_whitespace_only() { + // Whitespace-only messages should not be emitted + let whitespace_message = " \n\t ".to_string(); + let message_ptr = Box::into_raw(Box::new(whitespace_message)); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + + // Whitespace-only messages should not emit anything + assert!(buf.is_empty()); + + unsafe { drop(Box::from_raw(message_ptr)) }; + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_with_leading_trailing_whitespace() { + // Messages with content and whitespace should be emitted (with the whitespace) + let message_with_whitespace = " error message ".to_string(); + let message_ptr = Box::into_raw(Box::new(message_with_whitespace.clone())); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + let out = str::from_utf8(&buf).expect("to be valid UTF8"); + + // Should emit markers and preserve whitespace in content + assert!(out.contains("BEGIN_MESSAGE")); + assert!(out.contains("END_MESSAGE")); + assert!(out.contains(&message_with_whitespace)); + + unsafe { drop(Box::from_raw(message_ptr)) }; + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_with_newlines() { + let message_with_newlines = "line1\nline2\nline3".to_string(); + let message_ptr = Box::into_raw(Box::new(message_with_newlines)); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + let out = str::from_utf8(&buf).expect("to be valid UTF8"); + + assert!(out.contains("line1")); + assert!(out.contains("line2")); + assert!(out.contains("line3")); + + unsafe { drop(Box::from_raw(message_ptr)) }; + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_unicode() { + let unicode_message = "Hello δΈ–η•Œ πŸ¦€ Rust!".to_string(); + let message_ptr = Box::into_raw(Box::new(unicode_message.clone())); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + let out = str::from_utf8(&buf).expect("to be valid UTF8"); + + assert!(out.contains(&unicode_message)); + + unsafe { drop(Box::from_raw(message_ptr)) }; + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_emit_message_very_long() { + let long_message = "x".repeat(100000); // 100KB + let message_ptr = Box::into_raw(Box::new(long_message.clone())); + let mut buf = Vec::new(); + + emit_message(&mut buf, message_ptr).expect("to work"); + let out = str::from_utf8(&buf).expect("to be valid UTF8"); + + assert!(out.contains(&long_message[..100])); // At least first 100 chars + + unsafe { drop(Box::from_raw(message_ptr)) }; + } } diff --git a/libdd-crashtracker/src/crash_info/builder.rs b/libdd-crashtracker/src/crash_info/builder.rs index 3a087d181a..c4dc262384 100644 --- a/libdd-crashtracker/src/crash_info/builder.rs +++ b/libdd-crashtracker/src/crash_info/builder.rs @@ -438,4 +438,96 @@ mod tests { assert_eq!(crash_ping.metadata(), &metadata); assert!(crash_ping.message().contains("crash processing started")); } + + #[test] + fn test_with_message() { + let mut builder = CrashInfoBuilder::new(); + let test_message = "Test error message".to_string(); + + let result = builder.with_message(test_message.clone()); + assert!(result.is_ok()); + assert!(builder.has_message()); + + // Build and verify message is present + let sig_info = SigInfo::test_instance(42); + builder.with_sig_info(sig_info).unwrap(); + builder.with_metadata(Metadata::test_instance(1)).unwrap(); + + let crash_ping = builder.build_crash_ping().unwrap(); + assert!(crash_ping.message().contains(&test_message)); + } + + #[test] + fn test_has_message_empty() { + let builder = CrashInfoBuilder::new(); + assert!(!builder.has_message()); + } + + #[test] + fn test_has_message_after_setting() { + let mut builder = CrashInfoBuilder::new(); + builder.with_message("test".to_string()).unwrap(); + assert!(builder.has_message()); + } + + #[test] + fn test_message_overwrite() { + let mut builder = CrashInfoBuilder::new(); + + builder.with_message("first message".to_string()).unwrap(); + assert!(builder.has_message()); + + // Overwrite with second message + builder.with_message("second message".to_string()).unwrap(); + assert!(builder.has_message()); + + // Build and verify only second message is present + let sig_info = SigInfo::test_instance(42); + builder.with_sig_info(sig_info).unwrap(); + builder.with_metadata(Metadata::test_instance(1)).unwrap(); + + let crash_ping = builder.build_crash_ping().unwrap(); + assert!(crash_ping.message().contains("second message")); + assert!(!crash_ping.message().contains("first message")); + builder.with_kind(ErrorKind::Panic).unwrap(); + + let report = builder.build().unwrap(); + assert_eq!(report.error.message.as_deref(), Some("second message")); + } + + #[test] + fn test_message_with_special_characters() { + let mut builder = CrashInfoBuilder::new(); + let special_message = "Error: 'panic' with \"quotes\" and\nnewlines\t\ttabs"; + + builder.with_message(special_message.to_string()).unwrap(); + builder.with_sig_info(SigInfo::test_instance(42)).unwrap(); + builder.with_metadata(Metadata::test_instance(1)).unwrap(); + + let crash_ping = builder.build_crash_ping().unwrap(); + assert!(crash_ping.message().contains(special_message)); + builder.with_kind(ErrorKind::UnixSignal).unwrap(); + + let report = builder.build().unwrap(); + assert_eq!(report.error.message.as_deref(), Some(special_message)); + } + + #[test] + fn test_very_long_message() { + let mut builder = CrashInfoBuilder::new(); + let long_message = "x".repeat(10000); // 10KB message + + builder.with_message(long_message.clone()).unwrap(); + assert!(builder.has_message()); + + builder.with_sig_info(SigInfo::test_instance(42)).unwrap(); + builder.with_metadata(Metadata::test_instance(1)).unwrap(); + + let crash_ping = builder.build_crash_ping().unwrap(); + assert!(crash_ping.message().len() >= 10000); + + builder.with_kind(ErrorKind::UnixSignal).unwrap(); + let report = builder.build().unwrap(); + assert!(report.error.message.as_ref().unwrap().len() >= 10000); + } } diff --git a/libdd-crashtracker/src/receiver/receive_report.rs b/libdd-crashtracker/src/receiver/receive_report.rs index f40b7339f9..7fb82d7f9f 100644 --- a/libdd-crashtracker/src/receiver/receive_report.rs +++ b/libdd-crashtracker/src/receiver/receive_report.rs @@ -495,3 +495,115 @@ pub(crate) async fn receive_report_from_stream( Ok(Some((config, crash_info))) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_stdin_state_waiting_to_message() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + let state = StdinState::Waiting; + let line = DD_CRASHTRACK_BEGIN_MESSAGE; + + let next_state = process_line(&mut builder, &mut config, line, state).unwrap(); + + assert!(matches!(next_state, StdinState::Message)); + } + + #[test] + fn test_stdin_state_message_content() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + // Enter message state + let state = StdinState::Message; + let message_line = "program panicked"; + + let next_state = process_line(&mut builder, &mut config, message_line, state).unwrap(); + + // Should stay in message state + assert!(matches!(next_state, StdinState::Message)); + + // Verify message was stored + assert!(builder.has_message()); + } + + #[test] + fn test_stdin_state_message_to_waiting() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + let state = StdinState::Message; + let line = DD_CRASHTRACK_END_MESSAGE; + + let next_state = process_line(&mut builder, &mut config, line, state).unwrap(); + + assert!(matches!(next_state, StdinState::Waiting)); + } + + #[test] + fn test_message_state_with_empty_line() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + let state = StdinState::Message; + let empty_line = ""; + + let result = process_line(&mut builder, &mut config, empty_line, state); + + // Should handle empty line without error + assert!(result.is_ok()); + } + + #[test] + fn test_message_state_with_multiline_content() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + // First line of message + let state = process_line( + &mut builder, + &mut config, + "Line 1 of panic", + StdinState::Message, + ) + .unwrap(); + + // Should still be in message state + assert!(matches!(state, StdinState::Message)); + + // Note: Current implementation may only store last message + // This test documents current behavior + } + + #[test] + fn test_message_state_full_workflow() { + let mut builder = CrashInfoBuilder::new(); + let mut config = None; + + // Start in waiting state + let mut state = StdinState::Waiting; + + // Transition to message + state = process_line( + &mut builder, + &mut config, + DD_CRASHTRACK_BEGIN_MESSAGE, + state, + ) + .unwrap(); + assert!(matches!(state, StdinState::Message)); + + // Add message content + state = process_line(&mut builder, &mut config, "test panic message", state).unwrap(); + assert!(matches!(state, StdinState::Message)); + assert!(builder.has_message()); + + // End message + state = process_line(&mut builder, &mut config, DD_CRASHTRACK_END_MESSAGE, state).unwrap(); + assert!(matches!(state, StdinState::Waiting)); + } +} From 7ac033aa27397414bd4e66c4ec01d69062638a5b Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 25 Nov 2025 17:58:54 +0100 Subject: [PATCH 08/16] add bin_test to validate fork --- bin_tests/src/modes/behavior.rs | 1 + bin_tests/src/modes/unix/mod.rs | 1 + .../unix/test_013_panic_hook_after_fork.rs | 98 +++++++++++++++ bin_tests/tests/crashtracker_bin_test.rs | 118 +++++++++++++----- 4 files changed, 186 insertions(+), 32 deletions(-) create mode 100644 bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs diff --git a/bin_tests/src/modes/behavior.rs b/bin_tests/src/modes/behavior.rs index ce66270643..eb3ce0a5e3 100644 --- a/bin_tests/src/modes/behavior.rs +++ b/bin_tests/src/modes/behavior.rs @@ -134,6 +134,7 @@ pub fn get_behavior(mode_str: &str) -> Box { "runtime_callback_frame_invalid_utf8" => { Box::new(test_012_runtime_callback_frame_invalid_utf8::Test) } + "panic_hook_after_fork" => Box::new(test_013_panic_hook_after_fork::Test), _ => panic!("Unknown mode: {mode_str}"), } } diff --git a/bin_tests/src/modes/unix/mod.rs b/bin_tests/src/modes/unix/mod.rs index 394d693877..404cf96c89 100644 --- a/bin_tests/src/modes/unix/mod.rs +++ b/bin_tests/src/modes/unix/mod.rs @@ -13,3 +13,4 @@ pub mod test_009_prechain_with_abort; pub mod test_010_runtime_callback_frame; pub mod test_011_runtime_callback_string; pub mod test_012_runtime_callback_frame_invalid_utf8; +pub mod test_013_panic_hook_after_fork; diff --git a/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs new file mode 100644 index 0000000000..0a5e471ee8 --- /dev/null +++ b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs @@ -0,0 +1,98 @@ +// Copyright 2025-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 +// +// Test that panic hooks registered before fork() continue to work in child processes. +// This validates that: +// 1. The panic hook survives fork() +// 2. The panic message is captured in the child process +// 3. The crash report is correctly generated +use crate::modes::behavior::Behavior; +use libdd_crashtracker::{self as crashtracker, CrashtrackerConfiguration}; +use nix::sys::wait::{waitpid, WaitStatus}; +use nix::unistd::Pid; +use std::path::Path; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +pub struct Test; + +impl Behavior for Test { + fn setup( + &self, + _output_dir: &Path, + _config: &mut CrashtrackerConfiguration, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> { + Ok(()) + } + + fn post(&self, _output_dir: &Path) -> anyhow::Result<()> { + post() + } +} + +fn post() -> anyhow::Result<()> { + // Set up a panic hook to verify it gets called + let panic_hook_called = Arc::new(AtomicBool::new(false)); + let panic_hook_called_clone = Arc::clone(&panic_hook_called); + + std::panic::set_hook(Box::new(move |_panic_info| { + panic_hook_called_clone.store(true, Ordering::SeqCst); + })); + + match unsafe { libc::fork() } { + -1 => { + anyhow::bail!("Failed to fork"); + } + 0 => { + // Child - panic with a specific message + // The crashtracker should capture both the panic hook execution + // and the panic message + crashtracker::begin_op(crashtracker::OpTypes::ProfilerCollectingSample)?; + + // Give parent time to set up wait + std::thread::sleep(Duration::from_millis(10)); + + panic!("child panicked after fork - hook should fire"); + } + pid => { + // Parent - wait for child to panic and crash + let start_time = Instant::now(); + let max_wait = Duration::from_secs(5); + + loop { + match waitpid(Pid::from_raw(pid), None)? { + WaitStatus::StillAlive => { + if start_time.elapsed() > max_wait { + anyhow::bail!("Child process did not exit within 5 seconds"); + } + std::thread::sleep(Duration::from_millis(10)); + } + WaitStatus::Exited(_pid, exit_code) => { + // Child exited - this is what we expect after panic + eprintln!("Child exited with code: {}", exit_code); + break; + } + WaitStatus::Signaled(_pid, signal, _) => { + // Child was killed by signal (also acceptable for panic) + eprintln!("Child killed by signal: {:?}", signal); + break; + } + _ => { + // Other status - continue waiting + } + } + } + + // Parent exits with error code to indicate test completion + // The test harness will verify the crash report contains the panic message + unsafe { + libc::_exit(1); + } + } + } +} diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 3504a88201..08c89f9ed8 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -314,22 +314,8 @@ fn test_crash_tracking_app(crash_type: &str) { use bin_tests::test_runner::run_custom_crash_test; // Set up custom artifacts: receiver + crashing_test_app with panic_abort - let crashtracker_receiver = ArtifactsBuild { - name: "test_crashtracker_receiver".to_owned(), - build_profile: BuildProfile::Release, - artifact_type: ArtifactType::Bin, - triple_target: None, - ..Default::default() - }; - - let crashing_app = ArtifactsBuild { - name: "crashing_test_app".to_owned(), - build_profile: BuildProfile::Debug, - artifact_type: ArtifactType::Bin, - triple_target: None, - panic_abort: Some(true), - ..Default::default() - }; + let crashtracker_receiver = create_crashtracker_receiver(BuildProfile::Release); + let crashing_app = create_crashing_app(BuildProfile::Debug, true); let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashing_app]).unwrap(); @@ -371,6 +357,54 @@ fn test_crash_tracking_app(crash_type: &str) { .unwrap(); } +#[test] +#[cfg_attr(miri, ignore)] +#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests +fn test_crash_tracking_bin_panic_hook_after_fork() { + use bin_tests::test_runner::run_custom_crash_test; + + // Set up custom artifacts: receiver + crashtracker_bin_test + let crashtracker_receiver = create_crashtracker_receiver(BuildProfile::Release); + let crashtracker_bin_test = create_crashtracker_bin_test(BuildProfile::Debug); + + let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashtracker_bin_test]).unwrap(); + + let validator: ValidatorFn = Box::new(|payload, _fixtures| { + // Verify the panic message is captured + let error = &payload["error"]; + let message = error["message"].as_str().unwrap(); + assert!( + message.contains("child panicked after fork"), + "Expected panic message to contain 'child panicked after fork', got: {}", + message + ); + + // Verify it's marked as a panic + let kind = error["kind"].as_str().unwrap(); + assert_eq!( + kind, "Panic", + "Expected error kind to be Panic, got: {}", + kind + ); + + Ok(()) + }); + + run_custom_crash_test( + &artifacts_map[&crashtracker_bin_test], + |cmd, fixtures| { + cmd.arg(format!("file://{}", fixtures.crash_profile_path.display())) + .arg(&artifacts_map[&crashtracker_receiver]) + .arg(&fixtures.output_dir) + .arg("panic_hook_after_fork") // mode + .arg("donothing"); // crash method (not used in this mode) + }, + false, // expect crash (not success) + validator, + ) + .unwrap(); +} + // ==================================================================================== // CALLSTACK VALIDATION TESTS - MIGRATED TO CUSTOM TEST RUNNER // ==================================================================================== @@ -390,22 +424,9 @@ fn test_crash_tracking_callstack() { use bin_tests::test_runner::run_custom_crash_test; // Set up custom artifacts: receiver + crashing_test_app (in Debug mode) - let crashtracker_receiver = ArtifactsBuild { - name: "test_crashtracker_receiver".to_owned(), - build_profile: BuildProfile::Release, - artifact_type: ArtifactType::Bin, - triple_target: None, - ..Default::default() - }; - - let crashing_app = ArtifactsBuild { - name: "crashing_test_app".to_owned(), - // compile in debug so we avoid inlining and can check the callchain - build_profile: BuildProfile::Debug, - artifact_type: ArtifactType::Bin, - triple_target: None, - ..Default::default() - }; + let crashtracker_receiver = create_crashtracker_receiver(BuildProfile::Release); + // compile in debug so we avoid inlining and can check the callchain + let crashing_app = create_crashing_app(BuildProfile::Debug, false); let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashing_app]).unwrap(); @@ -1263,6 +1284,39 @@ fn setup_crashtracking_crates( (crashtracker_bin, crashtracker_receiver) } +// Helper functions for creating common artifact configurations + +fn create_crashtracker_receiver(profile: BuildProfile) -> ArtifactsBuild { + ArtifactsBuild { + name: "test_crashtracker_receiver".to_owned(), + build_profile: profile, + artifact_type: ArtifactType::Bin, + triple_target: None, + ..Default::default() + } +} + +fn create_crashing_app(profile: BuildProfile, panic_abort: bool) -> ArtifactsBuild { + ArtifactsBuild { + name: "crashing_test_app".to_owned(), + build_profile: profile, + artifact_type: ArtifactType::Bin, + triple_target: None, + panic_abort: if panic_abort { Some(true) } else { None }, + ..Default::default() + } +} + +fn create_crashtracker_bin_test(profile: BuildProfile) -> ArtifactsBuild { + ArtifactsBuild { + name: "crashtracker_bin_test".to_owned(), + build_profile: profile, + artifact_type: ArtifactType::Bin, + triple_target: None, + ..Default::default() + } +} + #[cfg(unix)] fn test_crash_tracking_bin_with_errors_intake_uds( crash_tracking_receiver_profile: BuildProfile, From abc26d3e7252d213b4d92c40d2081e9c4b53221c Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 10:16:31 +0100 Subject: [PATCH 09/16] Fix fork test --- .../unix/test_013_panic_hook_after_fork.rs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs index 0a5e471ee8..4b051897f1 100644 --- a/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs +++ b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs @@ -12,9 +12,11 @@ use nix::sys::wait::{waitpid, WaitStatus}; use nix::unistd::Pid; use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; use std::time::{Duration, Instant}; +// Shared state to track if the custom panic hook was called +static PANIC_HOOK_CALLED: AtomicBool = AtomicBool::new(false); + pub struct Test; impl Behavior for Test { @@ -27,7 +29,7 @@ impl Behavior for Test { } fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> { - Ok(()) + pre() } fn post(&self, _output_dir: &Path) -> anyhow::Result<()> { @@ -35,15 +37,23 @@ impl Behavior for Test { } } -fn post() -> anyhow::Result<()> { - // Set up a panic hook to verify it gets called - let panic_hook_called = Arc::new(AtomicBool::new(false)); - let panic_hook_called_clone = Arc::clone(&panic_hook_called); +fn pre() -> anyhow::Result<()> { + // Reset the flag in case the test runs multiple times + PANIC_HOOK_CALLED.store(false, Ordering::SeqCst); - std::panic::set_hook(Box::new(move |_panic_info| { - panic_hook_called_clone.store(true, Ordering::SeqCst); + let old_hook = std::panic::take_hook(); + // Set up a panic hook BEFORE crashtracker::init to verify the hook chain works + std::panic::set_hook(Box::new(move |panic_info| { + // Mark that our custom hook was called + PANIC_HOOK_CALLED.store(true, Ordering::SeqCst); + // Call the previous hook (usually the default panic hook) + old_hook(panic_info); })); + Ok(()) +} + +fn post() -> anyhow::Result<()> { match unsafe { libc::fork() } { -1 => { anyhow::bail!("Failed to fork"); @@ -88,6 +98,13 @@ fn post() -> anyhow::Result<()> { } } + // Verify that our custom panic hook was called + // This proves that the hook chain works correctly: + // crashtracker's hook -> our custom hook -> default hook + if !PANIC_HOOK_CALLED.load(Ordering::SeqCst) { + anyhow::bail!("Custom panic hook was not called - hook chaining failed!"); + } + // Parent exits with error code to indicate test completion // The test harness will verify the crash report contains the panic message unsafe { From c2aceb4afa0e9b856f04998217b6da169612ce1f Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 10:38:55 +0100 Subject: [PATCH 10/16] Make sure we panic/abort for fork test --- bin_tests/tests/crashtracker_bin_test.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 08c89f9ed8..a18954492b 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -365,7 +365,7 @@ fn test_crash_tracking_bin_panic_hook_after_fork() { // Set up custom artifacts: receiver + crashtracker_bin_test let crashtracker_receiver = create_crashtracker_receiver(BuildProfile::Release); - let crashtracker_bin_test = create_crashtracker_bin_test(BuildProfile::Debug); + let crashtracker_bin_test = create_crashtracker_bin_test(BuildProfile::Debug, true); let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashtracker_bin_test]).unwrap(); @@ -1267,20 +1267,8 @@ fn setup_test_fixtures<'a>(crates: &[&'a ArtifactsBuild]) -> TestFixtures<'a> { fn setup_crashtracking_crates( crash_tracking_receiver_profile: BuildProfile, ) -> (ArtifactsBuild, ArtifactsBuild) { - let crashtracker_bin = ArtifactsBuild { - name: "crashtracker_bin_test".to_owned(), - build_profile: crash_tracking_receiver_profile, - artifact_type: ArtifactType::Bin, - triple_target: None, - ..Default::default() - }; - let crashtracker_receiver = ArtifactsBuild { - name: "test_crashtracker_receiver".to_owned(), - build_profile: crash_tracking_receiver_profile, - artifact_type: ArtifactType::Bin, - triple_target: None, - ..Default::default() - }; + let crashtracker_bin = create_crashtracker_bin_test(crash_tracking_receiver_profile, false); + let crashtracker_receiver = create_crashtracker_receiver(crash_tracking_receiver_profile); (crashtracker_bin, crashtracker_receiver) } @@ -1307,12 +1295,13 @@ fn create_crashing_app(profile: BuildProfile, panic_abort: bool) -> ArtifactsBui } } -fn create_crashtracker_bin_test(profile: BuildProfile) -> ArtifactsBuild { +fn create_crashtracker_bin_test(profile: BuildProfile, panic_abort: bool) -> ArtifactsBuild { ArtifactsBuild { name: "crashtracker_bin_test".to_owned(), build_profile: profile, artifact_type: ArtifactType::Bin, triple_target: None, + panic_abort: if panic_abort { Some(true) } else { None }, ..Default::default() } } From f583b237925453c5093d3decf5c4bbf255a85c90 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 11:06:44 +0100 Subject: [PATCH 11/16] Fix ci --- .../unix/test_013_panic_hook_after_fork.rs | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs index 4b051897f1..e0366b838a 100644 --- a/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs +++ b/bin_tests/src/modes/unix/test_013_panic_hook_after_fork.rs @@ -10,13 +10,10 @@ use crate::modes::behavior::Behavior; use libdd_crashtracker::{self as crashtracker, CrashtrackerConfiguration}; use nix::sys::wait::{waitpid, WaitStatus}; use nix::unistd::Pid; +use std::fs; use std::path::Path; -use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; -// Shared state to track if the custom panic hook was called -static PANIC_HOOK_CALLED: AtomicBool = AtomicBool::new(false); - pub struct Test; impl Behavior for Test { @@ -28,24 +25,26 @@ impl Behavior for Test { Ok(()) } - fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> { - pre() + fn pre(&self, output_dir: &Path) -> anyhow::Result<()> { + pre(output_dir) } - fn post(&self, _output_dir: &Path) -> anyhow::Result<()> { - post() + fn post(&self, output_dir: &Path) -> anyhow::Result<()> { + post(output_dir) } } -fn pre() -> anyhow::Result<()> { - // Reset the flag in case the test runs multiple times - PANIC_HOOK_CALLED.store(false, Ordering::SeqCst); - +fn pre(output_dir: &Path) -> anyhow::Result<()> { let old_hook = std::panic::take_hook(); + let output_dir = output_dir.to_path_buf(); + // Set up a panic hook BEFORE crashtracker::init to verify the hook chain works std::panic::set_hook(Box::new(move |panic_info| { - // Mark that our custom hook was called - PANIC_HOOK_CALLED.store(true, Ordering::SeqCst); + // Mark that our custom hook was called by writing a marker file + // This works across fork() because it's persistent storage + let marker_path = output_dir.join("panic_hook_called.marker"); + let _ = fs::write(marker_path, "hook was called"); + // Call the previous hook (usually the default panic hook) old_hook(panic_info); })); @@ -53,7 +52,7 @@ fn pre() -> anyhow::Result<()> { Ok(()) } -fn post() -> anyhow::Result<()> { +fn post(output_dir: &Path) -> anyhow::Result<()> { match unsafe { libc::fork() } { -1 => { anyhow::bail!("Failed to fork"); @@ -98,11 +97,17 @@ fn post() -> anyhow::Result<()> { } } - // Verify that our custom panic hook was called + // Verify that our custom panic hook was called by checking for the marker file // This proves that the hook chain works correctly: // crashtracker's hook -> our custom hook -> default hook - if !PANIC_HOOK_CALLED.load(Ordering::SeqCst) { - anyhow::bail!("Custom panic hook was not called - hook chaining failed!"); + let marker_path = output_dir.join("panic_hook_called.marker"); + + if !marker_path.exists() { + anyhow::bail!( + "Custom panic hook was not called - hook chaining failed! \ + Expected marker file at: {}", + marker_path.display() + ); } // Parent exits with error code to indicate test completion From bb5c3b9e9b6cd1c43982b7fdd7ce52a8dd719552 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 14:57:03 +0100 Subject: [PATCH 12/16] Fix test --- bin_tests/src/test_runner.rs | 15 ++++++--------- bin_tests/tests/crashtracker_bin_test.rs | 6 +++--- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/bin_tests/src/test_runner.rs b/bin_tests/src/test_runner.rs index 8a179551c1..1f20dcb53b 100644 --- a/bin_tests/src/test_runner.rs +++ b/bin_tests/src/test_runner.rs @@ -190,7 +190,9 @@ where /// This is more flexible than `run_crash_test_with_artifacts` and allows for: /// - Custom binary selection (e.g., crashing_test_app instead of crashtracker_bin_test) /// - Custom command arguments -/// - Custom exit status expectations +/// +/// Note: This function always expects the test to crash (exit with non-success status). +/// All current uses of this function test crash scenarios, not successful exits. /// /// # Example /// ```no_run @@ -223,7 +225,6 @@ where /// .arg(&artifacts_map[&receiver]) /// .arg(&fixtures.output_dir); /// }, -/// false, // expect crash (not success) /// |payload, _fixtures| { /// // Custom validation /// Ok(()) @@ -235,7 +236,6 @@ where pub fn run_custom_crash_test( binary_path: &std::path::Path, command_builder: CB, - expect_success: bool, validator: V, ) -> Result<()> where @@ -251,13 +251,10 @@ where let exit_status = crate::timeit!("exit after signal", { p.wait()? }); - // Validate exit status - let actual_success = exit_status.success(); + // Validate exit status - custom crash tests always expect failure anyhow::ensure!( - expect_success == actual_success, - "Exit status mismatch: expected success={}, got success={} (exit code: {:?})", - expect_success, - actual_success, + !exit_status.success(), + "Expected test to crash (non-success exit), but it succeeded with code: {:?}", exit_status.code() ); diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index a18954492b..0f91c7721b 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -379,11 +379,11 @@ fn test_crash_tracking_bin_panic_hook_after_fork() { message ); - // Verify it's marked as a panic + // TODO change the kind into Panic instead let kind = error["kind"].as_str().unwrap(); assert_eq!( - kind, "Panic", - "Expected error kind to be Panic, got: {}", + kind, "UnixSignal", + "Expected error kind to be UnixSignal, got: {}", kind ); From 73050cd9ddf226087426ed0ecdd4e8d1661f03a8 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 15:05:39 +0100 Subject: [PATCH 13/16] Fix test --- bin_tests/src/bin/crashing_test_app.rs | 4 +++- bin_tests/tests/crashtracker_bin_test.rs | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin_tests/src/bin/crashing_test_app.rs b/bin_tests/src/bin/crashing_test_app.rs index 4630082635..6789d51bc6 100644 --- a/bin_tests/src/bin/crashing_test_app.rs +++ b/bin_tests/src/bin/crashing_test_app.rs @@ -124,10 +124,12 @@ mod unix { let is_panic_mode = matches!(crash_type, CrashType::Panic); let called_panic_hook = Arc::new(AtomicBool::new(false)); + let old_hook = std::panic::take_hook(); if is_panic_mode { let called_panic_hook_clone = Arc::clone(&called_panic_hook); - std::panic::set_hook(Box::new(move |_| { + std::panic::set_hook(Box::new(move |panic_info| { called_panic_hook_clone.store(true, Ordering::SeqCst); + old_hook(panic_info); })); } diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 0f91c7721b..dddce30da2 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -351,7 +351,6 @@ fn test_crash_tracking_app(crash_type: &str) { .arg(&fixtures.output_dir) .arg(crash_type); }, - false, // expect crash (not success) validator, ) .unwrap(); @@ -399,7 +398,6 @@ fn test_crash_tracking_bin_panic_hook_after_fork() { .arg("panic_hook_after_fork") // mode .arg("donothing"); // crash method (not used in this mode) }, - false, // expect crash (not success) validator, ) .unwrap(); @@ -445,9 +443,9 @@ fn test_crash_tracking_callstack() { |cmd, fixtures| { cmd.arg(format!("file://{}", fixtures.crash_profile_path.display())) .arg(&artifacts_map[&crashtracker_receiver]) - .arg(&fixtures.output_dir); + .arg(&fixtures.output_dir) + .arg("segfault"); }, - false, // expect crash (not success) |payload, _fixtures| { // Use the new callstack validator PayloadValidator::new(payload).validate_callstack_functions(&expected_functions)?; From e65e6be25f74c725da7b3de4257b5f21b4b6d2d3 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Tue, 9 Dec 2025 21:40:24 +0100 Subject: [PATCH 14/16] Make sure we can handle different type of panic payload --- bin_tests/src/modes/behavior.rs | 2 + bin_tests/src/modes/unix/mod.rs | 2 + .../modes/unix/test_014_panic_hook_string.rs | 31 +++++++++++ .../unix/test_015_panic_hook_unknown_type.rs | 30 ++++++++++ bin_tests/tests/crashtracker_bin_test.rs | 55 +++++++++++++------ .../src/collector/crash_handler.rs | 48 +++++++++++++--- 6 files changed, 143 insertions(+), 25 deletions(-) create mode 100644 bin_tests/src/modes/unix/test_014_panic_hook_string.rs create mode 100644 bin_tests/src/modes/unix/test_015_panic_hook_unknown_type.rs diff --git a/bin_tests/src/modes/behavior.rs b/bin_tests/src/modes/behavior.rs index eb3ce0a5e3..c5bdc51102 100644 --- a/bin_tests/src/modes/behavior.rs +++ b/bin_tests/src/modes/behavior.rs @@ -135,6 +135,8 @@ pub fn get_behavior(mode_str: &str) -> Box { Box::new(test_012_runtime_callback_frame_invalid_utf8::Test) } "panic_hook_after_fork" => Box::new(test_013_panic_hook_after_fork::Test), + "panic_hook_string" => Box::new(test_014_panic_hook_string::Test), + "panic_hook_unknown_type" => Box::new(test_015_panic_hook_unknown_type::Test), _ => panic!("Unknown mode: {mode_str}"), } } diff --git a/bin_tests/src/modes/unix/mod.rs b/bin_tests/src/modes/unix/mod.rs index 404cf96c89..2b8c2b0f2d 100644 --- a/bin_tests/src/modes/unix/mod.rs +++ b/bin_tests/src/modes/unix/mod.rs @@ -14,3 +14,5 @@ pub mod test_010_runtime_callback_frame; pub mod test_011_runtime_callback_string; pub mod test_012_runtime_callback_frame_invalid_utf8; pub mod test_013_panic_hook_after_fork; +pub mod test_014_panic_hook_string; +pub mod test_015_panic_hook_unknown_type; diff --git a/bin_tests/src/modes/unix/test_014_panic_hook_string.rs b/bin_tests/src/modes/unix/test_014_panic_hook_string.rs new file mode 100644 index 0000000000..f0b34bbc24 --- /dev/null +++ b/bin_tests/src/modes/unix/test_014_panic_hook_string.rs @@ -0,0 +1,31 @@ +// Copyright 2025-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 +// +// Test that panic hooks work correctly with String payloads (not just &str). +// This validates that: +// 1. String panic payloads are correctly captured +// 2. The panic message format is "Process panicked with message: " +use crate::modes::behavior::Behavior; +use libdd_crashtracker::CrashtrackerConfiguration; +use std::path::Path; + +pub struct Test; + +impl Behavior for Test { + fn setup( + &self, + _output_dir: &Path, + _config: &mut CrashtrackerConfiguration, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> { + Ok(()) + } + + fn post(&self, _output_dir: &Path) -> anyhow::Result<()> { + let dynamic_value = 42; + panic!("Panic with value: {}", dynamic_value); + } +} diff --git a/bin_tests/src/modes/unix/test_015_panic_hook_unknown_type.rs b/bin_tests/src/modes/unix/test_015_panic_hook_unknown_type.rs new file mode 100644 index 0000000000..74d8504ca5 --- /dev/null +++ b/bin_tests/src/modes/unix/test_015_panic_hook_unknown_type.rs @@ -0,0 +1,30 @@ +// Copyright 2025-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 +// +// Test that panic hooks work correctly with unknown types via panic_any. +// This validates that: +// 1. panic_any() with non-string types is handled gracefully +// 2. The message format is: "Process panicked with unknown type ()" +use crate::modes::behavior::Behavior; +use libdd_crashtracker::CrashtrackerConfiguration; +use std::path::Path; + +pub struct Test; + +impl Behavior for Test { + fn setup( + &self, + _output_dir: &Path, + _config: &mut CrashtrackerConfiguration, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> { + Ok(()) + } + + fn post(&self, _output_dir: &Path) -> anyhow::Result<()> { + std::panic::panic_any(42i32); + } +} diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index dddce30da2..1616aeb562 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -327,11 +327,11 @@ fn test_crash_tracking_app(crash_type: &str) { match crash_type_owned.as_str() { "panic" => { - let expected_message = "program panicked"; - assert_eq!( - error["message"].as_str().unwrap(), - expected_message, - "Expected panic message for panic crash type" + let message = error["message"].as_str().unwrap(); + assert!( + message.contains("Process panicked with message") && message.contains("program panicked"), + "Expected panic message to contain 'Process panicked with message' and 'program panicked', got: {}", + message ); } "segfault" => { @@ -360,6 +360,31 @@ fn test_crash_tracking_app(crash_type: &str) { #[cfg_attr(miri, ignore)] #[cfg(not(target_os = "macos"))] // Same restriction as other panic tests fn test_crash_tracking_bin_panic_hook_after_fork() { + test_panic_hook_mode("panic_hook_after_fork", "child panicked after fork"); +} + +#[test] +#[cfg_attr(miri, ignore)] +#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests +fn test_crash_tracking_bin_panic_hook_string() { + test_panic_hook_mode( + "panic_hook_string", + "Process panicked with message: Panic with value: 42", + ); +} + +#[test] +#[cfg_attr(miri, ignore)] +#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests +fn test_crash_tracking_bin_panic_hook_unknown_type() { + test_panic_hook_mode( + "panic_hook_unknown_type", + "Process panicked with unknown type", + ); +} + +/// Helper function to run panic hook tests with different payload types +fn test_panic_hook_mode(mode: &str, expected_message_substring: &str) { use bin_tests::test_runner::run_custom_crash_test; // Set up custom artifacts: receiver + crashtracker_bin_test @@ -368,24 +393,18 @@ fn test_crash_tracking_bin_panic_hook_after_fork() { let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashtracker_bin_test]).unwrap(); - let validator: ValidatorFn = Box::new(|payload, _fixtures| { + let expected_msg = expected_message_substring.to_owned(); + let validator: ValidatorFn = Box::new(move |payload, _fixtures| { // Verify the panic message is captured let error = &payload["error"]; let message = error["message"].as_str().unwrap(); assert!( - message.contains("child panicked after fork"), - "Expected panic message to contain 'child panicked after fork', got: {}", + message.contains(&expected_msg), + "Expected panic message to contain '{}', got: {}", + expected_msg, message ); - // TODO change the kind into Panic instead - let kind = error["kind"].as_str().unwrap(); - assert_eq!( - kind, "UnixSignal", - "Expected error kind to be UnixSignal, got: {}", - kind - ); - Ok(()) }); @@ -395,8 +414,8 @@ fn test_crash_tracking_bin_panic_hook_after_fork() { cmd.arg(format!("file://{}", fixtures.crash_profile_path.display())) .arg(&artifacts_map[&crashtracker_receiver]) .arg(&fixtures.output_dir) - .arg("panic_hook_after_fork") // mode - .arg("donothing"); // crash method (not used in this mode) + .arg(mode) + .arg("donothing"); // crash method (not used in panic hook tests) }, validator, ) diff --git a/libdd-crashtracker/src/collector/crash_handler.rs b/libdd-crashtracker/src/collector/crash_handler.rs index 8254187957..0ad3d90503 100644 --- a/libdd-crashtracker/src/collector/crash_handler.rs +++ b/libdd-crashtracker/src/collector/crash_handler.rs @@ -78,6 +78,30 @@ pub fn update_metadata(metadata: Metadata) -> anyhow::Result<()> { Ok(()) } +/// Format a panic message with optional location information. +fn format_panic_message( + category: &str, + description: &str, + location: Option<&panic::Location>, +) -> String { + let base = match location { + Some(loc) => format!( + "Process panicked with {} ({}:{}:{})", + category, + loc.file(), + loc.line(), + loc.column() + ), + None => format!("Process panicked with {}", category), + }; + + if description.is_empty() { + base + } else { + format!("{}: {}", base, description) + } +} + /// Register the panic hook. /// /// This function is used to register the panic hook and store the previous hook. @@ -98,15 +122,25 @@ pub fn register_panic_hook() -> anyhow::Result<()> { let old_hook_ptr = Box::into_raw(Box::new(old_hook)); PREVIOUS_PANIC_HOOK.swap(old_hook_ptr, SeqCst); panic::set_hook(Box::new(|panic_info| { - if let Some(s) = panic_info.payload().downcast_ref::<&str>() { - let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst); - // message_ptr should be null, but just in case. - if !message_ptr.is_null() { - unsafe { - std::mem::drop(Box::from_raw(message_ptr)); - } + // Extract panic message from payload (supports &str and String) + let message = if let Some(&s) = panic_info.payload().downcast_ref::<&str>() { + format_panic_message("message", s, panic_info.location()) + } else if let Some(s) = panic_info.payload().downcast_ref::() { + format_panic_message("message", s.as_str(), panic_info.location()) + } else { + // For non-string types, use a generic message + format_panic_message("unknown type", "", panic_info.location()) + }; + + // Store the message, cleaning up any old message + let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(message)), SeqCst); + // message_ptr should be null, but just in case. + if !message_ptr.is_null() { + unsafe { + std::mem::drop(Box::from_raw(message_ptr)); } } + call_previous_panic_hook(panic_info); })); Ok(()) From a863bacbee3f7d94338ef5b8fe3d8a1d4a048ff4 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Wed, 10 Dec 2025 09:09:51 +0100 Subject: [PATCH 15/16] Add unit tests + fix integration tests --- Cargo.lock | 1 + bin_tests/Cargo.toml | 1 + bin_tests/tests/crashtracker_bin_test.rs | 50 ++++++++--- .../src/collector/crash_handler.rs | 84 +++++++++++++++---- 4 files changed, 106 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f48205594a..c5866a0c68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -367,6 +367,7 @@ dependencies = [ "nix 0.29.0", "once_cell", "os_info", + "regex", "serde_json", "serial_test", "strum", diff --git a/bin_tests/Cargo.toml b/bin_tests/Cargo.toml index 86b745883b..7e5e9c3c45 100644 --- a/bin_tests/Cargo.toml +++ b/bin_tests/Cargo.toml @@ -23,6 +23,7 @@ libc = "0.2" nix = { version = "0.29", features = ["signal", "socket"] } hex = "0.4" os_info = "3.7.0" +regex = "1.0" [dev-dependencies] serial_test = "3.2" diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 1616aeb562..2c1da53c1c 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -360,17 +360,18 @@ fn test_crash_tracking_app(crash_type: &str) { #[cfg_attr(miri, ignore)] #[cfg(not(target_os = "macos"))] // Same restriction as other panic tests fn test_crash_tracking_bin_panic_hook_after_fork() { - test_panic_hook_mode("panic_hook_after_fork", "child panicked after fork"); + test_panic_hook_mode( + "panic_hook_after_fork", + "message", + Some("child panicked after fork"), + ); } #[test] #[cfg_attr(miri, ignore)] #[cfg(not(target_os = "macos"))] // Same restriction as other panic tests fn test_crash_tracking_bin_panic_hook_string() { - test_panic_hook_mode( - "panic_hook_string", - "Process panicked with message: Panic with value: 42", - ); + test_panic_hook_mode("panic_hook_string", "message", Some("Panic with value: 42")); } #[test] @@ -379,12 +380,14 @@ fn test_crash_tracking_bin_panic_hook_string() { fn test_crash_tracking_bin_panic_hook_unknown_type() { test_panic_hook_mode( "panic_hook_unknown_type", - "Process panicked with unknown type", + "unknown type", + None, // no panic message for unknown type ); } -/// Helper function to run panic hook tests with different payload types -fn test_panic_hook_mode(mode: &str, expected_message_substring: &str) { +/// Helper function to run panic hook tests with different payload types. +/// Note: Since tests are built with Debug profile, location is always expected. +fn test_panic_hook_mode(mode: &str, expected_category: &str, expected_panic_message: Option<&str>) { use bin_tests::test_runner::run_custom_crash_test; // Set up custom artifacts: receiver + crashtracker_bin_test @@ -393,15 +396,38 @@ fn test_panic_hook_mode(mode: &str, expected_message_substring: &str) { let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashtracker_bin_test]).unwrap(); - let expected_msg = expected_message_substring.to_owned(); + let expected_category = expected_category.to_owned(); + let expected_panic_message = expected_panic_message.map(|s| s.to_owned()); let validator: ValidatorFn = Box::new(move |payload, _fixtures| { // Verify the panic message is captured let error = &payload["error"]; let message = error["message"].as_str().unwrap(); + + // Check the message starts with "Process panicked with " + let expected_prefix = format!("Process panicked with {}", expected_category); + assert!( + message.starts_with(&expected_prefix), + "Expected panic message to start with '{}', got: {}", + expected_prefix, + message + ); + + // Check the panic message if expected (the message passed to panic! macro) + if let Some(ref panic_msg) = expected_panic_message { + assert!( + message.contains(panic_msg), + "Expected panic message to contain '{}', got: {}", + panic_msg, + message + ); + } + + // Check for location format (file:line:column) - always present in Debug builds + // Location should end with pattern like " (path/file.rs:123:45)" + let location_regex = regex::Regex::new(r" \(.+?:\d+:\d+\)$").unwrap(); assert!( - message.contains(&expected_msg), - "Expected panic message to contain '{}', got: {}", - expected_msg, + location_regex.is_match(message), + "Expected panic message to end with location ' (file:line:column)', got: {}", message ); diff --git a/libdd-crashtracker/src/collector/crash_handler.rs b/libdd-crashtracker/src/collector/crash_handler.rs index 0ad3d90503..413b78224e 100644 --- a/libdd-crashtracker/src/collector/crash_handler.rs +++ b/libdd-crashtracker/src/collector/crash_handler.rs @@ -79,26 +79,20 @@ pub fn update_metadata(metadata: Metadata) -> anyhow::Result<()> { } /// Format a panic message with optional location information. -fn format_panic_message( +fn format_message( category: &str, - description: &str, + panic_message: &str, location: Option<&panic::Location>, ) -> String { - let base = match location { - Some(loc) => format!( - "Process panicked with {} ({}:{}:{})", - category, - loc.file(), - loc.line(), - loc.column() - ), - None => format!("Process panicked with {}", category), + let base = if panic_message.is_empty() { + format!("Process panicked with {}", category) + } else { + format!("Process panicked with {} \"{}\"", category, panic_message) }; - if description.is_empty() { - base - } else { - format!("{}: {}", base, description) + match location { + Some(loc) => format!("{} ({}:{}:{})", base, loc.file(), loc.line(), loc.column()), + None => base, } } @@ -124,12 +118,12 @@ pub fn register_panic_hook() -> anyhow::Result<()> { panic::set_hook(Box::new(|panic_info| { // Extract panic message from payload (supports &str and String) let message = if let Some(&s) = panic_info.payload().downcast_ref::<&str>() { - format_panic_message("message", s, panic_info.location()) + format_message("message", s, panic_info.location()) } else if let Some(s) = panic_info.payload().downcast_ref::() { - format_panic_message("message", s.as_str(), panic_info.location()) + format_message("message", s.as_str(), panic_info.location()) } else { // For non-string types, use a generic message - format_panic_message("unknown type", "", panic_info.location()) + format_message("unknown type", "", panic_info.location()) }; // Store the message, cleaning up any old message @@ -394,4 +388,58 @@ mod tests { assert_eq!(stored_metadata.library_name, "test"); } } + + #[test] + fn test_format_message_with_message_and_location() { + let location = panic::Location::caller(); + let result = format_message("message", "test panic", Some(location)); + + assert!(result.starts_with("Process panicked with message \"test panic\" (")); + assert!(result.contains(&format!("{}:", location.file()))); + assert!(result.contains(&format!(":{}", location.line()))); + assert!(result.ends_with(&format!("{})", location.column()))); + } + + #[test] + fn test_format_message_with_message_no_location() { + let result = format_message("message", "test panic", None); + assert_eq!(result, "Process panicked with message \"test panic\""); + } + + #[test] + fn test_format_message_empty_message_with_location() { + let location = panic::Location::caller(); + let result = format_message("unknown type", "", Some(location)); + + assert!(result.starts_with("Process panicked with unknown type (")); + assert!(result.contains(&format!("{}:", location.file()))); + assert!(result.ends_with(&format!("{})", location.column()))); + } + + #[test] + fn test_format_message_empty_message_no_location() { + let result = format_message("unknown type", "", None); + assert_eq!(result, "Process panicked with unknown type"); + } + + #[test] + fn test_format_message_different_categories() { + let result1 = format_message("message", "test", None); + assert_eq!(result1, "Process panicked with message \"test\""); + + let result2 = format_message("unknown type", "", None); + assert_eq!(result2, "Process panicked with unknown type"); + + let result3 = format_message("custom category", "content", None); + assert_eq!(result3, "Process panicked with custom category \"content\""); + } + + #[test] + fn test_format_message_with_special_characters() { + let result = format_message("message", "test \"quoted\" 'text'", None); + assert_eq!( + result, + "Process panicked with message \"test \"quoted\" 'text'\"" + ); + } } From 8361d3b3e2b6e89a879e14e369a9408b97a2caef Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Mon, 29 Dec 2025 14:12:45 +0100 Subject: [PATCH 16/16] Update from main --- .../src/receiver/receive_report.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/libdd-crashtracker/src/receiver/receive_report.rs b/libdd-crashtracker/src/receiver/receive_report.rs index 7fb82d7f9f..f1980a90b6 100644 --- a/libdd-crashtracker/src/receiver/receive_report.rs +++ b/libdd-crashtracker/src/receiver/receive_report.rs @@ -508,7 +508,7 @@ mod tests { let state = StdinState::Waiting; let line = DD_CRASHTRACK_BEGIN_MESSAGE; - let next_state = process_line(&mut builder, &mut config, line, state).unwrap(); + let next_state = process_line(&mut builder, &mut config, line, state, &None).unwrap(); assert!(matches!(next_state, StdinState::Message)); } @@ -522,7 +522,8 @@ mod tests { let state = StdinState::Message; let message_line = "program panicked"; - let next_state = process_line(&mut builder, &mut config, message_line, state).unwrap(); + let next_state = + process_line(&mut builder, &mut config, message_line, state, &None).unwrap(); // Should stay in message state assert!(matches!(next_state, StdinState::Message)); @@ -539,7 +540,7 @@ mod tests { let state = StdinState::Message; let line = DD_CRASHTRACK_END_MESSAGE; - let next_state = process_line(&mut builder, &mut config, line, state).unwrap(); + let next_state = process_line(&mut builder, &mut config, line, state, &None).unwrap(); assert!(matches!(next_state, StdinState::Waiting)); } @@ -552,7 +553,7 @@ mod tests { let state = StdinState::Message; let empty_line = ""; - let result = process_line(&mut builder, &mut config, empty_line, state); + let result = process_line(&mut builder, &mut config, empty_line, state, &None); // Should handle empty line without error assert!(result.is_ok()); @@ -569,6 +570,7 @@ mod tests { &mut config, "Line 1 of panic", StdinState::Message, + &None, ) .unwrap(); @@ -593,17 +595,32 @@ mod tests { &mut config, DD_CRASHTRACK_BEGIN_MESSAGE, state, + &None, ) .unwrap(); assert!(matches!(state, StdinState::Message)); // Add message content - state = process_line(&mut builder, &mut config, "test panic message", state).unwrap(); + state = process_line( + &mut builder, + &mut config, + "test panic message", + state, + &None, + ) + .unwrap(); assert!(matches!(state, StdinState::Message)); assert!(builder.has_message()); // End message - state = process_line(&mut builder, &mut config, DD_CRASHTRACK_END_MESSAGE, state).unwrap(); + state = process_line( + &mut builder, + &mut config, + DD_CRASHTRACK_END_MESSAGE, + state, + &None, + ) + .unwrap(); assert!(matches!(state, StdinState::Waiting)); } }