Skip to content

sentry-panic: switch to PanicHookInfo from deprecated PanicInfo#1074

Open
mvanhorn wants to merge 1 commit intogetsentry:masterfrom
mvanhorn:osc/862-panichookinfo
Open

sentry-panic: switch to PanicHookInfo from deprecated PanicInfo#1074
mvanhorn wants to merge 1 commit intogetsentry:masterfrom
mvanhorn:osc/862-panichookinfo

Conversation

@mvanhorn
Copy link
Copy Markdown

Closes #862.

The workspace MSRV is 1.88 (Cargo.toml), so std::panic::PanicHookInfo (stabilized in 1.81) is always available. This removes the six #[allow(deprecated)] annotations that were silencing the PanicInfo deprecation warning and uses PanicHookInfo directly in sentry-panic.

Source compatibility

std keeps PanicInfo as a type alias for PanicHookInfo:

// library/std/src/panic.rs
pub type PanicInfo<'a> = PanicHookInfo<'a>;

So downstream extractor closures that still use &PanicInfo<'_> continue to match the new &PanicHookInfo<'_> signatures on add_extractor, event_from_panic_info, and message_from_panic_info.

Scope

Only sentry-panic/src/lib.rs is touched. No other crates reference PanicInfo.

Verified with cargo build --all, cargo clippy -p sentry-panic, cargo fmt -p sentry-panic --check, and cargo test -p sentry-panic.

The workspace MSRV is 1.88, so `std::panic::PanicHookInfo` is always
available. Drop the six `#[allow(deprecated)]` annotations that were
silencing the `PanicInfo` deprecation warning and use `PanicHookInfo`
directly.

Source compatibility is preserved because `std` keeps `PanicInfo` as a
type alias for `PanicHookInfo` (see `library/std/src/panic.rs`), so
downstream extractor closures that take `&PanicInfo<'_>` continue to
match the new `&PanicHookInfo<'_>` signatures.

Closes getsentry#862.
@sdk-maintainer-bot
Copy link
Copy Markdown

This PR has been automatically closed. The referenced issue does not show a discussion between you and a maintainer.

To avoid wasted effort on both sides, please discuss your proposed approach in the issue first and wait for a maintainer to respond before opening a PR.

Please review our contributing guidelines for more details.

@szokeasaurusrex
Copy link
Copy Markdown
Member

szokeasaurusrex commented Apr 22, 2026

Sorry about the bot @mvanhorn, this PR indeed looks valid. I'll review when I have time.

In general, we like issues to be opened before PRs, but for simple stuff like this change, I think the issue is not necessarily needed.

Thanks again for your contribution!

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.85%. Comparing base (a57b91c) to head (77c4b83).
⚠️ Report is 53 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1074      +/-   ##
==========================================
+ Coverage   73.81%   73.85%   +0.03%     
==========================================
  Files          64       65       +1     
  Lines        7538     7557      +19     
==========================================
+ Hits         5564     5581      +17     
- Misses       1974     1976       +2     

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Ok, just did a quick review; I am not sure whether we should do this at all, since it would break our public API.

Unless using PanicInfo is causing you some inconvenience, I would suggest we just keep the code as it is for now. Otherwise, perhaps it makes sense to open an issue to discuss the right approach after all 😅

If using PanicInfo is causing trouble for you, please open an issue to explain what the concrete problem is, and we can discuss a potential solution there.

@mvanhorn
Copy link
Copy Markdown
Author

Thanks for the quick look @szokeasaurusrex. The PR body covers this but may have buried it - std defines pub type PanicInfo<'a> = PanicHookInfo<'a> (type alias, not a separate type), so downstream extractor closures using &PanicInfo<'_> continue to match the new &PanicHookInfo<'_> signatures on add_extractor / event_from_panic_info / message_from_panic_info. No break at the public-API boundary, just silences the six #[allow(deprecated)] annotations.

Happy to close if you'd still rather leave things as-is - the deprecation warning is cosmetic and the allow attrs aren't hurting anything. Just didn't want you to have to merge another six #[allow(deprecated)] reminders down the line.

@lcian lcian removed their request for review April 23, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch to PanicHookInfo in sentry-panic

2 participants