sentry-native icon indicating copy to clipboard operation
sentry-native copied to clipboard

Linux (Breakpad) fails with `set_before_send` on crash.

Open daxpedda opened this issue 5 years ago • 1 comments

https://github.com/daxpedda/sentry-contrib-native/issues/8

Describe the bug If Options::set_before_send is used, and a native crash occurs, sentry fails to serialize the envelope contain the minidump to disk, which means subsequent runs can't send the crash and it goes unreported.

To Reproduce

  1. Setup Sentry, including set_before send
let mut opts = sentry_native::Options::new();
opts.set_debug(config.verbose);

opts.set_before_send(|val: sentry_native::Value| {
    if let Some(event) = val.as_map() {
        if let Some(id) = event.get("event_id") {
            println!("sending sentry event {}", id.as_str().unwrap());
        }
    }

    val
});

let shutdown = opts.init()?;
  1. Force a crash
let s: &u32 = unsafe { &*std::ptr::null() };

println!("we are crashing by accessing a null reference: {}", *s);
  1. With debug output enabled for the SDK, you should see a line saying [sentry] DEBUG failed to open file "/<uuid>.envelope" for writing

Expected behavior The envelope should be serialized to disk, and subsequent runs should send those envelopes to the upstream sentry server for consumption and analysis.

Environment:

  • OS: Fedora 32
  • Default transport both exhibit the same behavior
  • Version 5f4bf1f
  • Rust 1.44.1

Additional context So after some digging, it appears as if some memory might be getting stomped during the before_send callback, I don't think it's in the native code as there doesn't seem to be much going on there, but I could of course be wrong. But basically, the options run field has the run_path which is the directory where sentry stores serialized envelopes and temporary crash dumps, but if the before_send callback is set and called, run_path ends up being an empty string, so when sentry creates the path to store the serialized envelope in by joining the run_path with the uuid + .envelope extension, which should be an absolute path, it ends up being just /<uuid>.envelope which then fails to be created and sentry gives up serializing the envelope and the crash is lost forever.

I would look into this more but once I realized that before_send was causing this I just stopped using it (it was only for logging purposes) so I could move on, but thought I would at least make this report so that there's a record of it. 🙂

This is happening on Linux with the default Breakpad handling with the latest stable release. I will look into details into this later, will also verify this with C code just to make sure this is not a problem on my side.

daxpedda avatar Jul 14 '20 17:07 daxpedda

We are currently rethinking if we would like to split this hook up into different ones. The problem is that in the case of a hard crash, this hook actually runs as part of the signal handler, and I’m not sure that the rust code here is signal safe.

Maybe we will introduce a new before_crash hook which needs to be signal safe, and bypass the before_send hook for events captured from hard crashes.

Swatinem avatar Jul 15 '20 11:07 Swatinem