From 241ec58970cd1a9edbf804dcce577a0d2193a9f9 Mon Sep 17 00:00:00 2001 From: Alexey Golub Date: Fri, 25 Jun 2021 11:30:53 -0700 Subject: [PATCH] IHub.ResumeSession(): don't start a new session if pause wasn't called or if there is no active session (#1089) * IHub.ResumeSession(): don't start a new session if pause wasn't called or if there is no active session * Changelog * rearrange * asd --- CHANGELOG.md | 6 ++++ src/Sentry/GlobalSessionManager.cs | 2 ++ src/Sentry/ISessionManager.cs | 2 ++ src/Sentry/Internal/Hub.cs | 25 ++++++++++--- test/Sentry.Tests/HubTests.cs | 56 ++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d17e08ff1..e90f2c0e58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- `IHub.ResumeSession()`: don't start a new session if pause wasn't called or if there is no active session ([#1089](https://github.com/getsentry/sentry-dotnet/pull/1089)) + ## 3.6.0 ### Features diff --git a/src/Sentry/GlobalSessionManager.cs b/src/Sentry/GlobalSessionManager.cs index 99139fe49e..bb77dd0448 100644 --- a/src/Sentry/GlobalSessionManager.cs +++ b/src/Sentry/GlobalSessionManager.cs @@ -23,6 +23,8 @@ internal class GlobalSessionManager : ISessionManager // Internal for testing internal Session? CurrentSession => _currentSession; + public bool IsSessionActive => _currentSession is not null; + public GlobalSessionManager(SentryOptions options, ISystemClock clock) { _options = options; diff --git a/src/Sentry/ISessionManager.cs b/src/Sentry/ISessionManager.cs index 39f2aec4e1..c5396097ae 100644 --- a/src/Sentry/ISessionManager.cs +++ b/src/Sentry/ISessionManager.cs @@ -4,6 +4,8 @@ namespace Sentry { internal interface ISessionManager { + bool IsSessionActive { get; } + SessionUpdate? StartSession(); SessionUpdate? EndSession(DateTimeOffset timestamp, SessionEndStatus status); diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index b097d23829..dbcb5945db 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -22,7 +22,7 @@ internal class Hub : IHub, IDisposable private readonly IDisposable _rootScope; private readonly Enricher _enricher; - private DateTimeOffset _sessionPauseTimestamp; + private DateTimeOffset? _sessionPauseTimestamp; // Internal for testability internal ConditionalWeakTable ExceptionToSpanMap { get; } = new(); @@ -198,7 +198,13 @@ public void PauseSession() { lock (_sessionPauseLock) { - _sessionPauseTimestamp = _clock.GetUtcNow(); + // Only pause if there's anything to pause. + // This might race if a session is started at the same time, + // but that's fine. + if (_sessionManager.IsSessionActive) + { + _sessionPauseTimestamp = _clock.GetUtcNow(); + } } } @@ -206,7 +212,15 @@ public void ResumeSession() { lock (_sessionPauseLock) { - var pauseDuration = (_clock.GetUtcNow() - _sessionPauseTimestamp).Duration(); + // Ensure a session has been paused before + if (_sessionPauseTimestamp is not { } sessionPauseTimestamp) + { + return; + } + + // If the pause duration exceeded tracking interval, start a new session + // (otherwise do nothing) + var pauseDuration = (_clock.GetUtcNow() - sessionPauseTimestamp).Duration(); if (pauseDuration >= _options.AutoSessionTrackingInterval) { _options.DiagnosticLogger?.LogDebug( @@ -215,9 +229,12 @@ public void ResumeSession() pauseDuration ); - EndSession(_sessionPauseTimestamp, SessionEndStatus.Exited); + EndSession(sessionPauseTimestamp, SessionEndStatus.Exited); StartSession(); } + + // Reset the pause timestamp since the session is now resumed + _sessionPauseTimestamp = null; } } diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index ac721f2b1b..8081043a07 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -876,5 +876,61 @@ public void ResumeSession_BeyondAutoTrackingInterval_EndsPreviousSessionAndStart client.Received().CaptureSession(Arg.Is(s => s.EndStatus == SessionEndStatus.Exited)); client.Received().CaptureSession(Arg.Is(s => s.IsInitial)); } + + [Fact] + public void ResumeSession_NoActiveSession_DoesNothing() + { + // Arrange + var client = Substitute.For(); + var clock = Substitute.For(); + + var options = new SentryOptions + { + Dsn = DsnSamples.ValidDsnWithSecret, + AutoSessionTrackingInterval = TimeSpan.FromMilliseconds(10) + }; + + var hub = new Hub(client, clock, new GlobalSessionManager(options, clock), options); + + clock.GetUtcNow().Returns(DateTimeOffset.Now); + + hub.PauseSession(); + + clock.GetUtcNow().Returns(DateTimeOffset.Now + TimeSpan.FromDays(1)); + + // Act + hub.ResumeSession(); + + // Assert + client.DidNotReceive().CaptureSession(Arg.Any()); + } + + [Fact] + public void ResumeSession_NoPausedSession_DoesNothing() + { + // Arrange + var client = Substitute.For(); + var clock = Substitute.For(); + + var options = new SentryOptions + { + Dsn = DsnSamples.ValidDsnWithSecret, + AutoSessionTrackingInterval = TimeSpan.FromMilliseconds(10) + }; + + var hub = new Hub(client, clock, new GlobalSessionManager(options, clock), options); + + clock.GetUtcNow().Returns(DateTimeOffset.Now); + + hub.StartSession(); + + clock.GetUtcNow().Returns(DateTimeOffset.Now + TimeSpan.FromDays(1)); + + // Act + hub.ResumeSession(); + + // Assert + client.DidNotReceive().CaptureSession(Arg.Is(s => s.EndStatus != null)); + } } }