Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow inspecting event-id for crashpad events #779

Closed
Swatinem opened this issue Dec 14, 2022 · 5 comments
Closed

Allow inspecting event-id for crashpad events #779

Swatinem opened this issue Dec 14, 2022 · 5 comments
Assignees

Comments

@Swatinem
Copy link
Member

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:

  • App triggers crash hook, customer extracts event id from dummy event object.
  • Customer spawns their own crash feedback gui using that event id.
  • Crash handling is continued and crashpad captures the crash and sends a minidump using the event id embedded in the __sentry_event file.
  • End user submits a feedback form about the crash using the event id.
@supervacuus
Copy link
Collaborator

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.

@supervacuus supervacuus self-assigned this Mar 21, 2023
@barneygale
Copy link

barneygale commented Apr 24, 2023

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
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

fixed in #843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants