diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ff51f5c4b..511fffbfb9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Sentry/GlobalSessionManager.cs b/src/Sentry/GlobalSessionManager.cs index 38f2456c48..d60f6d4b3e 100644 --- a/src/Sentry/GlobalSessionManager.cs +++ b/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,7 +266,7 @@ 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, @@ -289,20 +274,27 @@ private void DeletePersistedSession() 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; } diff --git a/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs b/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs index a4c08c557b..ef85994e19 100644 --- a/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs +++ b/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs @@ -1,4 +1,5 @@ using System; +using Sentry.Extensibility; using Sentry.Internal; namespace Sentry.Integrations @@ -7,15 +8,15 @@ 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; } @@ -23,10 +24,12 @@ 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(); } } diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 91a7a49ba8..66f7ad1d77 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/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); } } diff --git a/test/Sentry.Tests/GlobalSessionManagerTests.cs b/test/Sentry.Tests/GlobalSessionManagerTests.cs index 5eb1a0c6a0..b3e9ff213a 100644 --- a/test/Sentry.Tests/GlobalSessionManagerTests.cs +++ b/test/Sentry.Tests/GlobalSessionManagerTests.cs @@ -21,7 +21,7 @@ private class Fixture : IDisposable public ISystemClock Clock { get; } - public Func PersistedSessionProvider { get; } + public Func PersistedSessionProvider { get; set; } public Fixture(Action 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); } }