fix: check crash callback for sessions

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Unity Android support: check for native crashes before closing session as Abnormal ([#1222](https://github.com/getsentry/sentry-dotnet/pull/1222))
+
 ## 3.9.3
 
 ### Fixes

src/Sentry/GlobalSessionManager.cs

@@ -61,8 +61,7 @@ public GlobalSessionManager(
 
                 Directory.CreateDirectory(directoryPath);
 
-                _options.DiagnosticLogger?.LogDebug(
-                    "Created directory for installation ID file ({0}).",
+                _options.DiagnosticLogger?.LogDebug("Created directory for installation ID file ({0}).",
                     directoryPath);
 
                 var filePath = Path.Combine(directoryPath, ".installation");
@@ -74,8 +73,7 @@ public GlobalSessionManager(
                 }
                 catch (FileNotFoundException)
                 {
-                    _options.DiagnosticLogger?.LogDebug(
-                        "File containing installation ID does not exist ({0}).",
+                    _options.DiagnosticLogger?.LogDebug("File containing installation ID does not exist ({0}).",
                         filePath);
                 }
                 catch (DirectoryNotFoundException)
@@ -90,8 +88,7 @@ public GlobalSessionManager(
                 var id = Guid.NewGuid().ToString();
                 File.WriteAllText(filePath, id);
 
-                _options.DiagnosticLogger?.LogDebug(
-                    "Saved installation ID '{0}' to file '{1}'.",
+                _options.DiagnosticLogger?.LogDebug("Saved installation ID '{0}' to file '{1}'.",
                     id, filePath);
 
                 return id;
@@ -100,9 +97,7 @@ public GlobalSessionManager(
             // and let the next installation id strategy kick in
             catch (Exception ex)
             {
-                _options.DiagnosticLogger?.LogError(
-                    "Failed to resolve persistent installation ID.",
-                    ex);
+                _options.DiagnosticLogger?.LogError("Failed to resolve persistent installation ID.", ex);
 
                 return null;
             }
@@ -133,9 +128,7 @@ public GlobalSessionManager(
             }
             catch (Exception ex)
             {
-                _options.DiagnosticLogger?.LogError(
-                    "Failed to resolve hardware installation ID.",
-                    ex);
+                _options.DiagnosticLogger?.LogError("Failed to resolve hardware installation ID.", ex);
 
                 return null;
             }
@@ -172,14 +165,11 @@ internal string GetMachineNameInstallationId() =>
 
                 if (!string.IsNullOrWhiteSpace(id))
                 {
-                    _options.DiagnosticLogger?.LogDebug(
-                        "Resolved installation ID '{0}'.",
-                        id);
+                    _options.DiagnosticLogger?.LogDebug("Resolved installation ID '{0}'.", id);
                 }
                 else
                 {
-                    _options.DiagnosticLogger?.LogDebug(
-                        "Failed to resolve installation ID.");
+                    _options.DiagnosticLogger?.LogDebug("Failed to resolve installation ID.");
                 }
 
                 return _resolvedInstallationId = id;
@@ -221,11 +211,9 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp
 
         private void DeletePersistedSession()
         {
-            _options.DiagnosticLogger?.LogDebug("Deleting persisted session file.");
-
             if (string.IsNullOrWhiteSpace(_persistenceDirectoryPath))
             {
-                _options.DiagnosticLogger?.LogDebug("Persistence directory is not set, returning.");
+                _options.DiagnosticLogger?.LogDebug("Persistence directory is not set, not deleting any persisted session file.");
                 return;
             }
 
@@ -237,8 +225,7 @@ private void DeletePersistedSession()
                 {
                     try
                     {
-                        _options.DiagnosticLogger?.LogDebug(
-                            "Deleting persisted session file with contents: {0}",
+                        _options.DiagnosticLogger?.LogDebug("Deleting persisted session file with contents: {0}",
                             File.ReadAllText(filePath));
                     }
                     catch (Exception ex)
@@ -252,9 +239,7 @@ private void DeletePersistedSession()
 
                 File.Delete(filePath);
 
-                _options.DiagnosticLogger?.LogInfo(
-                    "Deleted persisted session file '{0}'.",
-                    filePath);
+                _options.DiagnosticLogger?.LogInfo("Deleted persisted session file '{0}'.", filePath);
             }
             catch (Exception ex)
             {
@@ -281,28 +266,35 @@ private void DeletePersistedSession()
                 var recoveredUpdate = _persistedSessionProvider(filePath);
 
                 // Create a session update to end the recovered session
-                return new SessionUpdate(
+                var sessionUpdate = new SessionUpdate(
                     recoveredUpdate.Update,
                     // We're recovering an ongoing session, so this can never be initial
                     false,
                     // If the session was paused, then use that as timestamp, otherwise use current timestamp
                     recoveredUpdate.PauseTimestamp ?? _clock.GetUtcNow(),
                     // Increment sequence number
                     recoveredUpdate.Update.SequenceNumber + 1,
-                    // If the session was paused then end normally, otherwise abnormal or crashed
-                    _options.CrashedLastRun switch
+                    // If there's a callback for native crashes, check that first.
+                    _options.CrashedLastRun?.Invoke() switch
                     {
+                        // Native crash (if native SDK enabled):
+                        true => SessionEndStatus.Crashed,
+                        // Ended while on the background, healthy session:
                         _ when recoveredUpdate.PauseTimestamp is not null => SessionEndStatus.Exited,
-                        { } crashedLastRun => crashedLastRun() ? SessionEndStatus.Crashed : SessionEndStatus.Abnormal,
+                        // Possibly out of battery, killed by OS or user, solar flare:
                         _ => SessionEndStatus.Abnormal
                     });
+
+                _options.DiagnosticLogger?.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}",
+                    sessionUpdate.EndStatus,
+                    recoveredUpdate.PauseTimestamp);
+
+                return sessionUpdate;
             }
             catch (IOException ioEx) when (ioEx is FileNotFoundException or DirectoryNotFoundException)
             {
                 // Not a notable error
-                _options.DiagnosticLogger?.LogDebug(
-                    "A persisted session does not exist at {0}.",
-                    filePath);
+                _options.DiagnosticLogger?.LogDebug("A persisted session does not exist at {0}.", filePath);
 
                 return null;
             }
@@ -348,8 +340,7 @@ private void DeletePersistedSession()
                 EndSession(previousSession, _clock.GetUtcNow(), SessionEndStatus.Exited);
             }
 
-            _options.DiagnosticLogger?.LogInfo(
-                "Started new session (SID: {0}; DID: {1}).",
+            _options.DiagnosticLogger?.LogInfo("Started new session (SID: {0}; DID: {1}).",
                 session.Id, session.DistinctId);
 
             var update = session.CreateUpdate(true, _clock.GetUtcNow());
@@ -361,8 +352,7 @@ private void DeletePersistedSession()
 
         private SessionUpdate EndSession(Session session, DateTimeOffset timestamp, SessionEndStatus status)
         {
-            _options.DiagnosticLogger?.LogInfo(
-                "Ended session (SID: {0}; DID: {1}) with status '{2}'.",
+            _options.DiagnosticLogger?.LogInfo("Ended session (SID: {0}; DID: {1}) with status '{2}'.",
                 session.Id, session.DistinctId, status);
 
             var update = session.CreateUpdate(false, timestamp, status);
@@ -377,8 +367,7 @@ private SessionUpdate EndSession(Session session, DateTimeOffset timestamp, Sess
             var session = Interlocked.Exchange(ref _currentSession, null);
             if (session is null)
             {
-                _options.DiagnosticLogger?.LogDebug(
-                    "Failed to end session because there is none active.");
+                _options.DiagnosticLogger?.LogDebug("Failed to end session because there is none active.");
 
                 return null;
             }

src/Sentry/Integrations/AppDomainProcessExitIntegration.cs

@@ -1,4 +1,5 @@
 using System;
+using Sentry.Extensibility;
 using Sentry.Internal;
 
 namespace Sentry.Integrations
@@ -7,26 +8,28 @@ internal class AppDomainProcessExitIntegration : IInternalSdkIntegration
     {
         private readonly IAppDomain _appDomain;
         private IHub? _hub;
+        private SentryOptions? _options;
 
         public AppDomainProcessExitIntegration(IAppDomain? appDomain = null)
-        {
-            _appDomain = appDomain ?? AppDomainAdapter.Instance;
-        }
+            => _appDomain = appDomain ?? AppDomainAdapter.Instance;
 
         public void Register(IHub hub, SentryOptions options)
         {
             _hub = hub;
+            _options = options;
             _appDomain.ProcessExit += HandleProcessExit;
         }
 
         public void Unregister(IHub hub)
         {
             _appDomain.ProcessExit -= HandleProcessExit;
             _hub = null;
+            _options = null;
         }
 
         internal void HandleProcessExit(object? sender, EventArgs e)
         {
+            _options?.DiagnosticLogger?.LogInfo("AppDomain process exited: Disposing SDK.");
             (_hub as IDisposable)?.Dispose();
         }
     }

src/Sentry/Internal/Hub.cs

@@ -183,9 +183,7 @@ public void StartSession()
                 }
                 catch (Exception ex)
                 {
-                    _options.DiagnosticLogger?.LogError(
-                        "Failed to recover persisted session.",
-                        ex);
+                    _options.DiagnosticLogger?.LogError("Failed to recover persisted session.", ex);
                 }
             }
 
@@ -200,9 +198,7 @@ public void StartSession()
             }
             catch (Exception ex)
             {
-                _options.DiagnosticLogger?.LogError(
-                    "Failed to start a session.",
-                    ex);
+                _options.DiagnosticLogger?.LogError("Failed to start a session.", ex);
             }
         }
 
@@ -216,9 +212,7 @@ public void PauseSession()
                 }
                 catch (Exception ex)
                 {
-                    _options.DiagnosticLogger?.LogError(
-                        "Failed to pause a session.",
-                        ex);
+                    _options.DiagnosticLogger?.LogError("Failed to pause a session.", ex);
                 }
             }
         }
@@ -236,9 +230,7 @@ public void ResumeSession()
                 }
                 catch (Exception ex)
                 {
-                    _options.DiagnosticLogger?.LogError(
-                        "Failed to resume a session.",
-                        ex);
+                    _options.DiagnosticLogger?.LogError("Failed to resume a session.", ex);
                 }
             }
         }
@@ -255,9 +247,7 @@ private void EndSession(DateTimeOffset timestamp, SessionEndStatus status)
             }
             catch (Exception ex)
             {
-                _options.DiagnosticLogger?.LogError(
-                    "Failed to end a session.",
-                    ex);
+                _options.DiagnosticLogger?.LogError("Failed to end a session.", ex);
             }
         }
 

test/Sentry.Tests/GlobalSessionManagerTests.cs

@@ -21,7 +21,7 @@ private class Fixture : IDisposable
 
             public ISystemClock Clock { get; }
 
-            public Func<string, PersistedSessionUpdate> PersistedSessionProvider { get; }
+            public Func<string, PersistedSessionUpdate> PersistedSessionProvider { get; set; }
 
             public Fixture(Action<SentryOptions> configureOptions = null)
             {
@@ -355,6 +355,71 @@ public void TryGetPersistentInstallationId_SessionStarted_CrashDelegateReturnsFa
             persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Abnormal);
         }
 
+        [Fact]
+        public void TryGetPersistentInstallationId_CrashDelegateReturnsTrueWithPauseTimestamp_EndsAsCrashed()
+        {
+            // Arrange
+            _fixture.Options.CrashedLastRun = () => true;
+            // Session was paused before persisted:
+            var pausedTimestamp = DateTimeOffset.Now;
+            _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
+                AnySessionUpdate(),
+                pausedTimestamp);
+
+            var sut = _fixture.GetSut();
+
+            // Act
+            var persistedSessionUpdate = sut.TryRecoverPersistedSession();
+
+            // Assert
+            persistedSessionUpdate.Should().NotBeNull();
+            persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed);
+        }
+
+        [Fact]
+        public void TryGetPersistentInstallationId_CrashDelegateIsNullWithPauseTimestamp_EndsAsExited()
+        {
+            // Arrange
+            _fixture.Options.CrashedLastRun = null;
+            // Session was paused before persisted:
+            var pausedTimestamp = DateTimeOffset.Now;
+            _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
+                AnySessionUpdate(),
+                pausedTimestamp);
+
+            var sut = _fixture.GetSut();
+
+            // Act
+            var persistedSessionUpdate = sut.TryRecoverPersistedSession();
+
+            // Assert
+            persistedSessionUpdate.Should().NotBeNull();
+            persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Exited);
+        }
+
+        [Fact]
+        public void TryGetPersistentInstallationId_CrashDelegateIsNullWithoutPauseTimestamp_EndsAsAbnormal()
+        {
+            // Arrange
+            _fixture.Options.CrashedLastRun = null;
+            var pausedTimestamp = DateTimeOffset.Now;
+            _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
+                AnySessionUpdate(),
+                // No pause timestamp:
+                null);
+
+            var sut = _fixture.GetSut();
+
+            sut.StartSession();
+
+            // Act
+            var persistedSessionUpdate = sut.TryRecoverPersistedSession();
+
+            // Assert
+            persistedSessionUpdate.Should().NotBeNull();
+            persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Abnormal);
+        }
+
         [Fact]
         public void TryGetPersistentInstallationId_SessionStarted_CrashDelegateReturnsTrue_EndsAsCrashed()
         {
@@ -407,5 +472,21 @@ public void TryGetPersistentInstallationId_SessionEnded_ReturnsNull()
             // Assert
             persistedSessionUpdate.Should().BeNull();
         }
+
+        // A session update (of which the state doesn't matter for the test):
+        private static SessionUpdate AnySessionUpdate()
+            => new(
+                SentryId.Create(),
+                "did",
+                DateTimeOffset.Now,
+                "release",
+                "env",
+                "ip",
+                "ua",
+                0,
+                true,
+                DateTimeOffset.Now,
+                1,
+                null);
     }
 }