Feat: native last crash (#615)

* feat: native CrashedLastRun()

* test: CrashedLastRun()

* fix: iOS CrashedLastRun()

* update sentry-native to the latest HEAD

* refactor: use native crashed_last_run()

* chore: update changelog

* ci: simple retry logic for the smoke-crash test

* fix: merge-related import duplication

* Apply suggestions from code review

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

CHANGELOG.md

@@ -10,9 +10,11 @@
 
 - iOS builds no longer break when native support disabled or not available ([#592](https://github.com/getsentry/sentry-unity/pull/592))
 - Close sentry instance when quitting the app ([#608](https://github.com/getsentry/sentry-unity/pull/608))
+- iOS options.CrashedLastRun() reported an incorrect value ([#615](https://github.com/getsentry/sentry-unity/pull/615))
 
 ### Features
 
+- Update native SDK to v0.4.16 (pre-release) ([#615](https://github.com/getsentry/sentry-unity/pull/615))
 - Windows native support (64-bit)
   - native crash handler ([#380](https://github.com/getsentry/sentry-unity/pull/380))
   - configuration & log forwarding ([#577](https://github.com/getsentry/sentry-unity/pull/577))

package-dev/Plugins/iOS/SentryNativeBridge.m

@@ -2,8 +2,8 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-bool CrashedLastRun() {
-    return[SentrySDK crashedLastRun];
+int CrashedLastRun() {
+    return [SentrySDK crashedLastRun] ? 1 : 0;
 }
 
 void Close() {

samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs

@@ -82,6 +82,10 @@ public void Start()
         {
             SmokeTestCrash();
         }
+        else if (arg == "post-crash")
+        {
+            PostCrashTest();
+        }
         else
         {
             Debug.Log($"Unknown command line argument: {arg}");
@@ -127,7 +131,10 @@ public static void SmokeTest()
         {
             Debug.Log("SMOKE TEST: Start");
 
-            InitSentry(new SentryUnityOptions() { CreateHttpClientHandler = () => t });
+            var options = new SentryUnityOptions() { CreateHttpClientHandler = () => t };
+            InitSentry(options);
+
+            t.Expect("options.CrashedLastRun() == false", !options.CrashedLastRun());
 
             var currentMessage = 0;
             t.ExpectMessage(currentMessage, "'type':'session'");
@@ -196,6 +203,16 @@ public static void SmokeTestCrash()
         Application.Quit(-1);
     }
 
+    public static void PostCrashTest()
+    {
+        var options = new SentryUnityOptions();
+        InitSentry(options);
+
+        var t = new TestHandler();
+        t.Expect("options.CrashedLastRun() == true", options.CrashedLastRun());
+        t.Pass();
+    }
+
     private static void AddContext()
     {
         SentrySdk.AddBreadcrumb("crumb", "bread", "error", new Dictionary<string, string>() { { "foo", "bar" } }, BreadcrumbLevel.Critical);

src/Sentry.Unity.Native/SentryNative.cs

@@ -1,5 +1,6 @@
 using Sentry.Extensibility;
 using Sentry.Unity.Integrations;
+using System.Collections.Generic;
 
 namespace Sentry.Unity.Native
 {
@@ -8,6 +9,8 @@ namespace Sentry.Unity.Native
     /// </summary>
     public static class SentryNative
     {
+        private static Dictionary<string, bool> _perDirectoryCrashInfo = new Dictionary<string, bool>();
+
         /// <summary>
         /// Configures the native SDK.
         /// </summary>
@@ -24,24 +27,28 @@ public static void Configure(SentryUnityOptions options)
                 };
                 options.ScopeObserver = new NativeScopeObserver(options);
                 options.EnableScopeSync = true;
-                // options.CrashedLastRun = () =>
-                // {
-                //     var crashedLastRun = SentryJava.CrashedLastRun();
-                //     if (crashedLastRun is null)
-                //     {
-                //         // Could happen if the Android SDK wasn't initialized before the .NET layer.
-                //         options.DiagnosticLogger?
-                //             .LogWarning("Unclear from the native SDK if the previous run was a crash. Assuming it was not.");
-                //         crashedLastRun = false;
-                //     }
-                //     else
-                //     {
-                //         options.DiagnosticLogger?
-                //             .LogDebug("Native Android SDK reported: 'crashedLastRun': '{0}'", crashedLastRun);
-                //     }
 
-                //     return crashedLastRun.Value;
-                // };
+                // Note: we must actually call the function now and on every other call use the value we get here.
+                // Additionally, we cannot call this multiple times for the same directory, because the result changes
+                // on subsequent runs. Therefore, we cache the value during the whole runtime of the application.
+                var cacheDirectory = SentryNativeBridge.GetCacheDirectory(options);
+                bool crashedLastRun = false;
+                // In the event the SDK is re-initialized with a different path on disk, we need to track which ones were already read
+                // Similarly we need to cache the value of each call since a subsequent call would return a different value
+                // as the file used on disk to mark it as crashed is deleted after we read it.
+                lock (_perDirectoryCrashInfo)
+                {
+                    if (!_perDirectoryCrashInfo.TryGetValue(cacheDirectory, out crashedLastRun))
+                    {
+                        crashedLastRun = SentryNativeBridge.HandleCrashedLastRun(options);
+                        _perDirectoryCrashInfo.Add(cacheDirectory, crashedLastRun);
+
+                        options.DiagnosticLogger?
+                            .LogDebug("Native SDK reported: 'crashedLastRun': '{0}'", crashedLastRun);
+                    }
+                }
+                options.CrashedLastRun = () => crashedLastRun;
+
                 // At this point Unity has taken the signal handler and will not invoke the original handler (Sentry)
                 // So we register our backend once more to make sure user-defined data is available in the crash report.
                 SentryNativeBridge.ReinstallBackend();

src/Sentry.Unity.Native/SentryNativeBridge.cs

@@ -3,7 +3,6 @@
 using System.Runtime.InteropServices;
 using Sentry.Extensibility;
 using AOT;
-using System.Threading;
 using System.Text;
 
 namespace Sentry.Unity
@@ -20,6 +19,9 @@ namespace Sentry.Unity
     /// <see href="https://github.com/getsentry/sentry-native"/>
     public static class SentryNativeBridge
     {
+
+        public static bool CrashedLastRun;
+
         public static void Init(SentryUnityOptions options)
         {
             var cOptions = sentry_options_new();
@@ -52,19 +54,16 @@ public static void Init(SentryUnityOptions options)
             options.DiagnosticLogger?.LogDebug("Disabling native auto session tracking");
             sentry_options_set_auto_session_tracking(cOptions, 0);
 
-            if (options.CacheDirectoryPath is not null)
+            var dir = GetCacheDirectory(options);
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath on Windows: {0}", dir);
+                sentry_options_set_database_pathw(cOptions, dir);
+            }
+            else
             {
-                var dir = Path.Combine(options.CacheDirectoryPath, "SentryNative");
-                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-                {
-                    options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath on Windows: {0}", dir);
-                    sentry_options_set_database_pathw(cOptions, dir);
-                }
-                else
-                {
-                    options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath: {0}", dir);
-                    sentry_options_set_database_path(cOptions, dir);
-                }
+                options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath: {0}", dir);
+                sentry_options_set_database_path(cOptions, dir);
             }
 
             if (options.DiagnosticLogger is null)
@@ -84,6 +83,28 @@ public static void Init(SentryUnityOptions options)
 
         public static void Close() => sentry_close();
 
+        /// Call after native init() to check if the application has crashed in the previous run and clear the status.
+        /// Because the file is removed, the result will change on subsequent calls so it must be cached for the current runtime.
+        internal static bool HandleCrashedLastRun(SentryUnityOptions options)
+        {
+            var result = sentry_get_crashed_last_run() == 1;
+            sentry_clear_crashed_last_run();
+            return result;
+        }
+
+        internal static string GetCacheDirectory(SentryUnityOptions options)
+        {
+            if (options.CacheDirectoryPath is null)
+            {
+                // same as the default of sentry-native
+                return Path.Combine(Directory.GetCurrentDirectory(), ".sentry-native");
+            }
+            else
+            {
+                return Path.Combine(options.CacheDirectoryPath, "SentryNative");
+            }
+        }
+
         // libsentry.so
         [DllImport("sentry")]
         private static extern IntPtr sentry_options_new();
@@ -172,6 +193,12 @@ private static void nativeLog(int cLevel, string message, IntPtr args, IntPtr us
         [DllImport("sentry")]
         private static extern int sentry_close();
 
+        [DllImport("sentry")]
+        private static extern int sentry_get_crashed_last_run();
+
+        [DllImport("sentry")]
+        private static extern int sentry_clear_crashed_last_run();
+
         /// <summary>
         /// Re-installs the sentry-native backend essentially retaking the signal handlers.
         /// </summary>

src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs

@@ -12,7 +12,7 @@ namespace Sentry.Unity.iOS
     internal static class SentryCocoaBridgeProxy
     {
         [DllImport("__Internal")]
-        public static extern bool CrashedLastRun();
+        public static extern int CrashedLastRun();
 
         [DllImport("__Internal")]
         public static extern void Close();

src/Sentry.Unity.iOS/SentryNativeIos.cs

@@ -20,7 +20,7 @@ public static void Configure(SentryUnityOptions options)
                 options.EnableScopeSync = true;
                 options.CrashedLastRun = () =>
                 {
-                    var crashedLastRun = SentryCocoaBridgeProxy.CrashedLastRun();
+                    var crashedLastRun = SentryCocoaBridgeProxy.CrashedLastRun() == 1;
                     options.DiagnosticLogger?
                         .LogDebug("Native iOS SDK reported: 'crashedLastRun': '{0}'", crashedLastRun);
 

test/Scripts.Integration.Test/integration-run-smoke-test.ps1

@@ -85,9 +85,6 @@ function RunTest([string] $type) {
     Else {
         $info = "Test process finished with status code $($process.ExitCode)."
         If ($type -ne "smoke-crash") {
-            if ("$AppDataDir" -ne "") {
-                Get-Content "$AppDataDir/Player.log"
-            }
             throw $info
         }
         Write-Host $info
@@ -125,7 +122,8 @@ if ($Smoke) {
 
 # Native crash test
 if ($Crash) {
-    $runs = 1   # You can increase this to run the crash test multiple times in a loop (until it fails)
+    # You can increase this to retry multiple times. Seems a bit flaky at the moment in CI.
+    $runs = 5
     for ($run = 1; $run -le $runs; $run++) {
         $httpServer = RunApiServer
         RunTest "smoke-crash"
@@ -134,8 +132,8 @@ if ($Crash) {
         $successMessage = "POST /api/12345/minidump/"
 
         Write-Host "Waiting for the expected message to appear in the server output logs ..."
-        # Wait for 1 minute (600 * 100 milliseconds) until the expected message comes in
-        for ($i = 0; $i -lt 600; $i++) {
+        # Wait for 30 seconds (300 * 100 milliseconds) until the expected message comes in
+        for ($i = 0; $i -lt 300; $i++) {
             $output = (Get-Content $httpServer.outFile -Raw) + (Get-Content $httpServer.errFile -Raw)
             if ("$output".Contains($successMessage)) {
                 break
@@ -167,8 +165,10 @@ if ($Crash) {
         if ($output.Contains($successMessage)) {
             Write-Host "smoke-crash test $run/$runs : PASSED" -ForegroundColor Green
         }
-        else {
+        elseif ($run -ne $runs) {
             Write-Error "smoke-crash test $run/$runs : FAILED"
         }
     }
+
+    RunTest "post-crash"
 }