Skip to content

Include panic() and std::terminate() info in Sentry crash reports#459

Merged
edolstra merged 3 commits into
mainfrom
more-sentry
May 15, 2026
Merged

Include panic() and std::terminate() info in Sentry crash reports#459
edolstra merged 3 commits into
mainfrom
more-sentry

Conversation

@edolstra
Copy link
Copy Markdown
Collaborator

@edolstra edolstra commented May 15, 2026

Motivation

This includes panic messages and uncaught exception type/message in crash reports.

Context

Summary by CodeRabbit

  • New Features
    • Crash diagnostics now capture panic messages and additional exception details, and annotate unexpected process terminations for richer crash reports.
  • Tests
    • Expanded functional tests to cover the new crash scenarios and validate enhanced error-reporting behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6994ab35-5b1f-4d77-b25c-9f46d7457750

📥 Commits

Reviewing files that changed from the base of the PR and between 77ebf8f and 83df16d.

📒 Files selected for processing (1)
  • src/libutil/error.cc
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/libutil/error.cc

📝 Walkthrough

Walkthrough

Adds Sentry tagging for panic messages and uncaught termination exceptions, installs a terminate handler that tags exception type/message before termination, and extends crash tests to exercise panic and destructor-throw termination scenarios.

Changes

Sentry Crash Reporting

Layer / File(s) Summary
Panic function Sentry reporting
src/libutil/error.cc
Sentry header included; panic() sets panic_msg Sentry tag with the provided message before calling std::terminate().
Terminate exception handler installation
src/nix/main.cc
Adds terminateHandler() that extracts current exception info, sets Sentry tags for type and message, and delegates to default termination. Installs handler via std::set_terminate() during Sentry initialization.
Crash test scenarios for panic and terminate
src/nix/crash.cc, tests/functional/sentry.sh
Adds doThrow() helper and panic/terminate crash modes to CmdCrash; updates functional test loop to include the new crash types.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • cole-h

Poem

🐰 A hop, a tag, a careful trace,
I set the panic in its place.
When handlers catch what goes astray,
Sentry stores the words we say.
Hooray for logs that saved the day!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding panic() and std::terminate() information to Sentry crash reports, which is reflected across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch more-sentry

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/libutil/error.cc`:
- Line 456: setSentryTag is being called with msg.data() where msg is a
std::string_view, but std::string_view::data() is not null-terminated; convert
the view to a null-terminated string before passing it to setSentryTag (e.g.,
construct a std::string from msg and pass its c_str()), or alternatively change
setSentryTag to accept a std::string_view and make an internal copy; update the
call site (setSentryTag("panic_msg", msg)) or the setSentryTag implementation so
the value associated with "panic_msg" is always a null-terminated buffer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17bb80ac-731c-41fa-b04a-e06c950a2cdd

📥 Commits

Reviewing files that changed from the base of the PR and between 5dbb952 and 77ebf8f.

📒 Files selected for processing (4)
  • src/libutil/error.cc
  • src/nix/crash.cc
  • src/nix/main.cc
  • tests/functional/sentry.sh

Comment thread src/libutil/error.cc Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

@github-actions github-actions Bot temporarily deployed to pull request May 15, 2026 09:36 Inactive
@edolstra edolstra added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit c9453f7 May 15, 2026
29 checks passed
@edolstra edolstra deleted the more-sentry branch May 15, 2026 13:49
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.

2 participants