Skip to content

Commit 8561566

Browse files
bitsandfoxesbitsandfoxes
authored
feat: Cache Unhandled session instead of sending it immediately (#4653)
1 parent ae09873 commit 8561566

File tree

8 files changed

+218
-11
lines changed

8 files changed

+218
-11
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
### Features
2828

29+
- 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))
2930
- 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))
3031

3132
### Fixes

‎src/Sentry/GlobalSessionManager.cs‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public GlobalSessionManager(
4141

4242
// Take pause timestamp directly instead of referencing _lastPauseTimestamp to avoid
4343
// potential race conditions.
44-
private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp = null)
44+
private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp = null, bool pendingUnhandled = false)
4545
{
4646
_options.LogDebug("Persisting session (SID: '{0}') to a file.", update.Id);
4747

@@ -69,7 +69,7 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp
6969

7070
var filePath = Path.Combine(_persistenceDirectoryPath, PersistedSessionFileName);
7171

72-
var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp);
72+
var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp, pendingUnhandled);
7373
if (!_options.FileSystem.CreateFileForWriting(filePath, out var file))
7474
{
7575
_options.LogError("Failed to persist session file.");
@@ -161,7 +161,10 @@ private void DeletePersistedSession()
161161
status = _options.CrashedLastRun?.Invoke() switch
162162
{
163163
// Native crash (if native SDK enabled):
164+
// This takes priority - escalate to Crashed even if session had pending unhandled
164165
true => SessionEndStatus.Crashed,
166+
// Had unhandled exception but didn't crash:
167+
_ when recoveredUpdate.PendingUnhandled => SessionEndStatus.Unhandled,
165168
// Ended while on the background, healthy session:
166169
_ when recoveredUpdate.PauseTimestamp is not null => SessionEndStatus.Exited,
167170
// Possibly out of battery, killed by OS or user, solar flare:
@@ -185,9 +188,10 @@ private void DeletePersistedSession()
185188
// If there's a callback for native crashes, check that first.
186189
status);
187190

188-
_options.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}",
191+
_options.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}. PendingUnhandled: {2}",
189192
sessionUpdate.EndStatus,
190-
recoveredUpdate.PauseTimestamp);
193+
recoveredUpdate.PauseTimestamp,
194+
recoveredUpdate.PendingUnhandled);
191195

192196
return sessionUpdate;
193197
}
@@ -245,6 +249,13 @@ private void DeletePersistedSession()
245249

246250
private SessionUpdate EndSession(SentrySession session, DateTimeOffset timestamp, SessionEndStatus status)
247251
{
252+
// If we're ending as 'Exited' but he session has a pending 'Unhandled', end as 'Unhandled'
253+
if (status == SessionEndStatus.Exited && session.IsMarkedAsPendingUnhandled)
254+
{
255+
status = SessionEndStatus.Unhandled;
256+
_options.LogDebug("Session ended with pending 'Unhandled' (but not `Terminal`) exception.");
257+
}
258+
248259
if (status == SessionEndStatus.Crashed)
249260
{
250261
// increments the errors count, as crashed sessions should report a count of 1 per:
@@ -288,7 +299,7 @@ public void PauseSession()
288299

289300
var now = _clock.GetUtcNow();
290301
_lastPauseTimestamp = now;
291-
PersistSession(session.CreateUpdate(false, now), now);
302+
PersistSession(session.CreateUpdate(false, now), now, session.IsMarkedAsPendingUnhandled);
292303
}
293304

294305
public IReadOnlyList<SessionUpdate> ResumeSession()
@@ -364,4 +375,18 @@ public IReadOnlyList<SessionUpdate> ResumeSession()
364375

365376
return session.CreateUpdate(false, _clock.GetUtcNow());
366377
}
378+
379+
public void MarkSessionAsUnhandled()
380+
{
381+
if (_currentSession is not { } session)
382+
{
383+
_options.LogDebug("There is no session active. Skipping marking session as unhandled.");
384+
return;
385+
}
386+
387+
session.MarkUnhandledException();
388+
389+
var sessionUpdate = session.CreateUpdate(false, _clock.GetUtcNow());
390+
PersistSession(sessionUpdate, pendingUnhandled: true);
391+
}
367392
}

‎src/Sentry/ISessionManager.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,6 @@ internal interface ISessionManager
1717
public IReadOnlyList<SessionUpdate> ResumeSession();
1818

1919
public SessionUpdate? ReportError();
20+
21+
public void MarkSessionAsUnhandled();
2022
}

‎src/Sentry/PersistedSessionUpdate.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@ internal class PersistedSessionUpdate : ISentryJsonSerializable
99

1010
public DateTimeOffset? PauseTimestamp { get; }
1111

12-
public PersistedSessionUpdate(SessionUpdate update, DateTimeOffset? pauseTimestamp)
12+
public bool PendingUnhandled { get; }
13+
14+
public PersistedSessionUpdate(SessionUpdate update, DateTimeOffset? pauseTimestamp, bool pendingUnhandled = false)
1315
{
1416
Update = update;
1517
PauseTimestamp = pauseTimestamp;
18+
PendingUnhandled = pendingUnhandled;
1619
}
1720

1821
public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
@@ -26,14 +29,20 @@ public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
2629
writer.WriteString("paused", pauseTimestamp);
2730
}
2831

32+
if (PendingUnhandled)
33+
{
34+
writer.WriteBoolean("pendingUnhandled", PendingUnhandled);
35+
}
36+
2937
writer.WriteEndObject();
3038
}
3139

3240
public static PersistedSessionUpdate FromJson(JsonElement json)
3341
{
3442
var update = SessionUpdate.FromJson(json.GetProperty("update"));
3543
var pauseTimestamp = json.GetPropertyOrNull("paused")?.GetDateTimeOffset();
44+
var pendingUnhandled = json.GetPropertyOrNull("pendingUnhandled")?.GetBoolean() ?? false;
3645

37-
return new PersistedSessionUpdate(update, pauseTimestamp);
46+
return new PersistedSessionUpdate(update, pauseTimestamp, pendingUnhandled);
3847
}
3948
}

‎src/Sentry/SentryClient.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,8 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope)
351351
switch (exceptionType)
352352
{
353353
case SentryEvent.ExceptionType.UnhandledNonTerminal:
354-
_options.LogDebug("Ending session as 'Unhandled', due to non-terminal unhandled exception.");
355-
scope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Unhandled);
354+
_options.LogDebug("Marking session as 'Unhandled', due to non-terminal unhandled exception.");
355+
_sessionManager.MarkSessionAsUnhandled();
356356
break;
357357

358358
case SentryEvent.ExceptionType.UnhandledTerminal:

‎src/Sentry/SentrySession.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using Sentry.Internal;
2+
13
namespace Sentry;
24

35
/// <summary>
@@ -36,6 +38,13 @@ public class SentrySession : ISentrySession
3638
// Start at -1 so that the first increment puts it at 0
3739
private int _sequenceNumber = -1;
3840

41+
private InterlockedBoolean _isMarkedAsPendingUnhandled;
42+
43+
/// <summary>
44+
/// Gets whether this session has an unhandled exception that hasn't been finalized yet.
45+
/// </summary>
46+
internal bool IsMarkedAsPendingUnhandled => _isMarkedAsPendingUnhandled;
47+
3948
internal SentrySession(
4049
SentryId id,
4150
string? distinctId,
@@ -74,6 +83,12 @@ public SentrySession(string? distinctId, string release, string? environment)
7483
/// </summary>
7584
public void ReportError() => Interlocked.Increment(ref _errorCount);
7685

86+
/// <summary>
87+
/// Marks the session as having an unhandled exception without ending it.
88+
/// This allows the session to continue and potentially escalate to Crashed if the app crashes.
89+
/// </summary>
90+
internal void MarkUnhandledException() => _isMarkedAsPendingUnhandled.Exchange(true);
91+
7792
internal SessionUpdate CreateUpdate(
7893
bool isInitial,
7994
DateTimeOffset timestamp,

‎test/Sentry.Tests/GlobalSessionManagerTests.cs‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,161 @@ public void TryRecoverPersistedSession_HasRecoveredUpdateAndCrashedLastRunFailed
523523
TryRecoverPersistedSessionWithExceptionOnLastRun();
524524
}
525525

526+
[Fact]
527+
public void MarkSessionAsUnhandled_ActiveSessionExists_MarksSessionAndPersists()
528+
{
529+
// Arrange
530+
var sut = _fixture.GetSut();
531+
sut.StartSession();
532+
var session = sut.CurrentSession;
533+
534+
// Act
535+
sut.MarkSessionAsUnhandled();
536+
537+
// Assert
538+
session.Should().NotBeNull();
539+
session!.IsMarkedAsPendingUnhandled.Should().BeTrue();
540+
541+
// Session should still be active (not ended)
542+
sut.CurrentSession.Should().BeSameAs(session);
543+
}
544+
545+
[Fact]
546+
public void MarkSessionAsUnhandled_NoActiveSession_LogsDebug()
547+
{
548+
// Arrange
549+
var sut = _fixture.GetSut();
550+
551+
// Act
552+
sut.MarkSessionAsUnhandled();
553+
554+
// Assert
555+
_fixture.Logger.Entries.Should().Contain(e =>
556+
e.Message == "There is no session active. Skipping marking session as unhandled." &&
557+
e.Level == SentryLevel.Debug);
558+
}
559+
560+
[Fact]
561+
public void TryRecoverPersistedSession_WithPendingUnhandledAndNoCrash_EndsAsUnhandled()
562+
{
563+
// Arrange
564+
_fixture.Options.CrashedLastRun = () => false;
565+
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
566+
AnySessionUpdate(),
567+
pauseTimestamp: null,
568+
pendingUnhandled: true);
569+
570+
var sut = _fixture.GetSut();
571+
572+
// Act
573+
var persistedSessionUpdate = sut.TryRecoverPersistedSession();
574+
575+
// Assert
576+
persistedSessionUpdate.Should().NotBeNull();
577+
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Unhandled);
578+
}
579+
580+
[Fact]
581+
public void TryRecoverPersistedSession_WithPendingUnhandledAndCrash_EscalatesToCrashed()
582+
{
583+
// Arrange
584+
_fixture.Options.CrashedLastRun = () => true;
585+
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
586+
AnySessionUpdate(),
587+
pauseTimestamp: null,
588+
pendingUnhandled: true);
589+
590+
var sut = _fixture.GetSut();
591+
592+
// Act
593+
var persistedSessionUpdate = sut.TryRecoverPersistedSession();
594+
595+
// Assert
596+
persistedSessionUpdate.Should().NotBeNull();
597+
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed);
598+
}
599+
600+
[Fact]
601+
public void TryRecoverPersistedSession_WithPendingUnhandledAndPauseTimestamp_EscalatesToCrashedIfCrashed()
602+
{
603+
// Arrange - Session was paused AND had pending unhandled, then crashed
604+
_fixture.Options.CrashedLastRun = () => true;
605+
var pausedTimestamp = DateTimeOffset.Now;
606+
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
607+
AnySessionUpdate(),
608+
pausedTimestamp,
609+
pendingUnhandled: true);
610+
611+
var sut = _fixture.GetSut();
612+
613+
// Act
614+
var persistedSessionUpdate = sut.TryRecoverPersistedSession();
615+
616+
// Assert
617+
// Crash takes priority over all other end statuses
618+
persistedSessionUpdate.Should().NotBeNull();
619+
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed);
620+
}
621+
622+
[Fact]
623+
public void EndSession_WithPendingUnhandledException_PreservesUnhandledStatus()
624+
{
625+
// Arrange
626+
var sut = _fixture.GetSut();
627+
sut.StartSession();
628+
sut.MarkSessionAsUnhandled();
629+
630+
// Act - Try to end normally with Exited status
631+
var sessionUpdate = sut.EndSession(SessionEndStatus.Exited);
632+
633+
// Assert - Should be overridden to Unhandled
634+
sessionUpdate.Should().NotBeNull();
635+
sessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Unhandled);
636+
}
637+
638+
[Fact]
639+
public void EndSession_WithPendingUnhandledAndCrashedStatus_UsesCrashedStatus()
640+
{
641+
// Arrange
642+
var sut = _fixture.GetSut();
643+
sut.StartSession();
644+
sut.MarkSessionAsUnhandled();
645+
646+
// Act - Explicitly end with Crashed status
647+
var sessionUpdate = sut.EndSession(SessionEndStatus.Crashed);
648+
649+
// Assert - Crashed status takes priority
650+
sessionUpdate.Should().NotBeNull();
651+
sessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed);
652+
sessionUpdate.ErrorCount.Should().Be(1);
653+
}
654+
655+
[Fact]
656+
public void SessionEscalation_CompleteFlow_UnhandledThenCrash()
657+
{
658+
// Arrange - Simulate complete flow
659+
var sut = _fixture.GetSut();
660+
sut.StartSession();
661+
var originalSessionId = sut.CurrentSession!.Id;
662+
663+
// Act 1: Mark as unhandled (game encounters exception but continues)
664+
sut.MarkSessionAsUnhandled();
665+
666+
// Assert: Session still active with pending flag
667+
sut.CurrentSession.Should().NotBeNull();
668+
sut.CurrentSession!.Id.Should().Be(originalSessionId);
669+
sut.CurrentSession.IsMarkedAsPendingUnhandled.Should().BeTrue();
670+
671+
// Act 2: Recover on next launch with crash detected
672+
_fixture.Options.CrashedLastRun = () => true;
673+
var recovered = sut.TryRecoverPersistedSession();
674+
675+
// Assert: Session escalated from Unhandled to Crashed
676+
recovered.Should().NotBeNull();
677+
recovered!.EndStatus.Should().Be(SessionEndStatus.Crashed);
678+
recovered.Id.Should().Be(originalSessionId);
679+
}
680+
526681
// A session update (of which the state doesn't matter for the test):
527682
private static SessionUpdate AnySessionUpdate()
528683
=> new(

‎test/Sentry.Tests/SentryClientTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,7 +1649,7 @@ public void CaptureEvent_ActiveSessionAndUnhandledException_SessionEndedAsCrashe
16491649
}
16501650

16511651
[Fact]
1652-
public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionEndedAsUnhandled()
1652+
public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionMarkedAsUnhandled()
16531653
{
16541654
// Arrange
16551655
var client = _fixture.GetSut();
@@ -1660,6 +1660,6 @@ public void CaptureEvent_ActiveSessionAndNonTerminalUnhandledException_SessionEn
16601660
client.CaptureEvent(new SentryEvent(exception));
16611661

16621662
// Assert
1663-
_fixture.SessionManager.Received().EndSession(SessionEndStatus.Unhandled);
1663+
_fixture.SessionManager.Received().MarkSessionAsUnhandled();
16641664
}
16651665
}

0 commit comments

Comments
 (0)