Skip to content

fix(cli): install panic hook so panics flow through OpenTelemetry (OTel) #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aatran14 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(cli): install panic hook so panics flow through OpenTelemetry (OTel) #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8233fa1
fix(cli): install panic hook so panics flow through OpenTelemetry (OTel)
aatran14 Jul 22, 2026
6a96225
test(cli): verify panic_message falls back graceefully on non-string …
aatran14 Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ async fn run() -> (
);
}

puffgres_cli::observability::install_panic_hook();

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Flush telemetry before exiting on panic

In an OTEL-enabled command such as puffgres run, apply, or remove that panics after observability::init, this hook only enqueues a tracing::error! and then invokes the default panic hook; because the unwind never returns from run().await, the Telemetry::shutdown() call in main is skipped. The log this change is intended to export can therefore remain buffered and be lost at process exit unless the panic path catches the unwind and flushes/shuts down telemetry synchronously.

Useful? React with 👍 / 👎.

@aatran14 aatran14 Jul 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think agree with this

To my understanding:

  • This PR makes panic visible to the Otel pipeline; for example, structured payload & location
  • The "flush-on-panic" gap is existing behavior. Any batched telementry not yet exported is lost when a panic unwinds past main's shutdown(), regardless of this change.
  • This PR happens to be the first log emitted at panic time. I think that flushing on the panic path whether that be a sync force_flush in run() or the hook ultimately entails a lot more work which justifies its own PR.

To clarify:
In the interest of simplicity, scope this PR to

  1. puffgres_cli::observability::install_panic_hook(); in cli/src/main.rs
  2. the testing for it


// All remaining commands need at least ProjectPaths
let paths = match ProjectPaths::from_current_dir() {
Ok(p) => p,
Expand Down
27 changes: 21 additions & 6 deletions crates/cli/src/observability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,22 @@ pub fn init_fmt_only() {
.init();
}

/// Read a panic payload as text. Handles `&str` and `String`; otherwise
/// returns `"unknown panic"`.
fn panic_message(payload: &(dyn std::any::Any + Send)) -> &str {
payload
.downcast_ref::<&str>()
.copied()
.or_else(|| payload.downcast_ref::<String>().map(String::as_str))
.unwrap_or("unknown panic")
}

/// Install panic hook that emits tracing::error! so panics
/// flow through the OTel pipeline.
pub fn install_panic_hook() {
let default_hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
let payload = info
.payload()
.downcast_ref::<&str>()
.copied()
.or_else(|| info.payload().downcast_ref::<String>().map(|s| s.as_str()))
.unwrap_or("unknown panic");
let payload = panic_message(info.payload());
let location = info
.location()
.map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column()))
Expand Down Expand Up @@ -280,3 +285,13 @@ fn parse_otlp_headers(raw: &str) -> HashMap<String, String> {
})
.collect()
}

#[cfg(test)] // The crash reporter must never itself panic: a non-string payload falls back instead of unwrap/expect.

mod tests {
use super::*;
#[test]
fn panic_message_falls_back_for_non_string_payload() {
assert_eq!(panic_message(&42i32), "unknown panic");
}
}