diff --git a/CHANGELOG.md b/CHANGELOG.md index ec5a879a6..98da1ecff 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/modules/sentry-native b/modules/sentry-native index 179ee2b14..9eecb1bdf 160000 --- a/modules/sentry-native +++ b/modules/sentry-native @@ -1 +1 @@ -Subproject commit 179ee2b14c79a786fd1e75a696a05f95a8b59e77 +Subproject commit 9eecb1bdf2c7762ba7babe9ea437545a1f759bdc diff --git a/package-dev/Plugins/iOS/SentryNativeBridge.m b/package-dev/Plugins/iOS/SentryNativeBridge.m index 8c99f796e..4de55d26e 100644 --- a/package-dev/Plugins/iOS/SentryNativeBridge.m +++ b/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() { diff --git a/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs b/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs index 74a98dbea..e46cbc2e2 100644 --- a/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs +++ b/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() { { "foo", "bar" } }, BreadcrumbLevel.Critical); diff --git a/src/Sentry.Unity.Native/SentryNative.cs b/src/Sentry.Unity.Native/SentryNative.cs index a5005a404..5c5624a6b 100644 --- a/src/Sentry.Unity.Native/SentryNative.cs +++ b/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 /// public static class SentryNative { + private static Dictionary _perDirectoryCrashInfo = new Dictionary(); + /// /// Configures the native SDK. /// @@ -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(); diff --git a/src/Sentry.Unity.Native/SentryNativeBridge.cs b/src/Sentry.Unity.Native/SentryNativeBridge.cs index 37b0988bb..c9e862cd1 100644 --- a/src/Sentry.Unity.Native/SentryNativeBridge.cs +++ b/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 /// 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(); + /// /// Re-installs the sentry-native backend essentially retaking the signal handlers. /// diff --git a/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs b/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs index 45cee2766..0bc68e37a 100644 --- a/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs +++ b/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(); diff --git a/src/Sentry.Unity.iOS/SentryNativeIos.cs b/src/Sentry.Unity.iOS/SentryNativeIos.cs index 32d19075f..e9780262d 100644 --- a/src/Sentry.Unity.iOS/SentryNativeIos.cs +++ b/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); diff --git a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 b/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 index def8d51e9..5254a3f8c 100644 --- a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 +++ b/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" }