Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions src/Sentry/GlobalSessionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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:
Expand All @@ -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;
}
Expand Down Expand Up @@ -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.HasPendingUnhandledException)
{
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:
Expand Down Expand Up @@ -364,4 +375,18 @@ public IReadOnlyList<SessionUpdate> 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);
}
}
2 changes: 2 additions & 0 deletions src/Sentry/ISessionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ internal interface ISessionManager
public IReadOnlyList<SessionUpdate> ResumeSession();

public SessionUpdate? ReportError();

public void MarkSessionAsUnhandled();
}
13 changes: 11 additions & 2 deletions src/Sentry/PersistedSessionUpdate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -26,14 +29,20 @@ public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
writer.WriteString("paused", pauseTimestamp);
}

if (PendingUnhandled)
{
writer.WriteBoolean("pendingUnhandled", PendingUnhandled);
}

writer.WriteEndObject();
}

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);
}
}
2 changes: 1 addition & 1 deletion src/Sentry/SentryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope)
{
case SentryEvent.ExceptionType.UnhandledNonTerminal:
_options.LogDebug("Ending session as 'Unhandled', due to non-terminal unhandled exception.");
scope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Unhandled);
_sessionManager.MarkSessionAsUnhandled();
break;

case SentryEvent.ExceptionType.Unhandled:
Expand Down
17 changes: 17 additions & 0 deletions src/Sentry/SentrySession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 bool _hasPendingUnhandledException;

/// <summary>
/// Gets whether this session has an unhandled exception that hasn't been finalized yet.
/// </summary>
internal bool HasPendingUnhandledException => _hasPendingUnhandledException;

internal SentrySession(
SentryId id,
string? distinctId,
Expand Down Expand Up @@ -74,6 +81,16 @@ public SentrySession(string? distinctId, string release, string? environment)
/// </summary>
public void ReportError() => Interlocked.Increment(ref _errorCount);

/// <summary>
/// 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.
/// </summary>
internal void MarkUnhandledException()
{
_hasPendingUnhandledException = true;
ReportError();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Error Count Incrementation Bug

The MarkUnhandledException method increments the session's error count on every call. This results in the error count increasing multiple times for a single unhandled state, rather than just once as intended.

Fix in Cursor Fix in Web


internal SessionUpdate CreateUpdate(
bool isInitial,
DateTimeOffset timestamp,
Expand Down
178 changes: 178 additions & 0 deletions test/Sentry.Tests/GlobalSessionManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,184 @@
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!.HasPendingUnhandledException.Should().BeTrue();
session.ErrorCount.Should().Be(1);

// 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 =>

Check failure on line 556 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-musl-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_NoActiveSession_LogsDebug

Expected _fixture.Logger.Entries { Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {empty}, Exception = <null>, Level = SentryLevel.Debug {value: 0}, Message = "There is no session active. Skipping marking session as unhandled." } } to have an item matching ((e.Message == "No active session to mark as unhandled.") AndAlso (Convert(e.Level, Int32) == 0)).

Check failure on line 556 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_NoActiveSession_LogsDebug

Expected _fixture.Logger.Entries { Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {empty}, Exception = <null>, Level = SentryLevel.Debug {value: 0}, Message = "There is no session active. Skipping marking session as unhandled." } } to have an item matching ((e.Message == "No active session to mark as unhandled.") AndAlso (Convert(e.Level, Int32) == 0)).

Check failure on line 556 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_NoActiveSession_LogsDebug

Expected _fixture.Logger.Entries { Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {empty}, Exception = <null>, Level = SentryLevel.Debug {value: 0}, Message = "There is no session active. Skipping marking session as unhandled." } } to have an item matching ((e.Message == "No active session to mark as unhandled.") AndAlso (Convert(e.Level, Int32) == 0)).
e.Message == "No active session to mark as unhandled." &&
e.Level == SentryLevel.Debug);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Session Logging Mismatch Causes Test Failure

The MarkSessionAsUnhandled method in GlobalSessionManager.cs logs "There is no session active. Skipping marking session as unhandled." when no session is active. This differs from the "No active session to mark as unhandled." message expected by the MarkSessionAsUnhandled_NoActiveSession_LogsDebug test, causing the test to fail.

Additional Locations (1)

Fix in Cursor Fix in Web

}

[Fact]
public void MarkSessionAsUnhandled_MultipleUnhandledExceptions_OnlyCountsFirstError()
{
// Arrange
var sut = _fixture.GetSut();
sut.StartSession();
var session = sut.CurrentSession;

// Act
sut.MarkSessionAsUnhandled();
sut.MarkSessionAsUnhandled();
sut.MarkSessionAsUnhandled();

// Assert
session!.ErrorCount.Should().Be(1);

Check failure on line 575 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-musl-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_MultipleUnhandledExceptions_OnlyCountsFirstError

Expected session!.ErrorCount to be 1, but found 3.

Check failure on line 575 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_MultipleUnhandledExceptions_OnlyCountsFirstError

Expected session!.ErrorCount to be 1, but found 3.

Check failure on line 575 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.MarkSessionAsUnhandled_MultipleUnhandledExceptions_OnlyCountsFirstError

Expected session!.ErrorCount to be 1, but found 3.
}

[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);

_fixture.Logger.Entries.Should().Contain(e =>

Check failure on line 617 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-musl-arm64)

Sentry.Tests.GlobalSessionManagerTests.TryRecoverPersistedSession_WithPendingUnhandledAndCrash_EscalatesToCrashed

Expected _fixture.Logger.Entries { Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {empty}, Exception = <null>, Level = SentryLevel.Debug {value: 0}, Message = "Attempting to recover persisted session from file." }, Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {SessionEndStatus.Crashed {value: 2}, <null>, True}, Exception = <null>, Level = SentryLevel.Info {value: 1}, Message = "Recovered session: EndStatus: {0}. PauseTimestamp: {1}. PendingUnhandled: {2}" } } to have an item matching (e.Message.Contains("PendingUnhandled: True") AndAlso (Convert(e.Level, Int32) == 1)).

Check failure on line 617 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.TryRecoverPersistedSession_WithPendingUnhandledAndCrash_EscalatesToCrashed

Expected _fixture.Logger.Entries { Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {empty}, Exception = <null>, Level = SentryLevel.Debug {value: 0}, Message = "Attempting to recover persisted session from file." }, Sentry.Testing.InMemoryDiagnosticLogger+Entry { Args = {SessionEndStatus.Crashed {value: 2}, <null>, True}, Exception = <null>, Level = SentryLevel.Info {value: 1}, Message = "Recovered session: EndStatus: {0}. PauseTimestamp: {1}. PendingUnhandled: {2}" } } to have an item matching (e.Message.Contains("PendingUnhandled: True") AndAlso (Convert(e.Level, Int32) == 1)).
e.Message.Contains("PendingUnhandled: True") &&
e.Level == SentryLevel.Info);
}

[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);
sessionUpdate.ErrorCount.Should().Be(1);
}

[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);

Check failure on line 675 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-musl-arm64)

Sentry.Tests.GlobalSessionManagerTests.EndSession_WithPendingUnhandledAndCrashedStatus_UsesCrashedStatus

Expected sessionUpdate.ErrorCount to be 1, but found 2.

Check failure on line 675 in test/Sentry.Tests/GlobalSessionManagerTests.cs

View workflow job for this annotation

GitHub Actions / .NET (linux-arm64)

Sentry.Tests.GlobalSessionManagerTests.EndSession_WithPendingUnhandledAndCrashedStatus_UsesCrashedStatus

Expected sessionUpdate.ErrorCount to be 1, but found 2.
}

[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.HasPendingUnhandledException.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(
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Sentry.Tests.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>$(CurrentTfms)</TargetFrameworks>
<TargetFrameworks>$(CurrentTfms);net48</TargetFrameworks>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Duplicate Framework Entry Causes Build Issues

On Windows, the net48 target framework is added twice to the TargetFrameworks list. This occurs because it's now unconditionally included, but also still conditionally added for Windows, which may lead to build issues or unexpected behavior.

Fix in Cursor Fix in Web

<TargetFrameworks Condition="'$(NO_ANDROID)' == ''">$(TargetFrameworks);$(LatestAndroidTfm);$(PreviousAndroidTfm)</TargetFrameworks>
<TargetFrameworks Condition="'$(NO_IOS)' == '' And $([MSBuild]::IsOSPlatform('OSX'))">$(TargetFrameworks);$(LatestIosTfm);$(PreviousIosTfm)</TargetFrameworks>
<TargetFrameworks Condition="'$(NO_MACCATALYST)' == '' And $([MSBuild]::IsOSPlatform('OSX'))">$(TargetFrameworks);$(LatestMacCatalystTfm);$(PreviousMacCatalystTfm)</TargetFrameworks>
Expand Down
Loading