Skip to content

Commit 5a6dab0

Browse files
committed
feat(cli): surface panic payloads in debug diagnostics
Allow ADR-T-010 panic control-plane records to include string panic payloads when debug diagnostics are enabled, while continuing to omit the payload field for normal runs. Wire the debug flag through the shared CLI runners and config probe, and add coverage for the payload gate, JSON serialization, and string payload extraction.
1 parent 503e90e commit 5a6dab0

3 files changed

Lines changed: 88 additions & 8 deletions

File tree

packages/index-cli-common/src/lib.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub const CONTROL_PLANE_SCHEMA: u32 = 1;
3131
pub const REDACTED: &str = "[redacted]";
3232

3333
static PANIC_REPORTED: AtomicBool = AtomicBool::new(false);
34+
static PANIC_PAYLOAD_REPORTING_ENABLED: AtomicBool = AtomicBool::new(true);
3435
static STDERR_WRITE_LOCK: Mutex<()> = Mutex::new(());
3536

3637
/// Baseline exit-code classes shared by ADR-T-010 command-line tools.
@@ -116,11 +117,13 @@ pub enum ControlPlaneFields {
116117
UsageError { exit_code: u8, clap_error_kind: String },
117118
/// Details for stdout TTY refusal.
118119
TtyRefusal { exit_code: u8, stream: StandardStream },
119-
/// Details for a panic diagnostic. The panic payload is deliberately omitted.
120+
/// Details for a panic diagnostic. The panic payload is only exposed when debug diagnostics are enabled.
120121
Panic {
121122
exit_code: u8,
122123
thread: Option<String>,
123124
location: Option<String>,
125+
#[serde(skip_serializing_if = "Option::is_none")]
126+
payload: Option<String>,
124127
},
125128
}
126129

@@ -207,7 +210,7 @@ impl ControlPlaneRecord {
207210

208211
/// Build a panic diagnostic record.
209212
#[must_use]
210-
pub fn panic(command: &str, thread: Option<&str>, location: Option<&str>) -> Self {
213+
pub fn panic(command: &str, thread: Option<&str>, location: Option<&str>, payload: Option<&str>) -> Self {
211214
Self::new(
212215
command,
213216
ControlPlaneRecordKind::Panic,
@@ -216,6 +219,7 @@ impl ControlPlaneRecord {
216219
exit_code: CommandExit::Failure.code(),
217220
thread: thread.map(str::to_string),
218221
location: location.map(str::to_string),
222+
payload: payload.map(str::to_string),
219223
}),
220224
)
221225
}
@@ -421,6 +425,9 @@ fn level_directive(level: tracing::Level) -> String {
421425
///
422426
/// Returns [`CliExit`] when parsing should stop and the caller should write the
423427
/// enclosed record to stderr before exiting with the enclosed exit class.
428+
/// The error value is intentionally returned by value because this path is only
429+
/// used when parsing stops, and boxing it would complicate the public helper API.
430+
#[allow(clippy::result_large_err)]
424431
pub fn parse_args_from<T, I, A>(args: I) -> Result<T, CliExit>
425432
where
426433
T: Parser,
@@ -531,14 +538,41 @@ pub fn install_json_panic_hook(command_name: &str) {
531538
let location = panic_info
532539
.location()
533540
.map(|location| format!("{}:{}:{}", location.file(), location.line(), location.column()));
534-
let record = ControlPlaneRecord::panic(&command_name, thread_name, location.as_deref());
541+
let payload = panic_payload_reporting_enabled()
542+
.then(|| panic_payload_message(panic_info))
543+
.flatten();
544+
let record = ControlPlaneRecord::panic(&command_name, thread_name, location.as_deref(), payload);
535545
let _ignored = try_emit_control_plane_record(&record);
536546
}
537547

538548
exit_with(CommandExit::Failure);
539549
}));
540550
}
541551

552+
/// Enable or disable string panic payloads in JSON panic diagnostics.
553+
///
554+
/// Payload reporting starts enabled so panics before argument parsing still
555+
/// include their string payload. Call this with the parsed `--debug` value once
556+
/// arguments are available.
557+
pub fn set_panic_payload_reporting_enabled(enabled: bool) {
558+
PANIC_PAYLOAD_REPORTING_ENABLED.store(enabled, Ordering::SeqCst);
559+
}
560+
561+
fn panic_payload_reporting_enabled() -> bool {
562+
PANIC_PAYLOAD_REPORTING_ENABLED.load(Ordering::SeqCst)
563+
}
564+
565+
fn panic_payload_message<'a>(info: &'a std::panic::PanicHookInfo<'_>) -> Option<&'a str> {
566+
panic_payload_message_from_payload(info.payload())
567+
}
568+
569+
fn panic_payload_message_from_payload(payload: &(dyn std::any::Any + Send)) -> Option<&str> {
570+
payload
571+
.downcast_ref::<&str>()
572+
.copied()
573+
.or_else(|| payload.downcast_ref::<String>().map(String::as_str))
574+
}
575+
542576
/// Exit the current process with an ADR-T-010 exit class.
543577
#[allow(clippy::exit)]
544578
pub fn exit_with(exit: CommandExit) -> ! {
@@ -633,6 +667,7 @@ where
633667
CommandError: std::fmt::Display,
634668
Run: FnOnce() -> Result<Output, CommandError>,
635669
{
670+
set_panic_payload_reporting_enabled(debug);
636671
install_json_panic_hook(command_name);
637672
init_json_tracing_with_debug(debug, default_level);
638673

@@ -671,6 +706,7 @@ where
671706
CommandError: std::fmt::Display,
672707
Run: FnOnce() -> Result<(), CommandError>,
673708
{
709+
set_panic_payload_reporting_enabled(debug);
674710
install_json_panic_hook(command_name);
675711
init_json_tracing_with_debug(debug, default_level);
676712

@@ -699,6 +735,7 @@ where
699735
Run: FnOnce() -> RunFuture,
700736
RunFuture: Future<Output = Result<(), CommandError>>,
701737
{
738+
set_panic_payload_reporting_enabled(debug);
702739
install_json_panic_hook(command_name);
703740
init_json_tracing_with_debug(debug, default_level);
704741

packages/index-cli-common/src/tests/mod.rs

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313
//! | `json_line_writer_appends_newline` | JSON record helper writes one complete line. |
1414
//! | `usage_error_record_carries_fields` | Usage records include exit code and clap kind. |
1515
//! | `tty_refusal_record_carries_fields` | TTY refusal records identify stdout and code 2. |
16-
//! | `panic_record_omits_payload` | Panic records avoid serialising panic payloads. |
16+
//! | `panic_record_omits_payload_without_debug` | Panic records hide payloads without debug. |
17+
//! | `panic_record_carries_debug_payload` | Panic records can expose string payloads. |
18+
//! | `panic_payload_reporting_defaults_enabled_then_follows_debug_flag` | Startup payload gate behavior. |
19+
//! | `panic_payload_message_extracts_string_payloads` | String panic payloads are downcast. |
1720
//! | `parse_args_from_returns_help_record` | Clap help becomes JSON stderr control data. |
1821
//! | `parse_args_from_returns_version_record` | Clap version becomes JSON stderr control data. |
1922
//! | `parse_args_from_returns_usage_record` | Clap argv errors become JSON usage records. |
@@ -42,8 +45,8 @@ use serde_json::json;
4245

4346
use crate::{
4447
BaseArgs, CONTROL_PLANE_SCHEMA, CommandExit, ControlPlaneFields, ControlPlaneRecord, ControlPlaneRecordKind, REDACTED,
45-
StandardStream, TracingFilterSource, parse_args_from, redact_database_url, redact_field_value, tracing_filter_from_rust_log,
46-
write_json_line,
48+
StandardStream, TracingFilterSource, panic_payload_message_from_payload, panic_payload_reporting_enabled, parse_args_from,
49+
redact_database_url, redact_field_value, set_panic_payload_reporting_enabled, tracing_filter_from_rust_log, write_json_line,
4750
};
4851

4952
/// A `Write` that fails every call with `BrokenPipe`.
@@ -235,8 +238,8 @@ fn tty_refusal_record_carries_fields() {
235238
}
236239

237240
#[test]
238-
fn panic_record_omits_payload() {
239-
let record = ControlPlaneRecord::panic("fixture", Some("main"), Some("src/main.rs:12:34"));
241+
fn panic_record_omits_payload_without_debug() {
242+
let record = ControlPlaneRecord::panic("fixture", Some("main"), Some("src/main.rs:12:34"), None);
240243
let value = serde_json::to_value(record).unwrap();
241244

242245
assert_eq!(value["kind"], json!("panic"));
@@ -246,6 +249,44 @@ fn panic_record_omits_payload() {
246249
assert!(value["fields"].get("payload").is_none());
247250
}
248251

252+
#[test]
253+
fn panic_record_carries_debug_payload() {
254+
let record = ControlPlaneRecord::panic(
255+
"fixture",
256+
Some("main"),
257+
Some("src/main.rs:12:34"),
258+
Some("panic with \"quoted\" detail"),
259+
);
260+
let line = serde_json::to_string(&record).unwrap();
261+
262+
assert!(line.contains(r#""payload":"panic with \"quoted\" detail""#));
263+
264+
let value: serde_json::Value = serde_json::from_str(&line).unwrap();
265+
assert_eq!(value["fields"]["payload"], json!("panic with \"quoted\" detail"));
266+
}
267+
268+
#[test]
269+
fn panic_payload_reporting_defaults_enabled_then_follows_debug_flag() {
270+
assert!(panic_payload_reporting_enabled());
271+
272+
set_panic_payload_reporting_enabled(false);
273+
assert!(!panic_payload_reporting_enabled());
274+
275+
set_panic_payload_reporting_enabled(true);
276+
assert!(panic_payload_reporting_enabled());
277+
}
278+
279+
#[test]
280+
fn panic_payload_message_extracts_string_payloads() {
281+
let borrowed_payload: &(dyn std::any::Any + Send) = &"borrowed panic";
282+
let owned_payload: &(dyn std::any::Any + Send) = &String::from("owned panic");
283+
let numeric_payload: &(dyn std::any::Any + Send) = &1_u8;
284+
285+
assert_eq!(panic_payload_message_from_payload(borrowed_payload), Some("borrowed panic"));
286+
assert_eq!(panic_payload_message_from_payload(owned_payload), Some("owned panic"));
287+
assert_eq!(panic_payload_message_from_payload(numeric_payload), None);
288+
}
289+
249290
#[test]
250291
fn parse_args_from_returns_help_record() {
251292
let Err(exit) = parse_args_from::<FixtureCli, _, _>(["fixture-helper", "--help"]) else {

packages/index-config-probe/src/bin/torrust-index-config-probe.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::process::ExitCode;
1010
use clap::Parser;
1111
use torrust_index_cli_common::{
1212
BaseArgs, emit, init_json_tracing_with_debug, install_json_panic_hook, parse_args_or_exit, refuse_if_stdout_is_tty,
13+
set_panic_payload_reporting_enabled,
1314
};
1415
use torrust_index_config::{DEFAULT_CONFIG_TOML_PATH, Info, load_settings};
1516
use torrust_index_config_probe::{ProbeError, probe};
@@ -32,6 +33,7 @@ fn main() -> ExitCode {
3233
install_json_panic_hook(COMMAND_NAME);
3334

3435
let args = parse_args_or_exit::<Args>();
36+
set_panic_payload_reporting_enabled(args.base.debug);
3537
init_json_tracing_with_debug(args.base.debug, tracing::Level::INFO);
3638
refuse_if_stdout_is_tty(COMMAND_NAME);
3739

0 commit comments

Comments
 (0)