Repository: getsentry/sentry-rust (rust)

Issue:
`sentry::capture_error` does not capture the backtrace even when the feature is enabled
### Environment

How do you use Sentry?
Self-hosted 21.5.0 and 9.1.2

Which SDK and version?
Rust 0.18 and 0.22 (because of Tokio 0.2 and 1.0)

### Steps to Reproduce

1. Install sentry integration with backtraces enabled
2. Call `sentry::capture_error`
3. Go to sentry, see the absence of stacktrace

- `Cargo.toml`
```toml
sentry = { version = "0.22", features = ["backtrace"] }
```
- `main.rs`
```rust
fn main() {
    sentry::init(("https://my-dsn", sentry::ClientOptions {
        attach_stacktrace: true,
        ..Default::default()
    });
    println!("Sentry installed");

    let error = "thisisnotanumber".parse::<u32>().unwrap_err();
    sentry::capture_error(&error);
}
```

### Expected Result

The stacktrace would be shown in the web ui.

![sentry-backtrace](https://user-images.githubusercontent.com/4009336/116897532-a49e1600-ac35-11eb-8266-eafdf9e22192.png)

### Actual Result

The web UI shows no stacktrace for the issue.

![sentry-nobacktrace](https://user-images.githubusercontent.com/4009336/116897552-ab2c8d80-ac35-11eb-96ab-dc99b8e97c3c.png)

### Fix suggestion

The above screenshot with backtrace was obtained by slightly tweaking the condition in the `AttachStacktraceIntegration`, which checks whether the event contains exceptions before attaching the stacktrace. However, in the case of events made from `capture_error`, they do _not_ contain a backtrace, resulting in no backtrace being attached to the event. The tweak is as follows:

```rust
diff --git a/sentry-backtrace/src/integration.rs b/sentry-backtrace/src/integration.rs
index 49f6826..e6c3a5a 100644
--- a/sentry-backtrace/src/integration.rs
+++ b/sentry-backtrace/src/integration.rs
@@ -73,7 +73,7 @@ impl Integration for AttachStacktraceIntegration {
         mut event: Event<'static>,
         options: &ClientOptions,
     ) -> Option<Event<'static>> {
-        if options.attach_stacktrace && event.exception.is_empty() {
+        if options.attach_stacktrace && event.threads.is_empty() {
             let thread = current_thread(true);
             if thread.stacktrace.is_some() {
                 event.threads.values.push(thread);
```

While this works, it may not be ideal as I feel like this is not really what the `threads` field is for. A fix that may be cleaner would be to record the backtrace in `sentry_core::error::event_from_error` and/or `exception_from_error`.

Pull request:
fix: Honor the attach_stacktrace option when capturing errors
fixes #328

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 28ae548..2ac9472 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@
 
 - The minium supported Rust version was bumped to **1.46.0** due to requirements from dependencies.
 
+**Fixes**:
+
+- Honor the `attach_stacktrace` option correctly when capturing errors.
+
 ## 0.22.0
 
 **Breaking Changes**:
diff --git a/sentry-backtrace/src/integration.rs b/sentry-backtrace/src/integration.rs
index 49f6826..4a3b607 100644
--- a/sentry-backtrace/src/integration.rs
+++ b/sentry-backtrace/src/integration.rs
@@ -73,7 +73,7 @@ impl Integration for AttachStacktraceIntegration {
         mut event: Event<'static>,
         options: &ClientOptions,
     ) -> Option<Event<'static>> {
-        if options.attach_stacktrace && event.exception.is_empty() {
+        if options.attach_stacktrace && !has_stacktrace(&event) {
             let thread = current_thread(true);
             if thread.stacktrace.is_some() {
                 event.threads.values.push(thread);
@@ -83,6 +83,12 @@ impl Integration for AttachStacktraceIntegration {
     }
 }
 
+fn has_stacktrace(event: &Event) -> bool {
+    event.stacktrace.is_some()
+        || event.exception.iter().any(|exc| exc.stacktrace.is_some())
+        || event.threads.iter().any(|thrd| thrd.stacktrace.is_some())
+}
+
 /// Captures information about the current thread.
 ///
 /// If `with_stack` is set to `true` the current stacktrace is


Test patch:
diff --git a/sentry/tests/test_basic.rs b/sentry/tests/test_basic.rs
index 33f5607..54bedc9 100644
--- a/sentry/tests/test_basic.rs
+++ b/sentry/tests/test_basic.rs
@@ -139,3 +139,38 @@ fn test_reentrant_configure_scope() {
     // well, the "outer" `configure_scope` wins
     assert_eq!(events[0].tags["which_scope"], "scope1");
 }
+
+#[test]
+fn test_attached_stacktrace() {
+    use log_ as log;
+
+    let logger = sentry_log::SentryLogger::new();
+
+    log::set_boxed_logger(Box::new(logger))
+        .map(|()| log::set_max_level(log::LevelFilter::Info))
+        .unwrap();
+
+    let options = sentry::apply_defaults(sentry::ClientOptions {
+        attach_stacktrace: true,
+        ..Default::default()
+    });
+    let events = sentry::test::with_captured_events_options(
+        || {
+            let error = "thisisnotanumber".parse::<u32>().unwrap_err();
+            sentry::capture_error(&error);
+
+            sentry::capture_message("some kind of message", sentry::Level::Info);
+
+            log::error!("Shit's on fire yo");
+        },
+        options,
+    );
+
+    assert_eq!(events.len(), 3);
+
+    let stacktraces: Vec<_> = events
+        .into_iter()
+        .flat_map(|ev| ev.threads.into_iter().filter_map(|thrd| thrd.stacktrace))
+        .collect();
+    assert_eq!(stacktraces.len(), 3);
+}