-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
feat: Capture Native Instruction Addrs for Exceptions #683
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Capture Native Instruction Addrs for Exceptions ([#683](https://github.com/getsentry/sentry-unity/pull/683)) If none of the above apply, you can opt out of this check by adding |
62faa65
to
c5b626c
Compare
I squashed all the commits, so this should be "ready to review", though there is still some work to do here. Most notably:
Especially the conditional compiler is something I need help with, as I have no idea how that would need to be done for Unity. Other than that, code is ready to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need a strategy to gracefully downgrade features depending on Unity versions. Will chat offline about it
} | ||
} | ||
|
||
// This is the same logic as `MainExceptionProcessor` uses to create the `SentryEvent.SentryExceptions` list. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior just changed in the .NET SDK: https://github.com/getsentry/sentry-dotnet/pull/1672/files
We should instead refactor this out so we use either one strategy or the other, and have the base behavior on some base class or helper method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good point. I would have loved to just have an iterator that yields exceptions.
The .NET SDK could just map over that iterator to yield sentry exceptions. And this here could use the exact same iterator to zip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
eb60fa4
to
13b8d24
Compare
…ry-unity into feat/native-stack
1f842bd
to
dc339c1
Compare
For each of the chained Exceptions, this will query the list of native instruction addrs for the Exception from the libil2cpp runtime, to decorate the Sentry Event with those instruction addresses.
The libil2cpp APIs available to do this are completely internal, but have been there for a few releases and should not break in the near future.
One limitation is that the APIs return only addresses from, and relative to, the main
GameAssembly
library. As far as Unity is concerned, the game will only ever consist of a single Binary/Library and will not be split up into multiple things.