getsentry / sentry-native

Sentry SDK for C, C++ and native applications.
MIT License
381 stars 164 forks source link

Allow inspecting event-id for crashpad events #779

Closed Swatinem closed 1 year ago

Swatinem commented 1 year ago

Usecase

A customer would want to collect user feedback for a crash. They are trying to use the on_crash hook to do that. However the user feedback API requires an event-id to attach the user feedback to.

Background / Proposal

Our crashpad backend currently has no way to get the event-id of the to-be-sent event, as that is created on the server when the minidump is received.

According to @jan-auer, relay will use whatever id is provided in the special __sentry_event file which currently contains serialized scope information and is sent alongside crashpad minidumps.

It should be possible to generate a uuid ahead of time and serialize that into the __sentry_event file. When calling our hooks and provide a dummy event, we can then at least fill out the event id property so that customers can use it.

There might be the problem that the event/minidump needs to be sent before user feedback can be posted? Not entirely sure how the ordering and timing there could be guaranteed?

A possible flow could be this:

supervacuus commented 1 year ago

the special __sentry_event file which currently contains serialized scope information and is sent alongside crashpad minidumps.

So this is the same __sentry-event (underscore vs. dash relevant?) we use to flush the scope in the crashpad backend? This happens on all scope setters, so we'd need to merge these if we produce an event-id in the crash handler, right?

I still wonder whether generating an empty event with an id to pass into the hooks would generally make sense since it would allow client code to enrich events in all backends.

barneygale commented 1 year ago

It should be possible to set the event ID as metadata when uploading the minidump, right?

Docs:

You can add more information to crash reports by merely adding more fields to the upload HTTP request. All these fields will be collected in the “Extra Data” section in Sentry

This works for me if I set a sentry[event_id] POST field to something like 4b025e680c044287a428d556f5787dfd.

Perhaps the event ID for the crash (if any) could be provided when the SDK is initialised? This would give the application a chance to write it to a file. In a subsequent session the app could check sentry_get_crashed_last_run(), read that file back to get the event ID, prompt the user for feedback, and then submit it.

supervacuus commented 1 year ago

Yes, @barneygale, the minidump endpoint can consume a full event payload. This is what happens with the other backends. The problem is that with Crashpad, the crashpad_handler process is making the request. There is currently no way to pass the event(-id) from the crashpad client (residing in the crashing process) to the crashpad_handler process. This proposal is one way to fix this.

barneygale commented 1 year ago

There is currently no way to pass the event(-id) from the crashpad client (residing in the crashing process) to the crashpad_handler process.

Could you supply the event ID _when you spawn the crashpad_handler process_ (i.e. before any crash has taken place)? Like:

sentry_options_set_crash_event_id(options, "[some precomputed ID]");
sentry_init(options);

The crash event ID may or may not be used, depending on whether the app crashes. But at least the app controls it and can stash it in a file, so that a subsequent run can elicit user feedback.

I'm not a C++ programmer so I'm probably missing something crucial. Thanks!

supervacuus commented 1 year ago

fixed in #843