getsentry / sentry-native

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

Optionally allow the inproc handler to invoke the signal chain at the start #1026

Closed supervacuus closed 2 weeks ago

supervacuus commented 4 months ago

This is a new requirement from using the inproc backend in the dotnet runtime (via sentry-android). A detailed explanation of the behavior is here: https://github.com/dotnet/android/issues/9055#issuecomment-2261347912

There are multiple ways to implement this, but since this need is currently only coming from this particular use case and we are a transitive dependency via vanilla sentry-android, we can't expose it as a build parameter. So, I propose adding a boolean option to continue the signal chain at the start (rather than at the end) of the inproc handler. I could also imagine introducing an enum (instead of a boolean) to make this more generic and extensible (something like inproc_handler_strategy...).

I don't know the best approach to passing the option down (sentry-dotnet -> sentry-android -> sentry-native), so I would leave that aspect up to the downstream maintainers.

Another aspect that must be coordinated with downstream maintainers is the actual end-to-end test in a Maui app because my preliminary tests only ran with dotnet on Linux. I have no reason to believe that the runtime behaves differently on Android, but given that Android introduces quite a few offsets to "normal" Linux wrt to signal propagation, this should be verified by updating the sequence of downstream SDKs for an end-to-end test.

cc: @kahest, @bitsandfoxes, @jamescrosswell

jamescrosswell commented 4 months ago

I don't know the best approach to passing the option down (sentry-dotnet -> sentry-android -> sentry-native), so I would leave that aspect up to the downstream maintainers.

@kahest @bitsandfoxes we could pass it in via the SentryOptions when initialising the sentry-native integration...

bitsandfoxes commented 4 months ago

we could pass it in via the SentryOptions when initialising the sentry-native integration...

Sounds straight forward to me. We've got the native options ready to go.

kahest commented 3 months ago

fyi @markushi