sentry-unreal
sentry-unreal copied to clipboard
getsentry
Fix invalid assertion issues grouping
This PR adds a custom FOutputDeviceError implementation which should allow to hook into Unreal's error handling flow and override assert reporting logic.
By default, an exception is raised whenever assert is fired, which is then captured by crashpad/breakpad backend. The topmost callstack frames for such events are always the same preventing them from being grouped properly and they all appear under a single issue at Sentry.
To avoid this situation instead of crashing the app a corresponding event can be constructed manually and reported via CaptureEvent. Once it's done the app is forced to shut down which somewhat repeats the original flow.
The current problem with the approach suggested here is that the engine's error handling is mixed up with some platform-specific logic (i.e. logging, restoring UI, etc.) which leads to skipping some parts of it when using the new FSentryOutputDeviceError.
Related issues: #489, #514, #503
On Android assertions are intercepted by FSentryOutputDeviceError which also sets several global tags used for further analysis during the beforeSend on the next application launch. Those tags allow us to identify whether the assertion occurred and how many call stack frames must be cut off. This change also fixes the #1
One more thing that comes to mind: We can mark those engine frames as InAppExclude so those won't get considered by the grouping algorithm. And those should be able to be propagated to the respective SDKs.
Since there are quite a few questions regarding the assertions/ensures capturing on Android I'll try to address these with the following explanation:
The general idea of this PR is to hook into Unreal's error handling process using a custom FSentryOutputDeviceError so that whenever assertion is fired we capture a manually created exception event with a proper callstack and shut down the application.
The problem with this approach on Android is that Java event's callstack doesn't contain any C++ frames and thus it's not informative at all. There's no straightforward way to convert a native callstack which we can obtain on Unreal's side to a corresponding Java structure to override the default one either. However, instead of reporting assert in a described way within a single session we can set some flags identifying that it happened and force the app to crash. This allows to pass things over to sentry-native which captures a native crash with a "normal" C++ callstack. Now on the next application launch in beforeSend handler we verify if the crash was an Unreal assertion by checking aforementioned flags and cut off the redundant callstack part if there's any. Also, these flags (if any) should be removed and not get sent to Sentry since we need them only for client-side event pre-processing.
The assertion-specific flags are set to an event via tags and not context since sentry-java can't serialize the latter for native crashes and in this case the required values are lost upon the next application launch. I believe this is a known limitation of Java SDK which we've encountered a couple of times in the past.