feat: API to add a stack trace to an exception or thread

[supervacuus]

Proposal for #721

[jan-auer]

An exception event object, or an exception object (not event)? The implementation assumes the latter, which also makes more sense.

[supervacuus]

You are right, it should be

The value needs to be either an exception or a thread object.

[supervacuus]

How do you typically proceed with open conversations? I usually wait until the reviewer thinks the comment has been addressed in a follow-up commit, signaling by resolving.

[supervacuus]

Hi @jan-auer and @Swatinem, here are a few points to consider before I would make this a non-draft PR:

sentry-native with this PR has 3 functions in the public interface concerned with creating and attaching stack traces to values (i.e. excluding the actual unwinding- and the option/configuration interfaces):

• sentry_event_value_add_stacktrace (deprecated here: feat: Add more Event Payload convenience methods #517),
• sentry_value_new_stacktrace (added also here feat: Add more Event Payload convenience methods #517) and
• sentry_value_add_stacktrace (from this PR).

Given the similarity of the latter two, I wondered whether there is a customer use case where the call-site will create a sentry_value_t from a stack trace and then doesn't immediately attach it to an exception or thread object? Would you prefer for this API to tend towards completeness (giving customers more freedom to handle values) or to reduce the chance of creating incoherent (for the backend) value structures?

Is the behavior inherited by sentry_value_set_key in case a "stacktrace" key is already present in the value argument acceptable, or should something more involved happen?

Depending a bit on the last question should the function be called sentry_value_attach_stacktrace or sentry_value_set_stacktrace (or some other name) instead of sentry_value_add_stacktrace?

[Swatinem]

I don’t really have a strong opinion on this. The API generally gives more freedom (and with great power comes great responsibility)

As for naming, maybe sentry_value_set_stacktrace is a better name indeed. If you want to make things a bit more clearer, you could as well make two (internally identical) functions sentry_exception_set_stacktrace and sentry_thread_set_stacktrace.

[supervacuus]

At this point, I am mostly trying to synchronize with the API idea for sentry-native that you folks have. Thanks for the feedback.