Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat: add sentry panic reporting #1089

Merged
merged 1 commit into from
Nov 30, 2017
Merged

feat: add sentry panic reporting #1089

merged 1 commit into from
Nov 30, 2017

Conversation

bbangert
Copy link
Member

Closes #1066

alexcrichton
alexcrichton previously approved these changes Nov 30, 2017
.expect("Invalid Sentry DSN specified");
let mut core = Core::new().expect("Unable to create core");
let sentry = sentry::Sentry::from_settings(core.handle(), Default::default(), creds);
sentry.register_panic_handler(Some(|_: &PanicInfo| -> () {}));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the empty callback here may mean that a panic doesn't actually get logged to stdout/stderr, but maybe it'd be good to print something briefly or run the default panic handler perhaps? (just in case)

@bbangert
Copy link
Member Author

Just a note, this does not retain the JoinHandle for the new Sentry thread. For a future issue where we fix the exit to be graceful, the Join handle for the thread will need to be retained somewhere so it can be join'd on graceful shutdown.

@codecov-io
Copy link

codecov-io commented Nov 30, 2017

Codecov Report

Merging #1089 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1089   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files          58       58           
  Lines        9264     9264           
=======================================
  Hits         9238     9238           
  Misses         26       26

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8adbb3...66554d9. Read the comment docs.

@bbangert bbangert merged commit e4503d5 into master Nov 30, 2017
@bbangert bbangert deleted the feat/issue-1066 branch November 30, 2017 23:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants