Adding exception details appears to mess up the report on Sentry

[mcdurdin]

I'm not clear if this should be added here or in another location. Please move the issue as necessary.

We're manually reporting exceptions in our app by adding in the exception value when sending an event, as described at https://docs.sentry.io/platforms/native/#exceptions:

    exc = sentry_value_new_object();
    sentry_value_set_by_key(exc, "type", sentry_value_new_string(ExceptionClassName));
    sentry_value_set_by_key(exc, "value", sentry_value_new_string(Message));
    sentry_value_set_by_key(event, "exception", exc);

As soon as we do this, the report on Sentry ends up formatted wrongly, losing visibility of the call stack:

Using sentry-native 0.2.3. This issue arises both on sentry.io and our on premise sentry instance.

Originally raised at https://forum.sentry.io/t/corrupted-display-when-exception-data-is-set-using-native-sdk/9167/3

[Swatinem]

Interesting. Can you try with

sentry_event_value_add_stacktrace(event, NULL, 0);

I haven’t played around with that case specifically, I can try it later, though I’m tied up with something else right now.

[mcdurdin]

sentry_event_value_add_stacktrace(event, NULL, 0);

Yes, we do that already as well (after adding the exception data). Sorry I didn't note that initially.

[augustjd]

Still seeing this issue on 0.3.0 + sentry.io. Weirdly - if you go into the JSON for the event, you can see that source mapping worked, but it's not visible on the page in FULL mode.

[eakoli]

@mcdurdin Were attempting to do the same thing, it looks like you have to have the stack trace as part of the exception object for it to show up.

{ 
    name : "std::exception",
    value : "what() -> 'invalid state'",
    stacktrace : {
        frames: [
            { .. }
       ]
    }
}

were doing this via sentry_event_value_add_stacktrace like so:

sentry_value_t trace = sentry_value_new_object();
sentry_event_value_add_stacktrace( trace, NULL, 0 );
// should have threads.values[0]
sentry_value_t threads = sentry_value_get_by_key( trace, "threads" );
sentry_value_t thread_values = sentry_value_get_by_key( threads, "values" );
sentry_value_t thread = sentry_value_get_by_index( thread_values, 0 );
sentry_value_t stacktrace = sentry_value_get_by_key( thread, "stacktrace" );

sentry_value_incref( stacktrace );
sentry_value_set_by_key( exception, "stacktrace", stacktrace );
sentry_value_decref( trace );

[mcdurdin]

@eakoli Interesting. @Swatinem, does this sound like what we should be doing?

[Swatinem]

Yes, from an event payloads perspective, the stack trace can either be in threads or on the exception, but I think in this case you would rather want to have it on the exception. The API is a bit misleading here.

[mcdurdin]

Okay, thanks @eakoli and @Swatinem -- that's really helpful for us.

[Jake-Shadle]

I should have read the issues on this repo more closely as I ran into this same thing but ended up just adding a function to add a stacktrace to a value (in my case, the exception object) before adding that value to the event itself, so in Rust it looks something like

let exc = sentry_native::Map::new();
exc.insert("type", "panic");
exc.insert("value", msg);
exc.add_stacktrace(0);

let eve = Event::new();
eve.insert("level", "fatal");
eve.insert("exception", exc);
eve.capture();

void
sentry_value_add_stacktrace(sentry_value_t val, size_t len) {
    void *walked_backtrace[256];

    len = sentry_unwind_stack(NULL, walked_backtrace, 256);

    sentry_value_t frames = sentry__value_new_list_with_size(len);
    for (size_t i = 0; i < len; i++) {
        sentry_value_t frame = sentry_value_new_object();
        sentry_value_set_by_key(frame, "instruction_addr",
            sentry__value_new_addr((uint64_t)walked_backtrace[len - i - 1]));
        sentry_value_append(frames, frame);
    }

    sentry_value_t stacktrace = sentry_value_new_object();
    sentry_value_set_by_key(stacktrace, "frames", frames);

    sentry_value_set_by_key(val, "stacktrace", stacktrace);
}

[Swatinem]

@Jake-Shadle just commented on your rust PR.

I think the best thing to do here would be to deprecate sentry_event_value_add_stacktrace in favor of two new functions:

• sentry_event_value_add_thread_stacktrace
• sentry_event_value_add_exception_stacktrace

I think the naming here is quite clear. While yes, events are also values, the stack trace interface is part of an event, so we should be explicit with the event_value naming here.

[Jake-Shadle]

Yah, that sounds like it would be fine, trust me, totally aware this was a total hack. ;)

[eakoli]

@Swatinem An issue I see with sentry_event_value_add_exception_stacktrace would be that implies there can only be one exception logged per event.

I have code im using in our sentry integration (haven't made into a PR yet) that adds a sentry_value_t sentry__value_new_stacktrace( void**, size_t _len ) to ease creation of stack traces for wherever they would need to be added

[Swatinem]

An issue I see with sentry_event_value_add_exception_stacktrace would be that implies there can only be one exception logged per event.

That is true. I will discuss this internally to see what would make the most sense here.

adds a sentry_value_t sentry__value_new_stacktrace( void**, size_t _len ) to ease creation of stack traces for wherever they would need to be added

Great idea!