Skip to content

Commit df85e55

Browse files
committed
Make sure we can handle different type of panic payload
1 parent 1307f3b commit df85e55

File tree

6 files changed

+143
-25
lines changed

6 files changed

+143
-25
lines changed

bin_tests/src/modes/behavior.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ pub fn get_behavior(mode_str: &str) -> Box<dyn Behavior> {
135135
Box::new(test_012_runtime_callback_frame_invalid_utf8::Test)
136136
}
137137
"panic_hook_after_fork" => Box::new(test_013_panic_hook_after_fork::Test),
138+
"panic_hook_string" => Box::new(test_014_panic_hook_string::Test),
139+
"panic_hook_unknown_type" => Box::new(test_015_panic_hook_unknown_type::Test),
138140
_ => panic!("Unknown mode: {mode_str}"),
139141
}
140142
}

bin_tests/src/modes/unix/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,5 @@ pub mod test_010_runtime_callback_frame;
1414
pub mod test_011_runtime_callback_string;
1515
pub mod test_012_runtime_callback_frame_invalid_utf8;
1616
pub mod test_013_panic_hook_after_fork;
17+
pub mod test_014_panic_hook_string;
18+
pub mod test_015_panic_hook_unknown_type;
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2025-Present Datadog, Inc. https://www.datadoghq.com/
2+
// SPDX-License-Identifier: Apache-2.0
3+
//
4+
// Test that panic hooks work correctly with String payloads (not just &str).
5+
// This validates that:
6+
// 1. String panic payloads are correctly captured
7+
// 2. The panic message format is "Process panicked with message: <msg>"
8+
use crate::modes::behavior::Behavior;
9+
use libdd_crashtracker::CrashtrackerConfiguration;
10+
use std::path::Path;
11+
12+
pub struct Test;
13+
14+
impl Behavior for Test {
15+
fn setup(
16+
&self,
17+
_output_dir: &Path,
18+
_config: &mut CrashtrackerConfiguration,
19+
) -> anyhow::Result<()> {
20+
Ok(())
21+
}
22+
23+
fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> {
24+
Ok(())
25+
}
26+
27+
fn post(&self, _output_dir: &Path) -> anyhow::Result<()> {
28+
let dynamic_value = 42;
29+
panic!("Panic with value: {}", dynamic_value);
30+
}
31+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2025-Present Datadog, Inc. https://www.datadoghq.com/
2+
// SPDX-License-Identifier: Apache-2.0
3+
//
4+
// Test that panic hooks work correctly with unknown types via panic_any.
5+
// This validates that:
6+
// 1. panic_any() with non-string types is handled gracefully
7+
// 2. The message format is: "Process panicked with unknown type (<location>)"
8+
use crate::modes::behavior::Behavior;
9+
use libdd_crashtracker::CrashtrackerConfiguration;
10+
use std::path::Path;
11+
12+
pub struct Test;
13+
14+
impl Behavior for Test {
15+
fn setup(
16+
&self,
17+
_output_dir: &Path,
18+
_config: &mut CrashtrackerConfiguration,
19+
) -> anyhow::Result<()> {
20+
Ok(())
21+
}
22+
23+
fn pre(&self, _output_dir: &Path) -> anyhow::Result<()> {
24+
Ok(())
25+
}
26+
27+
fn post(&self, _output_dir: &Path) -> anyhow::Result<()> {
28+
std::panic::panic_any(42i32);
29+
}
30+
}

bin_tests/tests/crashtracker_bin_test.rs

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -327,11 +327,11 @@ fn test_crash_tracking_app(crash_type: &str) {
327327

328328
match crash_type_owned.as_str() {
329329
"panic" => {
330-
let expected_message = "program panicked";
331-
assert_eq!(
332-
error["message"].as_str().unwrap(),
333-
expected_message,
334-
"Expected panic message for panic crash type"
330+
let message = error["message"].as_str().unwrap();
331+
assert!(
332+
message.contains("Process panicked with message") && message.contains("program panicked"),
333+
"Expected panic message to contain 'Process panicked with message' and 'program panicked', got: {}",
334+
message
335335
);
336336
}
337337
"segfault" => {
@@ -360,6 +360,31 @@ fn test_crash_tracking_app(crash_type: &str) {
360360
#[cfg_attr(miri, ignore)]
361361
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
362362
fn test_crash_tracking_bin_panic_hook_after_fork() {
363+
test_panic_hook_mode("panic_hook_after_fork", "child panicked after fork");
364+
}
365+
366+
#[test]
367+
#[cfg_attr(miri, ignore)]
368+
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
369+
fn test_crash_tracking_bin_panic_hook_string() {
370+
test_panic_hook_mode(
371+
"panic_hook_string",
372+
"Process panicked with message: Panic with value: 42",
373+
);
374+
}
375+
376+
#[test]
377+
#[cfg_attr(miri, ignore)]
378+
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
379+
fn test_crash_tracking_bin_panic_hook_unknown_type() {
380+
test_panic_hook_mode(
381+
"panic_hook_unknown_type",
382+
"Process panicked with unknown type",
383+
);
384+
}
385+
386+
/// Helper function to run panic hook tests with different payload types
387+
fn test_panic_hook_mode(mode: &str, expected_message_substring: &str) {
363388
use bin_tests::test_runner::run_custom_crash_test;
364389

365390
// Set up custom artifacts: receiver + crashtracker_bin_test
@@ -368,24 +393,18 @@ fn test_crash_tracking_bin_panic_hook_after_fork() {
368393

369394
let artifacts_map = build_artifacts(&[&crashtracker_receiver, &crashtracker_bin_test]).unwrap();
370395

371-
let validator: ValidatorFn = Box::new(|payload, _fixtures| {
396+
let expected_msg = expected_message_substring.to_owned();
397+
let validator: ValidatorFn = Box::new(move |payload, _fixtures| {
372398
// Verify the panic message is captured
373399
let error = &payload["error"];
374400
let message = error["message"].as_str().unwrap();
375401
assert!(
376-
message.contains("child panicked after fork"),
377-
"Expected panic message to contain 'child panicked after fork', got: {}",
402+
message.contains(&expected_msg),
403+
"Expected panic message to contain '{}', got: {}",
404+
expected_msg,
378405
message
379406
);
380407

381-
// TODO change the kind into Panic instead
382-
let kind = error["kind"].as_str().unwrap();
383-
assert_eq!(
384-
kind, "UnixSignal",
385-
"Expected error kind to be UnixSignal, got: {}",
386-
kind
387-
);
388-
389408
Ok(())
390409
});
391410

@@ -395,8 +414,8 @@ fn test_crash_tracking_bin_panic_hook_after_fork() {
395414
cmd.arg(format!("file://{}", fixtures.crash_profile_path.display()))
396415
.arg(&artifacts_map[&crashtracker_receiver])
397416
.arg(&fixtures.output_dir)
398-
.arg("panic_hook_after_fork") // mode
399-
.arg("donothing"); // crash method (not used in this mode)
417+
.arg(mode)
418+
.arg("donothing"); // crash method (not used in panic hook tests)
400419
},
401420
validator,
402421
)

libdd-crashtracker/src/collector/crash_handler.rs

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,30 @@ pub fn update_metadata(metadata: Metadata) -> anyhow::Result<()> {
7878
Ok(())
7979
}
8080

81+
/// Format a panic message with optional location information.
82+
fn format_panic_message(
83+
category: &str,
84+
description: &str,
85+
location: Option<&panic::Location>,
86+
) -> String {
87+
let base = match location {
88+
Some(loc) => format!(
89+
"Process panicked with {} ({}:{}:{})",
90+
category,
91+
loc.file(),
92+
loc.line(),
93+
loc.column()
94+
),
95+
None => format!("Process panicked with {}", category),
96+
};
97+
98+
if description.is_empty() {
99+
base
100+
} else {
101+
format!("{}: {}", base, description)
102+
}
103+
}
104+
81105
/// Register the panic hook.
82106
///
83107
/// 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<()> {
98122
let old_hook_ptr = Box::into_raw(Box::new(old_hook));
99123
PREVIOUS_PANIC_HOOK.swap(old_hook_ptr, SeqCst);
100124
panic::set_hook(Box::new(|panic_info| {
101-
if let Some(s) = panic_info.payload().downcast_ref::<&str>() {
102-
let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(s.to_string())), SeqCst);
103-
// message_ptr should be null, but just in case.
104-
if !message_ptr.is_null() {
105-
unsafe {
106-
std::mem::drop(Box::from_raw(message_ptr));
107-
}
125+
// Extract panic message from payload (supports &str and String)
126+
let message = if let Some(&s) = panic_info.payload().downcast_ref::<&str>() {
127+
format_panic_message("message", s, panic_info.location())
128+
} else if let Some(s) = panic_info.payload().downcast_ref::<String>() {
129+
format_panic_message("message", s.as_str(), panic_info.location())
130+
} else {
131+
// For non-string types, use a generic message
132+
format_panic_message("unknown type", "", panic_info.location())
133+
};
134+
135+
// Store the message, cleaning up any old message
136+
let message_ptr = PANIC_MESSAGE.swap(Box::into_raw(Box::new(message)), SeqCst);
137+
// message_ptr should be null, but just in case.
138+
if !message_ptr.is_null() {
139+
unsafe {
140+
std::mem::drop(Box::from_raw(message_ptr));
108141
}
109142
}
143+
110144
call_previous_panic_hook(panic_info);
111145
}));
112146
Ok(())

0 commit comments

Comments
 (0)