sentry-native icon indicating copy to clipboard operation
sentry-native copied to clipboard

Pass EXCEPTION_POINTERS to before_send_func for crashpad backend

Open espkk opened this issue 3 years ago • 2 comments

Hi,

I'm wondering why ExceptionInfo is not passed to before_send_func as a second (hint) argument here: https://github.com/getsentry/sentry-native/blob/c3b7b07c99d19f39a4910093378e29bba86fde37/src/backends/sentry_backend_crashpad.cpp#L143 This could be useful for example to choose a cleanup strategy depending on the exception code when before_send_func is called. Is this by design or can be changed?

espkk avatar Sep 11 '21 13:09 espkk

As this callback is being invoked for every event (not only crashes), that wouldn’t easily be possible.

I think the best solution here would be to implement a dedicated on_crash hook, which does get the exception info, and signal info on unix platforms.

Swatinem avatar Sep 28 '21 08:09 Swatinem

As this callback is being invoked for every event (not only crashes), that wouldn’t easily be possible.

As far as I can see sentry__prepare_event and sentry__crashpad_handler always set hint to NULL. Since crashpad guarantees ExceptionInfo to contain exception pointers, testing the hint pointer could be used to differentiate the context. So I'm still thinking of it as the "cheapest" solution as it's just 1 line change. Also, that parameter can be used for holding some structure representing Unix signal data following the same considerations. The advantage of this approach is also that the context could be deduced with nullptr check using the same callback signature regardless of the platform.

Though, I agree that on_crash would be probably semantically cleaner.

Anyway, thank you for your answer! I will stick to my patched solution for now and wait for updates.

espkk avatar Sep 28 '21 09:09 espkk

We are currently investigating implementing a "before_crash"/"on_crash" hook.

ashwoods avatar Nov 10 '23 12:11 ashwoods

This was completed with https://github.com/getsentry/sentry-native/releases/tag/0.5.0.

The author of this issue (@espkk) implemented a considerable part of this feature.

supervacuus avatar Nov 10 '23 17:11 supervacuus