diff --git a/CHANGELOG.md b/CHANGELOG.md index 4963e017c1..6f82912502 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ ### Features -- The SDK now makes use of the new SessionEndStatus `Unhandled` when capturing an unhandled but non-terminal exception, i.e. through the UnobservedTaskExceptionIntegration ([#4633](https://github.com/getsentry/sentry-dotnet/pull/4633)) +- The SDK now makes use of the new SessionEndStatus `Unhandled` when capturing an unhandled but non-terminal exception, i.e. through the UnobservedTaskExceptionIntegration ([#4633](https://github.com/getsentry/sentry-dotnet/pull/4633), [#4653](https://github.com/getsentry/sentry-dotnet/pull/4653)) ### Fixes diff --git a/src/Sentry/GlobalSessionManager.cs b/src/Sentry/GlobalSessionManager.cs index 29aa66b3e1..5f30c64909 100644 --- a/src/Sentry/GlobalSessionManager.cs +++ b/src/Sentry/GlobalSessionManager.cs @@ -41,7 +41,7 @@ public GlobalSessionManager( // Take pause timestamp directly instead of referencing _lastPauseTimestamp to avoid // potential race conditions. - private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp = null) + private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp = null, bool pendingUnhandled = false) { _options.LogDebug("Persisting session (SID: '{0}') to a file.", update.Id); @@ -69,7 +69,7 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp var filePath = Path.Combine(_persistenceDirectoryPath, PersistedSessionFileName); - var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp); + var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp, pendingUnhandled); if (!_options.FileSystem.CreateFileForWriting(filePath, out var file)) { _options.LogError("Failed to persist session file."); @@ -161,7 +161,10 @@ private void DeletePersistedSession() status = _options.CrashedLastRun?.Invoke() switch { // Native crash (if native SDK enabled): + // This takes priority - escalate to Crashed even if session had pending unhandled true => SessionEndStatus.Crashed, + // Had unhandled exception but didn't crash: + _ when recoveredUpdate.PendingUnhandled => SessionEndStatus.Unhandled, // Ended while on the background, healthy session: _ when recoveredUpdate.PauseTimestamp is not null => SessionEndStatus.Exited, // Possibly out of battery, killed by OS or user, solar flare: @@ -185,9 +188,10 @@ private void DeletePersistedSession() // If there's a callback for native crashes, check that first. status); - _options.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}", + _options.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}. PendingUnhandled: {2}", sessionUpdate.EndStatus, - recoveredUpdate.PauseTimestamp); + recoveredUpdate.PauseTimestamp, + recoveredUpdate.PendingUnhandled); return sessionUpdate; } @@ -245,6 +249,13 @@ private void DeletePersistedSession() private SessionUpdate EndSession(SentrySession session, DateTimeOffset timestamp, SessionEndStatus status) { + // If we're ending as 'Exited' but he session has a pending 'Unhandled', end as 'Unhandled' + if (status == SessionEndStatus.Exited && session.IsMarkedAsPendingUnhandled) + { + status = SessionEndStatus.Unhandled; + _options.LogDebug("Session ended as 'Unhandled' due to pending status."); + } + if (status == SessionEndStatus.Crashed) { // increments the errors count, as crashed sessions should report a count of 1 per: @@ -364,4 +375,18 @@ public IReadOnlyList ResumeSession() return session.CreateUpdate(false, _clock.GetUtcNow()); } + + public void MarkSessionAsUnhandled() + { + if (_currentSession is not { } session) + { + _options.LogDebug("There is no session active. Skipping marking session as unhandled."); + return; + } + + session.MarkUnhandledException(); + + var sessionUpdate = session.CreateUpdate(false, _clock.GetUtcNow()); + PersistSession(sessionUpdate, pendingUnhandled: true); + } } diff --git a/src/Sentry/ISessionManager.cs b/src/Sentry/ISessionManager.cs index b529a72b56..02e45e119b 100644 --- a/src/Sentry/ISessionManager.cs +++ b/src/Sentry/ISessionManager.cs @@ -17,4 +17,6 @@ internal interface ISessionManager public IReadOnlyList ResumeSession(); public SessionUpdate? ReportError(); + + public void MarkSessionAsUnhandled(); } diff --git a/src/Sentry/PersistedSessionUpdate.cs b/src/Sentry/PersistedSessionUpdate.cs index 8ca45ab03e..1caed46497 100644 --- a/src/Sentry/PersistedSessionUpdate.cs +++ b/src/Sentry/PersistedSessionUpdate.cs @@ -9,10 +9,13 @@ internal class PersistedSessionUpdate : ISentryJsonSerializable public DateTimeOffset? PauseTimestamp { get; } - public PersistedSessionUpdate(SessionUpdate update, DateTimeOffset? pauseTimestamp) + public bool PendingUnhandled { get; } + + public PersistedSessionUpdate(SessionUpdate update, DateTimeOffset? pauseTimestamp, bool pendingUnhandled = false) { Update = update; PauseTimestamp = pauseTimestamp; + PendingUnhandled = pendingUnhandled; } public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) @@ -26,6 +29,11 @@ public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) writer.WriteString("paused", pauseTimestamp); } + if (PendingUnhandled) + { + writer.WriteBoolean("pendingUnhandled", PendingUnhandled); + } + writer.WriteEndObject(); } @@ -33,7 +41,8 @@ public static PersistedSessionUpdate FromJson(JsonElement json) { var update = SessionUpdate.FromJson(json.GetProperty("update")); var pauseTimestamp = json.GetPropertyOrNull("paused")?.GetDateTimeOffset(); + var pendingUnhandled = json.GetPropertyOrNull("pendingUnhandled")?.GetBoolean() ?? false; - return new PersistedSessionUpdate(update, pauseTimestamp); + return new PersistedSessionUpdate(update, pauseTimestamp, pendingUnhandled); } } diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index d8cf1bb842..7e8f9a4111 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -351,8 +351,8 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope) switch (exceptionType) { case SentryEvent.ExceptionType.UnhandledNonTerminal: - _options.LogDebug("Ending session as 'Unhandled', due to non-terminal unhandled exception."); - scope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Unhandled); + _options.LogDebug("Marking session as 'Unhandled', due to non-terminal unhandled exception."); + _sessionManager.MarkSessionAsUnhandled(); break; case SentryEvent.ExceptionType.Unhandled: diff --git a/src/Sentry/SentrySession.cs b/src/Sentry/SentrySession.cs index 696c8f149e..e8141de21a 100644 --- a/src/Sentry/SentrySession.cs +++ b/src/Sentry/SentrySession.cs @@ -36,6 +36,13 @@ public class SentrySession : ISentrySession // Start at -1 so that the first increment puts it at 0 private int _sequenceNumber = -1; + private int _isMarkedAsPendingUnhandled; + + /// + /// Gets whether this session has an unhandled exception that hasn't been finalized yet. + /// + internal bool IsMarkedAsPendingUnhandled => _isMarkedAsPendingUnhandled != 0; + internal SentrySession( SentryId id, string? distinctId, @@ -74,6 +81,12 @@ public SentrySession(string? distinctId, string release, string? environment) /// public void ReportError() => Interlocked.Increment(ref _errorCount); + /// + /// Marks the session as having an unhandled exception without ending it. + /// This allows the session to continue and potentially escalate to Crashed if the app crashes. + /// + internal void MarkUnhandledException() => Interlocked.Exchange(ref _isMarkedAsPendingUnhandled, 1); + internal SessionUpdate CreateUpdate( bool isInitial, DateTimeOffset timestamp, diff --git a/test/Sentry.Tests/GlobalSessionManagerTests.cs b/test/Sentry.Tests/GlobalSessionManagerTests.cs index 1cfde77220..2649319a4f 100644 --- a/test/Sentry.Tests/GlobalSessionManagerTests.cs +++ b/test/Sentry.Tests/GlobalSessionManagerTests.cs @@ -523,6 +523,161 @@ public void TryRecoverPersistedSession_HasRecoveredUpdateAndCrashedLastRunFailed TryRecoverPersistedSessionWithExceptionOnLastRun(); } + [Fact] + public void MarkSessionAsUnhandled_ActiveSessionExists_MarksSessionAndPersists() + { + // Arrange + var sut = _fixture.GetSut(); + sut.StartSession(); + var session = sut.CurrentSession; + + // Act + sut.MarkSessionAsUnhandled(); + + // Assert + session.Should().NotBeNull(); + session!.IsMarkedAsPendingUnhandled.Should().BeTrue(); + + // Session should still be active (not ended) + sut.CurrentSession.Should().BeSameAs(session); + } + + [Fact] + public void MarkSessionAsUnhandled_NoActiveSession_LogsDebug() + { + // Arrange + var sut = _fixture.GetSut(); + + // Act + sut.MarkSessionAsUnhandled(); + + // Assert + _fixture.Logger.Entries.Should().Contain(e => + e.Message == "There is no session active. Skipping marking session as unhandled." && + e.Level == SentryLevel.Debug); + } + + [Fact] + public void TryRecoverPersistedSession_WithPendingUnhandledAndNoCrash_EndsAsUnhandled() + { + // Arrange + _fixture.Options.CrashedLastRun = () => false; + _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate( + AnySessionUpdate(), + pauseTimestamp: null, + pendingUnhandled: true); + + var sut = _fixture.GetSut(); + + // Act + var persistedSessionUpdate = sut.TryRecoverPersistedSession(); + + // Assert + persistedSessionUpdate.Should().NotBeNull(); + persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Unhandled); + } + + [Fact] + public void TryRecoverPersistedSession_WithPendingUnhandledAndCrash_EscalatesToCrashed() + { + // Arrange + _fixture.Options.CrashedLastRun = () => true; + _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate( + AnySessionUpdate(), + pauseTimestamp: null, + pendingUnhandled: true); + + var sut = _fixture.GetSut(); + + // Act + var persistedSessionUpdate = sut.TryRecoverPersistedSession(); + + // Assert + persistedSessionUpdate.Should().NotBeNull(); + persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed); + } + + [Fact] + public void TryRecoverPersistedSession_WithPendingUnhandledAndPauseTimestamp_EscalatesToCrashedIfCrashed() + { + // Arrange - Session was paused AND had pending unhandled, then crashed + _fixture.Options.CrashedLastRun = () => true; + var pausedTimestamp = DateTimeOffset.Now; + _fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate( + AnySessionUpdate(), + pausedTimestamp, + pendingUnhandled: true); + + var sut = _fixture.GetSut(); + + // Act + var persistedSessionUpdate = sut.TryRecoverPersistedSession(); + + // Assert + // Crash takes priority over all other end statuses + persistedSessionUpdate.Should().NotBeNull(); + persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed); + } + + [Fact] + public void EndSession_WithPendingUnhandledException_PreservesUnhandledStatus() + { + // Arrange + var sut = _fixture.GetSut(); + sut.StartSession(); + sut.MarkSessionAsUnhandled(); + + // Act - Try to end normally with Exited status + var sessionUpdate = sut.EndSession(SessionEndStatus.Exited); + + // Assert - Should be overridden to Unhandled + sessionUpdate.Should().NotBeNull(); + sessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Unhandled); + } + + [Fact] + public void EndSession_WithPendingUnhandledAndCrashedStatus_UsesCrashedStatus() + { + // Arrange + var sut = _fixture.GetSut(); + sut.StartSession(); + sut.MarkSessionAsUnhandled(); + + // Act - Explicitly end with Crashed status + var sessionUpdate = sut.EndSession(SessionEndStatus.Crashed); + + // Assert - Crashed status takes priority + sessionUpdate.Should().NotBeNull(); + sessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed); + sessionUpdate.ErrorCount.Should().Be(1); + } + + [Fact] + public void SessionEscalation_CompleteFlow_UnhandledThenCrash() + { + // Arrange - Simulate complete flow + var sut = _fixture.GetSut(); + sut.StartSession(); + var originalSessionId = sut.CurrentSession!.Id; + + // Act 1: Mark as unhandled (game encounters exception but continues) + sut.MarkSessionAsUnhandled(); + + // Assert: Session still active with pending flag + sut.CurrentSession.Should().NotBeNull(); + sut.CurrentSession!.Id.Should().Be(originalSessionId); + sut.CurrentSession.IsMarkedAsPendingUnhandled.Should().BeTrue(); + + // Act 2: Recover on next launch with crash detected + _fixture.Options.CrashedLastRun = () => true; + var recovered = sut.TryRecoverPersistedSession(); + + // Assert: Session escalated from Unhandled to Crashed + recovered.Should().NotBeNull(); + recovered!.EndStatus.Should().Be(SessionEndStatus.Crashed); + recovered.Id.Should().Be(originalSessionId); + } + // A session update (of which the state doesn't matter for the test): private static SessionUpdate AnySessionUpdate() => new( diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index 2be51cfd98..a1bfd471ac 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -1649,7 +1649,7 @@ public void CaptureEvent_ActiveSessionAndUnhandledException_SessionEndedAsCrashe } [Fact] - public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionEndedAsUnhandled() + public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionMarkedAsUnhandled() { // Arrange var client = _fixture.GetSut(); @@ -1660,6 +1660,6 @@ public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionEn client.CaptureEvent(new SentryEvent(exception)); // Assert - _fixture.SessionManager.Received().EndSession(SessionEndStatus.Unhandled); + _fixture.SessionManager.Received().MarkSessionAsUnhandled(); } }